fix(test_runner): fix statement coverage visitor and edge cases

The acorn-walk `simple` function does not fire a generic "Statement"
visitor for concrete statement node types, which meant the statements
array was always empty and coveredStatementPercent was always 100%.

Fix by enumerating each concrete statement type (ExpressionStatement,
ReturnStatement, IfStatement, etc.) as individual visitor keys.

Also fixes:
- sourceType fallback: try 'module' first, catch and retry 'script'
- Infinity primordial: replace bare Infinity with bestRange null pattern
- double disk I/O: summary() now reads source once and passes it to
  both getLines() and getStatements()
- tests: assert totalStatementCount > 0 to catch this regression

Author: Felipeness

lib/internal/test_runner/coverage.js

@@ -43,6 +43,15 @@ const { Parser: AcornParser } =
 const { simple: acornWalkSimple } =
   require('internal/deps/acorn/acorn-walk/dist/walk');
 
+const kStatementTypes = [
+  'ExpressionStatement', 'ReturnStatement', 'ThrowStatement',
+  'IfStatement', 'WhileStatement', 'DoWhileStatement',
+  'ForStatement', 'ForInStatement', 'ForOfStatement',
+  'SwitchStatement', 'TryStatement', 'BreakStatement',
+  'ContinueStatement', 'VariableDeclaration', 'LabeledStatement',
+  'WithStatement', 'DebuggerStatement',
+];
+
 const kCoverageFileRegex = /^coverage-(\d+)-(\d{13})-(\d+)\.json$/;
 const kIgnoreRegex = /\/\* node:coverage ignore next (?<count>\d+ )?\*\//;
 const kLineEndingRegex = /\r?\n$/u;
@@ -88,34 +97,52 @@ class TestCoverage {
     }
 
     const statements = [];
+
+    // Build a visitor with one handler per concrete statement type.
+    // acorn-walk does not fire a generic "Statement" visitor for concrete
+    // node types, so each type must be registered individually.
+    const visitor = { __proto__: null };
+    const handler = (node) => {
+      ArrayPrototypePush(statements, {
+        __proto__: null,
+        startOffset: node.start,
+        endOffset: node.end,
+        count: 0,
+      });
+    };
+    for (let i = 0; i < kStatementTypes.length; ++i) {
+      visitor[kStatementTypes[i]] = handler;
+    }
+
+    // Try parsing as a module first, fall back to script for files that
+    // use CommonJS syntax incompatible with module mode.
+    let ast;
     try {
-      const ast = AcornParser.parse(source, {
+      ast = AcornParser.parse(source, {
         __proto__: null,
         ecmaVersion: 'latest',
         sourceType: 'module',
         allowReturnOutsideFunction: true,
         allowImportExportEverywhere: true,
         allowAwaitOutsideFunction: true,
       });
-
-      acornWalkSimple(ast, {
-        Statement(node) {
-          if (node.type === 'BlockStatement') {
-            return;
-          }
-          ArrayPrototypePush(statements, {
-            __proto__: null,
-            startOffset: node.start,
-            endOffset: node.end,
-            count: 0,
-          });
-        },
-      });
     } catch {
-      this.#sourceStatements.set(fileUrl, null);
-      return null;
+      try {
+        ast = AcornParser.parse(source, {
+          __proto__: null,
+          ecmaVersion: 'latest',
+          sourceType: 'script',
+          allowReturnOutsideFunction: true,
+          allowAwaitOutsideFunction: true,
+        });
+      } catch {
+        this.#sourceStatements.set(fileUrl, null);
+        return null;
+      }
     }
 
+    acornWalkSimple(ast, visitor);
+
     this.#sourceStatements.set(fileUrl, statements);
     return statements;
   }
@@ -228,7 +255,16 @@ class TestCoverage {
       const functionReports = [];
       const branchReports = [];
 
-      const lines = this.getLines(url);
+      // Read source once and pass to both getLines and getStatements to
+      // avoid double disk I/O for the same file.
+      let source;
+      try {
+        source = readFileSync(fileURLToPath(url), 'utf8');
+      } catch {
+        continue;
+      }
+
+      const lines = this.getLines(url, source);
       if (!lines) {
         continue;
       }
@@ -298,7 +334,8 @@ class TestCoverage {
       }
 
       // Compute statement coverage by mapping V8 ranges to AST statements.
-      const statements = this.getStatements(url);
+      // Pass the source already read above to avoid double disk I/O.
+      const statements = this.getStatements(url, source);
       let totalStatements = 0;
       let statementsCovered = 0;
       const statementReports = [];
@@ -307,8 +344,7 @@ class TestCoverage {
         for (let j = 0; j < statements.length; ++j) {
           const stmt = statements[j];
           let bestCount = 0;
-          let bestSize = Infinity;
-          let found = false;
+          let bestRange = null;
 
           for (let fi = 0; fi < functions.length; ++fi) {
             const { ranges } = functions[fi];
@@ -317,16 +353,16 @@ class TestCoverage {
               if (range.startOffset <= stmt.startOffset &&
                   range.endOffset >= stmt.endOffset) {
                 const size = range.endOffset - range.startOffset;
-                if (!found || size < bestSize) {
+                if (bestRange === null ||
+                    size < (bestRange.endOffset - bestRange.startOffset)) {
                   bestCount = range.count;
-                  bestSize = size;
+                  bestRange = range;
                 }
-                found = true;
               }
             }
           }
 
-          stmt.count = found ? bestCount : 0;
+          stmt.count = bestRange !== null ? bestCount : 0;
 
           const stmtLine = findLineForOffset(stmt.startOffset, lines);
           const isIgnored = stmtLine != null && stmtLine.ignore;

test/parallel/test-runner-coverage-statements.js

@@ -40,6 +40,11 @@ test('statement coverage is reported via custom reporter', () => {
             summary.totals.coveredStatementPercent <= 100,
             `statement percent should be 0-100, got ${summary.totals.coveredStatementPercent}`);
 
+  // Regression: totalStatementCount must be > 0 for files with actual
+  // statements. A zero here means the AST visitor is not firing.
+  assert.ok(summary.totals.totalStatementCount > 0,
+            `totalStatementCount must be > 0, got ${summary.totals.totalStatementCount}`);
+
   // Each file should have statement data
   for (const file of summary.files) {
     assert.ok('coveredStatementPercent' in file,
@@ -74,6 +79,11 @@ test('statement coverage reports covered and uncovered statements', () => {
 
   assert.ok(coverageFile, 'coverage.js should be in the report');
 
+  // Regression: totalStatementCount must be > 0 for files with actual
+  // statements. A zero here means the AST visitor is not firing.
+  assert.ok(coverageFile.totalStatementCount > 0,
+            `totalStatementCount must be > 0, got ${coverageFile.totalStatementCount}`);
+
   // The file has uncalled functions and dead branches, so statement
   // coverage should be less than 100%.
   assert.ok(coverageFile.coveredStatementPercent < 100,