-
-
Notifications
You must be signed in to change notification settings - Fork 716
feat(cli): rdjson reporter #7698
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: c68dece The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
CodSpeed Performance ReportMerging #7698 will not alter performanceComparing Summary
Footnotes
|
170e8e7
to
c6ff794
Compare
WalkthroughAdds an RDJSON reporter to the CLI: extends Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (3)crates/biome_*/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (1)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (5)
crates/biome_cli/src/execute/mod.rs (1)
250-252
: Replace non-authoritative link in docs.The comment links to deepwiki. Please replace with an authoritative Reviewdog/RDJSON reference or remove the link.
If helpful, I can search for the canonical spec and update the docstring.
crates/biome_cli/tests/cases/reporter_rdjson.rs (1)
28-170
: Reduce duplication with a small helper.Setup/exec/assert blocks are copy‑pasted across tests. Extract a helper like run_and_assert(cmd_name, snapshot) and reuse it; also reuse a single SRC const for both files.
crates/biome_cli/src/cli_options.rs (1)
161-163
: Replace non-authoritative link in docs.Avoid deepwiki; link to Reviewdog’s RDJSON reference or repo docs instead, or omit until sourced.
crates/biome_cli/src/reporter/rdjson.rs (2)
191-201
: Typo in function name.to_rdjson_suggetions → to_rdjson_suggestions.
Apply this diff:
-fn to_rdjson_suggetions(diagnostic: &Error) -> Vec<RdJsonSuggestion> { +fn to_rdjson_suggestions(diagnostic: &Error) -> Vec<RdJsonSuggestion> {And update the call site accordingly.
216-223
: Add diagnostic.severity RDJSON schema recommends including a severity (UNKNOWN / ERROR / WARNING / INFO); map Biome severity levels accordingly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
crates/biome_cli/tests/snapshots/main_cases_reporter_rdjson/reports_diagnostics_rdjson_check_command.snap
is excluded by!**/*.snap
and included by**
crates/biome_cli/tests/snapshots/main_cases_reporter_rdjson/reports_diagnostics_rdjson_ci_command.snap
is excluded by!**/*.snap
and included by**
crates/biome_cli/tests/snapshots/main_cases_reporter_rdjson/reports_diagnostics_rdjson_format_command.snap
is excluded by!**/*.snap
and included by**
crates/biome_cli/tests/snapshots/main_cases_reporter_rdjson/reports_diagnostics_rdjson_lint_command.snap
is excluded by!**/*.snap
and included by**
crates/biome_cli/tests/snapshots/main_commands_check/check_help.snap
is excluded by!**/*.snap
and included by**
crates/biome_cli/tests/snapshots/main_commands_ci/ci_help.snap
is excluded by!**/*.snap
and included by**
crates/biome_cli/tests/snapshots/main_commands_format/format_help.snap
is excluded by!**/*.snap
and included by**
crates/biome_cli/tests/snapshots/main_commands_lint/lint_help.snap
is excluded by!**/*.snap
and included by**
crates/biome_cli/tests/snapshots/main_commands_migrate/migrate_help.snap
is excluded by!**/*.snap
and included by**
crates/biome_cli/tests/snapshots/main_commands_rage/rage_help.snap
is excluded by!**/*.snap
and included by**
📒 Files selected for processing (10)
.changeset/gentle-pots-hunt.md
(1 hunks)crates/biome_cli/src/cli_options.rs
(4 hunks)crates/biome_cli/src/execute/mod.rs
(4 hunks)crates/biome_cli/src/reporter/mod.rs
(1 hunks)crates/biome_cli/src/reporter/rdjson.rs
(1 hunks)crates/biome_cli/tests/cases/mod.rs
(1 hunks)crates/biome_cli/tests/cases/reporter_rdjson.rs
(1 hunks)crates/biome_cli/tests/snapshots/main_cases_html/should_apply_fixes_to_embedded_languages.snap.new
(1 hunks)crates/biome_diagnostics/src/display.rs
(1 hunks)crates/biome_diagnostics/src/display_github.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_cli/tests/cases/mod.rs
crates/biome_diagnostics/src/display_github.rs
crates/biome_cli/src/cli_options.rs
crates/biome_cli/tests/snapshots/main_cases_html/should_apply_fixes_to_embedded_languages.snap.new
crates/biome_cli/tests/cases/reporter_rdjson.rs
crates/biome_cli/src/reporter/mod.rs
crates/biome_cli/src/execute/mod.rs
crates/biome_cli/src/reporter/rdjson.rs
crates/biome_diagnostics/src/display.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_cli/tests/cases/mod.rs
crates/biome_cli/tests/snapshots/main_cases_html/should_apply_fixes_to_embedded_languages.snap.new
crates/biome_cli/tests/cases/reporter_rdjson.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f
/just format
)
Files:
crates/biome_cli/tests/cases/mod.rs
crates/biome_diagnostics/src/display_github.rs
crates/biome_cli/src/cli_options.rs
crates/biome_cli/tests/cases/reporter_rdjson.rs
crates/biome_cli/src/reporter/mod.rs
crates/biome_cli/src/execute/mod.rs
crates/biome_cli/src/reporter/rdjson.rs
crates/biome_diagnostics/src/display.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_cli/tests/cases/mod.rs
crates/biome_diagnostics/src/display_github.rs
crates/biome_cli/src/cli_options.rs
crates/biome_cli/tests/cases/reporter_rdjson.rs
crates/biome_cli/src/reporter/mod.rs
crates/biome_cli/src/execute/mod.rs
crates/biome_cli/src/reporter/rdjson.rs
crates/biome_diagnostics/src/display.rs
crates/biome_diagnostics/**/*.rs
📄 CodeRabbit inference engine (crates/biome_diagnostics/CONTRIBUTING.md)
crates/biome_diagnostics/**/*.rs
: Prefer implementing diagnostics via #[derive(Diagnostic)] rather than manual trait impls
Types deriving Diagnostic must implement Debug
Fields annotated with #[advice] or #[verbose_advice] must implement the Advices trait
Use helper advice types (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) from biome_diagnostics::v2 when suitable; implement Advices manually only when needed for control/performance
When deriving Diagnostic for an enum, ensure each variant contains a type that is itself a diagnostic
Files:
crates/biome_diagnostics/src/display_github.rs
crates/biome_diagnostics/src/display.rs
.changeset/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/**/*.md
: Create changesets using thejust new-changeset
command; do not author them manually
In changeset markdown, only use headers #### or #####
Changeset descriptions must end every sentence with a full stop (.)
For bug fixes, start the changeset description with a linked issue reference like “Fixed #1234”
Prefer past tense for what was done and present tense for current behavior in changesets
Files:
.changeset/gentle-pots-hunt.md
🧠 Learnings (3)
📚 Learning: 2025-10-02T12:56:59.406Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:56:59.406Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Types deriving Diagnostic must implement Debug
Applied to files:
crates/biome_diagnostics/src/display_github.rs
📚 Learning: 2025-10-02T12:57:33.209Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.209Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/specs/**/*.jsonc : Use .jsonc files to hold arrays of code snippets for snapshot tests; these snippets run in script mode (no import/export)
Applied to files:
crates/biome_cli/tests/snapshots/main_cases_html/should_apply_fixes_to_embedded_languages.snap.new
📚 Learning: 2025-10-02T13:00:18.232Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T13:00:18.232Z
Learning: Applies to crates/biome_service/**/biome_lsp/src/server.tests.rs : End-to-end LSP tests reside in biome_lsp/src/server.tests.rs
Applied to files:
crates/biome_cli/tests/snapshots/main_cases_html/should_apply_fixes_to_embedded_languages.snap.new
🧬 Code graph analysis (4)
crates/biome_diagnostics/src/display_github.rs (1)
crates/biome_diagnostics/src/display.rs (1)
markup_to_string
(692-698)
crates/biome_cli/tests/cases/reporter_rdjson.rs (3)
crates/biome_cli/tests/main.rs (1)
run_cli
(332-347)crates/biome_cli/tests/snap_test.rs (1)
assert_cli_snapshot
(407-409)crates/biome_cli/src/execute/mod.rs (4)
new
(280-286)from
(77-79)from
(89-91)from
(263-276)
crates/biome_cli/src/reporter/rdjson.rs (4)
crates/biome_cli/src/cli_options.rs (5)
fmt
(191-202)fmt
(234-243)from
(266-271)from
(275-280)default
(228-230)crates/biome_diagnostics/src/display.rs (8)
fmt
(30-35)fmt
(73-91)fmt
(98-208)fmt
(361-376)markup_to_string
(692-698)verbose
(55-61)location
(783-789)source
(795-797)crates/biome_diagnostics/src/display_github.rs (2)
fmt
(13-73)fmt
(79-83)crates/biome_cli/src/reporter/mod.rs (3)
write
(50-50)report_summary
(56-61)report_diagnostics
(73-79)
crates/biome_diagnostics/src/display.rs (2)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
MarkupBuf
(9233-9233)crates/biome_cli/src/reporter/terminal.rs (6)
write
(27-40)fmt
(156-163)fmt
(169-184)fmt
(190-194)fmt
(200-237)fmt
(246-302)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: autofix
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
🔇 Additional comments (8)
crates/biome_diagnostics/src/display.rs (1)
691-698
: Shared markup stringifier looks neatNice to see the conversion logic centralised; the helper is tidy and matches the old display_github code. Cheers.
crates/biome_cli/src/execute/mod.rs (3)
16-16
: RDJSON reporter wired-in correctly.Import aligns with the new reporter module. No issues.
274-275
: Mapping looks good.CliReporter::RdJson → ReportMode::RdJson is correct and consistent.
710-718
: Execution path for RDJSON LGTM.Instantiates and invokes the visitor like the other reporters. No concerns.
crates/biome_cli/src/cli_options.rs (3)
59-61
: CLI surface updated correctly.--reporter recognises rdjson. Parsing fallback remains sane.
182-182
: Parser arm for rdjson is correct.Matches Display/From mapping elsewhere.
200-201
: Display string for rdjson is consistent.All good.
crates/biome_diagnostics/src/display_github.rs (1)
2-2
: Nice reuse of shared utility.Switching to crate::display::markup_to_string de-duplicates logic and keeps behaviour consistent across outputs. Based on learnings.
Docs biomejs/website#3194 |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
crates/biome_cli/src/reporter/rdjson.rs (4)
75-79
: Convert serde_json errors into io::Error
serde_json::to_string_pretty
yieldsserde_json::Error
, so using?
here breaks theio::Result
contract. Wrap it in anio::Error
before bubbling out so the code compiles.- let result = serde_json::to_string_pretty(&report)?; + let result = serde_json::to_string_pretty(&report) + .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
105-112
: Switch RDJSON ranges to 1-based indicesRDJSON consumers (e.g. reviewdog) expect 1-based line/column values. Emitting zero-based positions breaks their parsing. Add one to both start/end positions.
- column: start.column_number.to_zero_indexed(), - line: start.line_number.to_zero_indexed(), + column: start.column_number.to_zero_indexed() + 1, + line: start.line_number.to_zero_indexed() + 1, ... - column: end.column_number.to_zero_indexed(), - line: end.line_number.to_zero_indexed(), + column: end.column_number.to_zero_indexed() + 1, + line: end.line_number.to_zero_indexed() + 1,
155-157
: Fallback when markup serialisation fails
markup_to_string
can returnNone
; callingexpect
will crash the reporter. Fall back to the plain text so we still emit something sensible.- markup_to_string(&message).expect("Invalid markup") + markup_to_string(&message).unwrap_or_else(|| text.to_string())
176-199
: Skip suggestions without a valid rangeDefaulting to
0:0–0:0
(andexpect
on spans) violates the RDJSON contract and risks panics. Only push a suggestion when both span locations resolve, otherwise quietly skip.- let range = if let (Some(span), Some(source_code)) = (location.span, location.source_code) { - let source = SourceFile::new(source_code); - let start = source.location(span.start()).expect("Invalid span"); - let end = source.location(span.end()).expect("Invalid span"); - - RdJsonRange { - end: RdJsonLineColumn { - line: end.line_number.to_zero_indexed(), - column: end.column_number.to_zero_indexed(), - }, - start: RdJsonLineColumn { - line: start.line_number.to_zero_indexed(), - column: start.column_number.to_zero_indexed(), - }, - } - } else { - RdJsonRange::default() - }; - - self.last_diagnostic_length = self.suggestions.len(); - self.suggestions.push(RdJsonSuggestion { - text: self.current_message.take().unwrap_or_default(), - range, - }); + if let (Some(span), Some(source_code)) = (location.span, location.source_code) { + let source = SourceFile::new(source_code); + if let (Ok(start), Ok(end)) = ( + source.location(span.start()), + source.location(span.end()), + ) { + let range = RdJsonRange { + end: RdJsonLineColumn { + line: end.line_number.to_zero_indexed() + 1, + column: end.column_number.to_zero_indexed() + 1, + }, + start: RdJsonLineColumn { + line: start.line_number.to_zero_indexed() + 1, + column: start.column_number.to_zero_indexed() + 1, + }, + }; + + self.last_diagnostic_length = self.suggestions.len(); + self.suggestions.push(RdJsonSuggestion { + text: self.current_message.take().unwrap_or_default(), + range, + }); + } + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_cli/src/reporter/rdjson.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_cli/src/reporter/rdjson.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f
/just format
)
Files:
crates/biome_cli/src/reporter/rdjson.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_cli/src/reporter/rdjson.rs
🧬 Code graph analysis (1)
crates/biome_cli/src/reporter/rdjson.rs (4)
crates/biome_cli/src/cli_options.rs (6)
fmt
(191-202)fmt
(234-243)s
(252-252)from
(266-271)from
(275-280)default
(228-230)crates/biome_diagnostics/src/display.rs (15)
fmt
(30-35)fmt
(73-91)fmt
(98-208)fmt
(361-376)markup_to_string
(692-698)verbose
(55-61)location
(783-789)source
(795-797)category
(751-753)record_log
(416-423)record_log
(568-571)record_frame
(299-304)record_frame
(382-384)record_frame
(441-443)record_frame
(578-581)crates/biome_cli/src/reporter/mod.rs (3)
write
(50-50)report_summary
(56-61)report_diagnostics
(73-79)crates/biome_diagnostics/src/location.rs (1)
source_code
(216-219)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: autofix
I also can't really find any concrete info about RDJSON. Why should we have it? |
I know it was requested somewhere, and Ruff has it too |
Summary
This PR adds a new reporter to emit diagnostics using the RDJSON format.
I couldn't find a webpage that explains the format, so I had to use the one from deepwiki (AI generated) in the changeset.
If you happen to find the real source, please let me know
Test Plan
Added new tests
Docs
biomejs/website#3194