Skip to content

feat: add collectCoverageFrom config option for uncovered file reporting#27694

Open
dvargas92495 wants to merge 2 commits intooven-sh:mainfrom
vellum-ai:devin/1772464004-collect-coverage-from
Open

feat: add collectCoverageFrom config option for uncovered file reporting#27694
dvargas92495 wants to merge 2 commits intooven-sh:mainfrom
vellum-ai:devin/1772464004-collect-coverage-from

Conversation

@dvargas92495
Copy link

@dvargas92495 dvargas92495 commented Mar 2, 2026

Implements a Jest-compatible collectCoverageFrom configuration option that allows users to specify glob patterns for source files that should appear in coverage reports even if they are not imported by any test.

This addresses #5928 where files not imported by tests are invisible in coverage reports, making it impossible to identify completely untested source files.

Changes:

  • Add collectCoverageFrom field to CodeCoverageOptions
  • Parse collectCoverageFrom in bunfig.toml (string or array of strings)
  • Walk filesystem to find matching files not in ByteRangeMapping
  • Show uncovered files with 0% coverage in text and LCOV reporters
  • Update documentation for the new configuration option

What does this PR do?

Add the collectCoverageFrom flag to enable collecting coverage from all files in a project, not just used ones.

How did you verify your code works?

I haven't yet - will run it against our repo later tonight. Hoping to at least further the conversation.

Implements a Jest-compatible collectCoverageFrom configuration option that
allows users to specify glob patterns for source files that should appear
in coverage reports even if they are not imported by any test.

This addresses oven-sh#5928 where files not imported by tests are
invisible in coverage reports, making it impossible to identify completely
untested source files.

Changes:
- Add collectCoverageFrom field to CodeCoverageOptions
- Parse collectCoverageFrom in bunfig.toml (string or array of strings)
- Walk filesystem to find matching files not in ByteRangeMapping
- Show uncovered files with 0% coverage in text and LCOV reporters
- Update documentation for the new configuration option

Co-Authored-By: vargas@vellum.ai <vargas@vellum.ai>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 2, 2026

Walkthrough

Adds a new collectCoverageFrom test configuration option: parsed from bunfig, used to discover uncovered JS/TS files, integrated into text and LCOV coverage reporting, with documentation updates showing configuration examples and troubleshooting guidance.

Changes

Cohort / File(s) Summary
Documentation
docs/test/code-coverage.mdx, docs/test/configuration.mdx
Adds guidance and examples for collectCoverageFrom, duplicates an "Including Uncovered Files" block near LCOV examples, and appends the option to the complete configuration example.
Config parsing
src/bunfig.zig
Parses collectCoverageFrom as a string or string-array, validates elements, allocates patterns array, and assigns to this.ctx.test_options.coverage.collect_coverage_from.
Coverage reporting / CLI
src/cli/test_command.zig
Adds collect_coverage_from to CodeCoverageOptions, implements findUncoveredFiles, computes uncovered_files from project patterns, and integrates uncovered paths into text and LCOV reporters (per-file metrics, averages, and LCOV entries).
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: adding a collectCoverageFrom config option for reporting uncovered files in code coverage.
Description check ✅ Passed The description provides clear context about what the PR does and addresses a specific issue. However, it lacks verification details as the author states they haven't verified the code yet.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cli/test_command.zig`:
- Around line 1041-1050: The early return in generateCodeCoverage when
byte_ranges is empty prevents the new findUncoveredFiles path from running for
the case where opts.collect_coverage_from is set; change the logic so we only
return early if byte_ranges is empty AND opts.collect_coverage_from.len == 0, or
alternatively compute uncovered_files (call findUncoveredFiles) before the early
return; update the code referencing byte_ranges, generateCodeCoverage,
findUncoveredFiles, and opts.collect_coverage_from/uncovered_files so that
collectCoverageFrom is honored even when byte_ranges is empty.
- Around line 1232-1233: The uncovered-file failure check currently sets
`failed` using only `base_fraction.functions` and `base_fraction.lines`; update
the boolean expression to also consider `base_fraction.statements` (e.g.,
include `base_fraction.statements > 0.0` in the OR chain) so that
`coverageThreshold.statements` configuration will cause uncovered files to mark
`failed` as true; modify the `failed` assignment (the symbol `failed`) to
include this additional condition referencing `base_fraction.statements`.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 32edef7 and e8b53d5.

