Conversation
🦋 Changeset detectedLatest commit: 3e07005 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Netail
approved these changes
Feb 24, 2026
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis pull request refactors the JSON reporter by removing serde-derived serialization from reporter structs and switching to manual JSON construction. It adds command duration (and optional scannerDuration) to JSON output, using a placeholder string in debug builds and millisecond numbers in non-debug builds. A changeset documents a patch release for Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_cli/src/reporter/mod.rs (1)
36-54:⚠️ Potential issue | 🟡 MinorRemove unused
Serializederive fromTraversalSummary.The
Serializederive (and its import) is dead code—TraversalSummary is never serialised directly. json.rs manually constructs JSON output via factory functions; other reporters serialise their own structs, not TraversalSummary itself. Safe to remove both the derive and theuse serde::Serializeimport.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/src/reporter/mod.rs` around lines 36 - 54, Remove the unused Serialize derive and its import for TraversalSummary: delete Serialize from the #[derive(...)] on the TraversalSummary struct and remove the corresponding use serde::Serialize import; keep the other derives (Debug, Default, Copy, Clone) and the #[serde(rename_all = "camelCase")] attribute if still needed for any serde usage, and ensure json.rs continues to construct JSON via its factory functions without relying on TraversalSummary being serialized directly.
🧹 Nitpick comments (3)
crates/biome_cli/tests/cases/reporter_json.rs (2)
8-26:MAIN_1andMAIN_2are byte-for-byte identical.Two constants, one soul. If the intent is to test the reporter across multiple files, giving them different content (e.g., different lint violations) would make the snapshots more meaningful and easier to read.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/tests/cases/reporter_json.rs` around lines 8 - 26, The two test constants MAIN_1 and MAIN_2 are identical, so update one (e.g., MAIN_2) to contain different source text to exercise different reporter output; change MAIN_2 to introduce a distinct lint scenario (for example alter spacing, change a token to trigger a different rule, remove or add a line, or introduce a different error like a missing semicolon or unused variable) so the reporter snapshots differ—locate the constants MAIN_1 and MAIN_2 in the test and replace MAIN_2's string with the modified source that produces a different set of diagnostics.
29-207: Heavy boilerplate repetition across all five tests — consider a small helper.Each test recreates the same
MemoryFileSystem+BufferConsole+ two-file insert +run_cli+assert_cli_snapshotcycle. Extracting a helper keeps future additions down to a couple of lines.♻️ Sketch of a shared helper
+fn run_json_reporter(verb: &str, extra_args: &[&str]) -> (MemoryFileSystem, biome_console::BufferConsole, Result<(), _>) { + let fs = MemoryFileSystem::default(); + let mut console = BufferConsole::default(); + fs.insert(Utf8Path::new("main.ts").into(), MAIN_1.as_bytes()); + fs.insert(Utf8Path::new("index.ts").into(), MAIN_2.as_bytes()); + let mut args = vec![verb, "--reporter=json-pretty", "main.ts", "index.ts"]; + args.extend_from_slice(extra_args); + let (fs, result) = run_cli(fs, &mut console, Args::from(args.as_slice())); + (fs, console, result) +} + #[test] fn reports_diagnostics_json_check_command() { - let fs = MemoryFileSystem::default(); - let mut console = BufferConsole::default(); - let file_path1 = Utf8Path::new("main.ts"); - fs.insert(file_path1.into(), MAIN_1.as_bytes()); - let file_path2 = Utf8Path::new("index.ts"); - fs.insert(file_path2.into(), MAIN_2.as_bytes()); - let (fs, result) = run_cli(fs, &mut console, Args::from(["check", "--reporter=json-pretty", "main.ts", "index.ts"].as_slice())); + let (fs, console, result) = run_json_reporter("check", &[]); assert!(result.is_err(), "run_cli returned {result:?}"); assert_cli_snapshot(SnapshotPayload::new(module_path!(), "reports_diagnostics_json_check_command", fs, console, result)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/tests/cases/reporter_json.rs` around lines 29 - 207, Extract the repeated setup+execution+assertion into a small helper (e.g., fn run_report_test(args: &[&str], snapshot_name: &str) -> () ) that creates MemoryFileSystem and BufferConsole, inserts MAIN_1 and MAIN_2 into the fs, calls run_cli with Args::from(args), asserts result.is_err(), and calls assert_cli_snapshot with SnapshotPayload::new(module_path!(), snapshot_name, fs, console, result); then replace the bodies of the tests reports_diagnostics_json_check_command, reports_diagnostics_json_ci_command, reports_diagnostics_json_lint_command, reports_diagnostics_json_format_command, and reports_diagnostics_json_check_command_file to call this helper with the appropriate argument arrays (e.g., ["check","--reporter=json-pretty", file1, file2]) and snapshot names to remove boilerplate.crates/biome_cli/src/reporter/mod.rs (1)
137-153: Optional: groupscannerDurationadjacent todurationin the output.Currently
scannerDurationis appended afterdiagnosticsNotPrinted, so in the serialised JSON it sits far from its siblingduration. A reader of the output has to hunt for the related field. Consider collecting both duration fields together near the top of the members list.♻️ Suggested restructure
- let mut members = vec![ + // Build duration block first so related fields stay adjacent + #[cfg(debug_assertions)] + let scanner_duration_member = self.scanner_duration.map(|_| { + json_member( + AnyJsonMemberName::JsonMemberName(json_member_name(json_string_literal("scannerDuration"))), + token(T![:]), + AnyJsonValue::JsonStringValue(json_string_value(json_string_literal("<TIME>"))), + ) + }); + #[cfg(not(debug_assertions))] + let scanner_duration_member = self.scanner_duration.map(|d| { + json_member( + AnyJsonMemberName::JsonMemberName(json_member_name(json_string_literal("scannerDuration"))), + token(T![:]), + AnyJsonValue::JsonNumberValue(json_number_value(json_number_literal(d.as_millis()))), + ) + }); + + let mut members = vec![ json_member(/* changed */...), json_member(/* unchanged */...), json_member(/* matches */...), json_member(/* duration */ ..., duration_value), + // scannerDuration immediately follows duration when present ]; + if let Some(m) = scanner_duration_member { + members.push(m); + } members.extend([ json_member(/* errors */...), // … rest of fields … ]); - - if let Some(_scanner_duration) = self.scanner_duration { - // … build & push scannerDuration … - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/src/reporter/mod.rs` around lines 137 - 153, The scannerDuration member is currently pushed after diagnosticsNotPrinted which separates it from its sibling duration; move the scannerDuration construction & members.push block (the code referencing self.scanner_duration, _scanner_duration, AnyJsonValue::JsonStringValue/JsonNumberValue, and json_member with "scannerDuration") so it is inserted immediately adjacent to where duration is assembled and pushed into members (the existing duration-related code that creates the duration member), preserving the same cfg(debug_assertions) and value construction logic; remove the original scannerDuration push to avoid duplication and ensure the members order places "duration" and "scannerDuration" consecutively near the top of the members list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@crates/biome_cli/src/reporter/mod.rs`:
- Around line 36-54: Remove the unused Serialize derive and its import for
TraversalSummary: delete Serialize from the #[derive(...)] on the
TraversalSummary struct and remove the corresponding use serde::Serialize
import; keep the other derives (Debug, Default, Copy, Clone) and the
#[serde(rename_all = "camelCase")] attribute if still needed for any serde
usage, and ensure json.rs continues to construct JSON via its factory functions
without relying on TraversalSummary being serialized directly.
---
Nitpick comments:
In `@crates/biome_cli/src/reporter/mod.rs`:
- Around line 137-153: The scannerDuration member is currently pushed after
diagnosticsNotPrinted which separates it from its sibling duration; move the
scannerDuration construction & members.push block (the code referencing
self.scanner_duration, _scanner_duration,
AnyJsonValue::JsonStringValue/JsonNumberValue, and json_member with
"scannerDuration") so it is inserted immediately adjacent to where duration is
assembled and pushed into members (the existing duration-related code that
creates the duration member), preserving the same cfg(debug_assertions) and
value construction logic; remove the original scannerDuration push to avoid
duplication and ensure the members order places "duration" and "scannerDuration"
consecutively near the top of the members list.
In `@crates/biome_cli/tests/cases/reporter_json.rs`:
- Around line 8-26: The two test constants MAIN_1 and MAIN_2 are identical, so
update one (e.g., MAIN_2) to contain different source text to exercise different
reporter output; change MAIN_2 to introduce a distinct lint scenario (for
example alter spacing, change a token to trigger a different rule, remove or add
a line, or introduce a different error like a missing semicolon or unused
variable) so the reporter snapshots differ—locate the constants MAIN_1 and
MAIN_2 in the test and replace MAIN_2's string with the modified source that
produces a different set of diagnostics.
- Around line 29-207: Extract the repeated setup+execution+assertion into a
small helper (e.g., fn run_report_test(args: &[&str], snapshot_name: &str) -> ()
) that creates MemoryFileSystem and BufferConsole, inserts MAIN_1 and MAIN_2
into the fs, calls run_cli with Args::from(args), asserts result.is_err(), and
calls assert_cli_snapshot with SnapshotPayload::new(module_path!(),
snapshot_name, fs, console, result); then replace the bodies of the tests
reports_diagnostics_json_check_command, reports_diagnostics_json_ci_command,
reports_diagnostics_json_lint_command, reports_diagnostics_json_format_command,
and reports_diagnostics_json_check_command_file to call this helper with the
appropriate argument arrays (e.g., ["check","--reporter=json-pretty", file1,
file2]) and snapshot names to remove boilerplate.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
crates/biome_cli/tests/snapshots/main_cases_reporter_json/reports_diagnostics_json_check_command.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_json/reports_diagnostics_json_check_command_file.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_json/reports_diagnostics_json_ci_command.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_json/reports_diagnostics_json_format_command.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_cases_reporter_json/reports_diagnostics_json_lint_command.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (5)
.changeset/breezy-parrots-fold.mdcrates/biome_cli/src/reporter/json.rscrates/biome_cli/src/reporter/mod.rscrates/biome_cli/tests/cases/mod.rscrates/biome_cli/tests/cases/reporter_json.rs
ematipico
force-pushed
the
fix/json-reporter
branch
from
9150b25 to
3e07005
Compare
Open
This was referenced Feb 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Found it while working on the ecosystem CI.
durationandscannerDurationto the reportUsed AI to remove the code we didn't need. The rest wa me
Test Plan
We didn't have tests! I added them :)
Docs
I'll have to update the docs after we land this