📒 Files selected for processing (4)
  • docs/test/code-coverage.mdx
  • docs/test/configuration.mdx
  • src/bunfig.zig
  • src/cli/test_command.zig

}
}

if (comptime reporters.lcov) {
Copy link

@izaakschroeder izaakschroeder Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think most JS ecosystems that support this also include DA: LINE,0 to indicate which executable lines actually are missing coverage, as opposed to things like code comments. This is arguably important for calculating missing coverage amounts accurately (there will be a clear difference between the coverage result you give here vs. the one you get from simply await import(missingFile)).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can test against something like https://gist.github.com/izaakschroeder/5b883a31c5e39e987f70271d235adadc to see the difference.

Copy link

@izaakschroeder izaakschroeder Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally? you would want a way to feed these unexecuted files into JSC in such a way they get assigned a sourceId but are not executed, and then the internals can take probably take care of the rest:

auto basicBlocks = vm.controlFlowProfiler()->getBasicBlocksForSourceIDWithoutFunctionRange(

Perhaps

Ref<SourceProvider> SourceProvider::create(
would be a useful place to start?

- Fix early return in generateCodeCoverage that bypassed collectCoverageFrom
  when no files were imported (byte_ranges empty / map null)
- Add base_fraction.stmts to uncovered-file threshold check so
  coverageThreshold.statements triggers failure correctly
- Use bun.path.join() instead of std.fmt.allocPrint for path construction
  to follow codebase conventions for cross-platform handling
- Fix findUncoveredFiles to work when coverage map is null (no imports)
- Improve LCOV output: read actual file lines and emit DA:LINE,0 entries
  for each line instead of placeholder LF:1/LH:0

Co-Authored-By: vargas@vellum.ai <vargas@vellum.ai>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
src/cli/test_command.zig (1)

1433-1434: 🛠️ Refactor suggestion | 🟠 Major

Avoid std.fs.path.extension in src/**/*.zig unless there is a documented exception.

Line 1433 uses std.fs.path.extension(...) in newly added code; this conflicts with the repo rule to prefer bun.*/bun.path path APIs.

#!/bin/bash
set -euo pipefail

# Verify current usage in this file
rg -n "std\\.fs\\.path\\.extension\\(" src/cli/test_command.zig -C2

# Check whether a bun.path extension helper exists in repo APIs
rg -n "pub fn .*extension|fn .*extension" src -tzig | head -40

As per coding guidelines: "Always use bun.* APIs instead of std.* for cross-platform operations" and "Use bun.path functions instead of std.fs.path for path manipulation."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/test_command.zig` around lines 1433 - 1434, Replace the use of
std.fs.path.extension when computing ext from dir_entry.name with the
repo-approved bun.path API (e.g., bun.path.extension or the equivalent bun.path
helper), and update imports/usages in src/cli/test_command.zig so dir_entry.name
is fed to bun.path's extension helper before calling
bundle_options.loader(ext).isJavaScriptLike(); specifically, change the symbol
std.fs.path.extension to the bun.path extension helper and add any required
bun.path import or namespace qualification so the code compiles and follows the
cross-platform path API rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cli/test_command.zig`:
- Around line 1270-1301: The LCOV line counting loop (label count_lines using
file_for_lines, read_buf, and count) miscounts files without trailing newlines
and empty files; modify the loop to track whether any bytes were read and
whether the last byte seen was a newline (e.g., add boolean flags like saw_bytes
and last_was_newline updated while iterating read_buf), increment count for each
'\n' as now, but after the read loop set count = 0 if saw_bytes is false (empty
file) or if saw_bytes is true and last_was_newline is false then add 1 to
account for the final line without newline; keep defer f.close() and existing
error handling.
- Around line 1422-1438: The code allocates sub_path and abs_path with
bun.default_allocator but frees them with the function parameter allocator,
causing an allocator mismatch; change the allocations to use the passed-in
allocator (i.e., replace bun.default_allocator.dupe(...) with
allocator.dupe(...) where sub_path and abs_path are created) so that
deallocation sites (the error handler that calls allocator.free(sub_path) and
the defer allocator.free(abs_path)) match the allocator used for allocation in
the functions handling directory traversal and file path creation.
- Around line 993-1011: The byte_ranges
std.array_list.Managed(bun.SourceMap.coverage.ByteRangeMapping) created in the
brk block is never deinitialized and leaks on the early return; fix by adding a
defer to call byte_ranges.deinit() immediately after the brk block so the
managed list is always freed on return or when the function exits (i.e., insert
"defer byte_ranges.deinit();" right after the byte_ranges assignment),
referencing the variable byte_ranges and the Managed list type to locate the
change.

---

Duplicate comments:
In `@src/cli/test_command.zig`:
- Around line 1433-1434: Replace the use of std.fs.path.extension when computing
ext from dir_entry.name with the repo-approved bun.path API (e.g.,
bun.path.extension or the equivalent bun.path helper), and update imports/usages
in src/cli/test_command.zig so dir_entry.name is fed to bun.path's extension
helper before calling bundle_options.loader(ext).isJavaScriptLike();
specifically, change the symbol std.fs.path.extension to the bun.path extension
helper and add any required bun.path import or namespace qualification so the
code compiles and follows the cross-platform path API rule.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e8b53d5 and 38167bf.

📒 Files selected for processing (1)
  • src/cli/test_command.zig

Comment on lines +993 to 1011
var byte_ranges = brk: {
var map = coverage.ByteRangeMapping.map orelse {
// No coverage map exists at all. If collectCoverageFrom is set we still
// need to report uncovered files, so fall through with an empty slice.
if (opts.collect_coverage_from.len > 0) {
break :brk std.array_list.Managed(bun.SourceMap.coverage.ByteRangeMapping).init(bun.default_allocator);
}
return;
};
var iter = map.valueIterator();
var list = try std.array_list.Managed(bun.SourceMap.coverage.ByteRangeMapping).initCapacity(bun.default_allocator, map.count());
while (iter.next()) |entry| {
list.appendAssumeCapacity(entry.*);
}
break :brk list;
};

if (byte_ranges.items.len == 0) {
if (byte_ranges.items.len == 0 and opts.collect_coverage_from.len == 0) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd src/cli && wc -l test_command.zig

Repository: oven-sh/bun

Length of output: 76


🏁 Script executed:

cd src/cli && sed -n '990,1020p' test_command.zig

Repository: oven-sh/bun

Length of output: 1240


🏁 Script executed:

cd src/cli && sed -n '1000,1050p' test_command.zig

Repository: oven-sh/bun

Length of output: 1921


🏁 Script executed:

cd src/cli && sed -n '1020,1060p' test_command.zig

Repository: oven-sh/bun

Length of output: 1502


🏁 Script executed:

cd src/cli && sed -n '980,1030p' test_command.zig | cat -n

Repository: oven-sh/bun

Length of output: 2476


🏁 Script executed:

cd src/cli && sed -n '980,1050p' test_command.zig | cat -n

Repository: oven-sh/bun

Length of output: 3394


🏁 Script executed:

cd src/cli && sed -n '1050,1100p' test_command.zig

Repository: oven-sh/bun

Length of output: 1875


🏁 Script executed:

cd src/cli && sed -n '1020,1025p' test_command.zig

Repository: oven-sh/bun

Length of output: 221


🏁 Script executed:

cd src/cli && awk '/pub fn generateCodeCoverage/,/^    pub fn [^g]/ {print NR": "$0}' test_command.zig | head -100

Repository: oven-sh/bun

Length of output: 1854


🏁 Script executed:

cd src/cli && sed -n '1024,1035p' test_command.zig

Repository: oven-sh/bun

Length of output: 599


🏁 Script executed:

cd src/cli && grep -A 5 "pub fn printCodeCoverage" test_command.zig | head -12

Repository: oven-sh/bun

Length of output: 313


🏁 Script executed:

cd src/cli && grep -n "printCodeCoverage" test_command.zig

Repository: oven-sh/bun

Length of output: 404


🏁 Script executed:

cd src/cli && rg "byte_ranges.*deinit" src/cli/test_command.zig

Repository: oven-sh/bun

Length of output: 119


🏁 Script executed:

rg "byte_ranges.*deinit" src/

Repository: oven-sh/bun

Length of output: 37


🏁 Script executed:

cd src/cli && grep -B5 -A20 "pub fn generateCodeCoverage" test_command.zig | tail -30

Repository: oven-sh/bun

Length of output: 1283


Add defer to deinitialize byte_ranges heap allocation.

byte_ranges is a std.array_list.Managed heap allocation created at lines 998 or 1003, but it is never deinitialized. The early return at line 1012 leaks the list, and the implicit end of scope at the function return (line 1022) also leaks. In repeated or watch-mode executions, this causes unbounded heap growth.

🔧 Proposed fix
        var byte_ranges = brk: {
            var map = coverage.ByteRangeMapping.map orelse {
                // No coverage map exists at all. If collectCoverageFrom is set we still
                // need to report uncovered files, so fall through with an empty slice.
                if (opts.collect_coverage_from.len > 0) {
                    break :brk std.array_list.Managed(bun.SourceMap.coverage.ByteRangeMapping).init(bun.default_allocator);
                }
                return;
            };
            var iter = map.valueIterator();
            var list = try std.array_list.Managed(bun.SourceMap.coverage.ByteRangeMapping).initCapacity(bun.default_allocator, map.count());
            while (iter.next()) |entry| {
                list.appendAssumeCapacity(entry.*);
            }
            break :brk list;
        };
+        defer byte_ranges.deinit(bun.default_allocator);

         if (byte_ranges.items.len == 0 and opts.collect_coverage_from.len == 0) {
             return;
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/test_command.zig` around lines 993 - 1011, The byte_ranges
std.array_list.Managed(bun.SourceMap.coverage.ByteRangeMapping) created in the
brk block is never deinitialized and leaks on the early return; fix by adding a
defer to call byte_ranges.deinit() immediately after the brk block so the
managed list is always freed on return or when the function exits (i.e., insert
"defer byte_ranges.deinit();" right after the byte_ranges assignment),
referencing the variable byte_ranges and the Managed list type to locate the
change.

Comment on lines +1270 to +1301
// Read the file to count lines and emit DA:LINE,0 for each executable line
const abs_uncovered = bun.path.joinAbsStringZ(
relative_dir,
&.{rel_path},
.auto,
);
const line_count: usize = count_lines: {
const file_for_lines = bun.sys.File.openat(
.cwd(),
abs_uncovered,
bun.O.RDONLY | bun.O.CLOEXEC,
0,
);
switch (file_for_lines) {
.err => break :count_lines 0,
.result => |f| {
defer f.close();
var count: usize = 0;
var read_buf: [4096]u8 = undefined;
while (true) {
const n = switch (f.read(&read_buf)) {
.result => |n| n,
.err => break :count_lines count,
};
if (n == 0) break;
for (read_buf[0..n]) |byte| {
if (byte == '\n') count += 1;
}
}
// Account for last line without trailing newline
if (count == 0) count = 1;
break :count_lines count;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/cli/test_command.zig | sed -n '1270,1310p'

Repository: oven-sh/bun

Length of output: 2285


Fix LCOV line counting for files without trailing newlines and empty files.

Current code only handles the zero-newline case and incorrectly counts files like a\nb as 1 line instead of 2. Empty files are also incorrectly reported as 1 line instead of 0.

The fix tracks whether any bytes were read and whether the file ends with a newline, then adds 1 to the count only when the file has content but doesn't end with a newline:

-                const line_count: usize = count_lines: {
+                const line_count: usize = count_lines: {
                     const file_for_lines = bun.sys.File.openat(
                         .cwd(),
                         abs_uncovered,
                         bun.O.RDONLY | bun.O.CLOEXEC,
                         0,
                     );
                     switch (file_for_lines) {
                         .err => break :count_lines 0,
                         .result => |f| {
                             defer f.close();
                             var count: usize = 0;
+                            var saw_any_bytes = false;
+                            var ends_with_newline = false;
                             var read_buf: [4096]u8 = undefined;
                             while (true) {
                                 const n = switch (f.read(&read_buf)) {
                                     .result => |n| n,
                                     .err => break :count_lines count,
                                 };
                                 if (n == 0) break;
+                                saw_any_bytes = true;
                                 for (read_buf[0..n]) |byte| {
                                     if (byte == '\n') count += 1;
                                 }
+                                ends_with_newline = read_buf[n - 1] == '\n';
                             }
-                            // Account for last line without trailing newline
-                            if (count == 0) count = 1;
+                            if (saw_any_bytes and !ends_with_newline) count += 1;
                             break :count_lines count;
                         },
                     }
                 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Read the file to count lines and emit DA:LINE,0 for each executable line
const abs_uncovered = bun.path.joinAbsStringZ(
relative_dir,
&.{rel_path},
.auto,
);
const line_count: usize = count_lines: {
const file_for_lines = bun.sys.File.openat(
.cwd(),
abs_uncovered,
bun.O.RDONLY | bun.O.CLOEXEC,
0,
);
switch (file_for_lines) {
.err => break :count_lines 0,
.result => |f| {
defer f.close();
var count: usize = 0;
var read_buf: [4096]u8 = undefined;
while (true) {
const n = switch (f.read(&read_buf)) {
.result => |n| n,
.err => break :count_lines count,
};
if (n == 0) break;
for (read_buf[0..n]) |byte| {
if (byte == '\n') count += 1;
}
}
// Account for last line without trailing newline
if (count == 0) count = 1;
break :count_lines count;
// Read the file to count lines and emit DA:LINE,0 for each executable line
const abs_uncovered = bun.path.joinAbsStringZ(
relative_dir,
&.{rel_path},
.auto,
);
const line_count: usize = count_lines: {
const file_for_lines = bun.sys.File.openat(
.cwd(),
abs_uncovered,
bun.O.RDONLY | bun.O.CLOEXEC,
0,
);
switch (file_for_lines) {
.err => break :count_lines 0,
.result => |f| {
defer f.close();
var count: usize = 0;
var saw_any_bytes = false;
var ends_with_newline = false;
var read_buf: [4096]u8 = undefined;
while (true) {
const n = switch (f.read(&read_buf)) {
.result => |n| n,
.err => break :count_lines count,
};
if (n == 0) break;
saw_any_bytes = true;
for (read_buf[0..n]) |byte| {
if (byte == '\n') count += 1;
}
ends_with_newline = read_buf[n - 1] == '\n';
}
if (saw_any_bytes and !ends_with_newline) count += 1;
break :count_lines count;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/test_command.zig` around lines 1270 - 1301, The LCOV line counting
loop (label count_lines using file_for_lines, read_buf, and count) miscounts
files without trailing newlines and empty files; modify the loop to track
whether any bytes were read and whether the last byte seen was a newline (e.g.,
add boolean flags like saw_bytes and last_was_newline updated while iterating
read_buf), increment count for each '\n' as now, but after the read loop set
count = 0 if saw_bytes is false (empty file) or if saw_bytes is true and
last_was_newline is false then add 1 to account for the final line without
newline; keep defer f.close() and existing error handling.

Comment on lines +1422 to +1438
const sub_path = bun.default_allocator.dupe(u8, bun.path.join(&[_]string{ dir_path, dir_entry.name }, .auto)) catch continue;
dir_stack.append(allocator, sub_path) catch {
allocator.free(sub_path);
continue;
};
continue;
}

if (dir_entry.kind != .file) continue;

// Check if this is a JS/TS file
const ext = std.fs.path.extension(dir_entry.name);
if (!bundle_options.loader(ext).isJavaScriptLike()) continue;

// Build absolute path and compute relative path
const abs_path = bun.default_allocator.dupe(u8, bun.path.join(&[_]string{ dir_path, dir_entry.name }, .auto)) catch continue;
defer allocator.free(abs_path);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find and read the relevant section around lines 1400-1450
sed -n '1395,1450p' src/cli/test_command.zig

Repository: oven-sh/bun

Length of output: 2200


🏁 Script executed:

# Find the function definition containing this code
sed -n '1370,1405p' src/cli/test_command.zig | head -40

Repository: oven-sh/bun

Length of output: 1305


🏁 Script executed:

# Let's search backwards from line 1422 to find the function signature
rg -n 'fn\s+\w+.*allocator' src/cli/test_command.zig | grep -B5 '142[0-9]'

Repository: oven-sh/bun

Length of output: 37


🏁 Script executed:

# Get broader context - find the function containing lines 1422-1438
sed -n '1250,1430p' src/cli/test_command.zig | tail -200

Repository: oven-sh/bun

Length of output: 7119


Use the function allocator consistently for path allocation and deallocation.

Lines 1422 and 1437 allocate paths with bun.default_allocator, but they are freed with the allocator parameter on lines 1424 (error path) and 1438. This allocator mismatch can lead to memory corruption when deallocating with a different allocator than was used for allocation.

Both sub_path (line 1422) and abs_path (line 1437) should be allocated with the allocator parameter to match their deallocation sites (line 1424 error handler, line 1405 defer block, and line 1438 defer statement respectively).

Proposed fix
-                const sub_path = bun.default_allocator.dupe(u8, bun.path.join(&[_]string{ dir_path, dir_entry.name }, .auto)) catch continue;
+                const sub_path = allocator.dupe(u8, bun.path.join(&[_]string{ dir_path, dir_entry.name }, .auto)) catch continue;
                 dir_stack.append(allocator, sub_path) catch {
                     allocator.free(sub_path);
                     continue;
                 };
                 continue;
             }

             if (dir_entry.kind != .file) continue;

             // Check if this is a JS/TS file
             const ext = std.fs.path.extension(dir_entry.name);
             if (!bundle_options.loader(ext).isJavaScriptLike()) continue;

             // Build absolute path and compute relative path
-            const abs_path = bun.default_allocator.dupe(u8, bun.path.join(&[_]string{ dir_path, dir_entry.name }, .auto)) catch continue;
+            const abs_path = allocator.dupe(u8, bun.path.join(&[_]string{ dir_path, dir_entry.name }, .auto)) catch continue;
             defer allocator.free(abs_path);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const sub_path = bun.default_allocator.dupe(u8, bun.path.join(&[_]string{ dir_path, dir_entry.name }, .auto)) catch continue;
dir_stack.append(allocator, sub_path) catch {
allocator.free(sub_path);
continue;
};
continue;
}
if (dir_entry.kind != .file) continue;
// Check if this is a JS/TS file
const ext = std.fs.path.extension(dir_entry.name);
if (!bundle_options.loader(ext).isJavaScriptLike()) continue;
// Build absolute path and compute relative path
const abs_path = bun.default_allocator.dupe(u8, bun.path.join(&[_]string{ dir_path, dir_entry.name }, .auto)) catch continue;
defer allocator.free(abs_path);
const sub_path = allocator.dupe(u8, bun.path.join(&[_]string{ dir_path, dir_entry.name }, .auto)) catch continue;
dir_stack.append(allocator, sub_path) catch {
allocator.free(sub_path);
continue;
};
continue;
}
if (dir_entry.kind != .file) continue;
// Check if this is a JS/TS file
const ext = std.fs.path.extension(dir_entry.name);
if (!bundle_options.loader(ext).isJavaScriptLike()) continue;
// Build absolute path and compute relative path
const abs_path = allocator.dupe(u8, bun.path.join(&[_]string{ dir_path, dir_entry.name }, .auto)) catch continue;
defer allocator.free(abs_path);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/test_command.zig` around lines 1422 - 1438, The code allocates
sub_path and abs_path with bun.default_allocator but frees them with the
function parameter allocator, causing an allocator mismatch; change the
allocations to use the passed-in allocator (i.e., replace
bun.default_allocator.dupe(...) with allocator.dupe(...) where sub_path and
abs_path are created) so that deallocation sites (the error handler that calls
allocator.free(sub_path) and the defer allocator.free(abs_path)) match the
allocator used for allocation in the functions handling directory traversal and
file path creation.

}
};

for (1..line_count + 1) |line_num| {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slightly better 😄 but still doesn't handle things like comments and other lines that are not executable. If you test your code on a TS/JS file that looks like:

// Hello
// World

console.log(3);

You will get a total of 4 DA entries instead of the correct number of entries: 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants