Conversation
🦋 Changeset detectedLatest commit: f22d006 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 |
Netail
force-pushed
the
feat/multi-reporters
branch
from
e23f494 to
854d552
Compare
Netail
force-pushed
the
feat/multi-reporters
branch
4 times, most recently
from
58da66d to
aed0cb4
Compare
WalkthroughReplaces the single-reporter CLI with multi-reporter support: CliOptions now accepts multiple Possibly related PRs
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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_diagnostics/src/error.rs (1)
4-9: Update the documentation to reflect Arc usage.The comment still references
Box<Box<dyn Diagnostic>>, but the implementation now usesArc<dyn Diagnostic>. This should be updated to reflect the current design and explain why Arc was chosen despite the size trade-off.
🧹 Nitpick comments (1)
crates/biome_cli/src/execute/mod.rs (1)
622-744: Consider optimising payload cloning.The
diagnostics_payloadis cloned for each reporter (lines 628, 638, 653, 693, 702, 715, 725, 736). Whilst necessary given the currentReportertrait design, this could be expensive with large diagnostic counts since it containsVec<Error>.Consider wrapping
DiagnosticsPayloadinArcor refactoring theReportertrait to accept references if this becomes a performance bottleneck.💡 Potential optimisation approach
Wrap the payload in
Arcto enable cheap cloning:+use std::sync::Arc; + let diagnostics_payload = DiagnosticsPayload { diagnostic_level: cli_options.diagnostic_level, diagnostics, max_diagnostics: cli_options.max_diagnostics, }; +let diagnostics_payload = Arc::new(diagnostics_payload); for report_mode in &execution.report_mode { match report_mode { ReportMode::Terminal { with_summary } => { if *with_summary { let reporter = SummaryReporter { summary, - diagnostics_payload: diagnostics_payload.clone(), + diagnostics_payload: Arc::clone(&diagnostics_payload),Then update reporter structs to hold
Arc<DiagnosticsPayload>instead.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/slimy-files-feel.mdcrates/biome_cli/src/cli_options.rscrates/biome_cli/src/commands/mod.rscrates/biome_cli/src/execute/mod.rscrates/biome_cli/src/reporter/mod.rscrates/biome_diagnostics/src/error.rs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_cli/src/cli_options.rscrates/biome_cli/src/reporter/mod.rscrates/biome_cli/src/commands/mod.rscrates/biome_diagnostics/src/error.rscrates/biome_cli/src/execute/mod.rs
crates/biome_diagnostics/**/*.rs
📄 CodeRabbit inference engine (crates/biome_diagnostics/CONTRIBUTING.md)
crates/biome_diagnostics/**/*.rs: Implement the Diagnostic trait on types, or use the #[derive(Diagnostic)] procedural macro to implement the trait. Configure category, severity, description, message, location, and tags using the #[diagnostic] attribute
Fields with #[advice] or #[verbose_advice] attributes must implement the Advices trait to record advices on the diagnostic
Use helper types from the biome_diagnostics::v2 module (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) or implement the Advices trait yourself for custom advice handling
Use #[derive(Diagnostic)] on enums when every variant contains a type that is itself a diagnostic
Ensure the type implementing Diagnostic derives Debug
Files:
crates/biome_diagnostics/src/error.rs
🧠 Learnings (20)
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Wrap rule options fields in `Option<>` to properly track set and unset options during merge
Applied to files:
crates/biome_cli/src/cli_options.rscrates/biome_cli/src/commands/mod.rscrates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Use `Box<[T]>` instead of `Vec<T>` for rule options arrays to save memory
Applied to files:
crates/biome_cli/src/cli_options.rscrates/biome_cli/src/commands/mod.rscrates/biome_diagnostics/src/error.rscrates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Implement the Diagnostic trait on types, or use the #[derive(Diagnostic)] procedural macro to implement the trait. Configure category, severity, description, message, location, and tags using the #[diagnostic] attribute
Applied to files:
crates/biome_cli/src/reporter/mod.rscrates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Ensure the type implementing Diagnostic derives Debug
Applied to files:
crates/biome_cli/src/reporter/mod.rscrates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use #[derive(Diagnostic)] on enums when every variant contains a type that is itself a diagnostic
Applied to files:
crates/biome_cli/src/reporter/mod.rscrates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper types from the biome_diagnostics::v2 module (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) or implement the Advices trait yourself for custom advice handling
Applied to files:
crates/biome_cli/src/reporter/mod.rscrates/biome_cli/src/commands/mod.rscrates/biome_diagnostics/src/error.rscrates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Fields with #[advice] or #[verbose_advice] attributes must implement the Advices trait to record advices on the diagnostic
Applied to files:
crates/biome_cli/src/reporter/mod.rscrates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/crates/biome_diagnostics_categories/src/categories.rs : Register all new diagnostic categories in crates/biome_diagnostics_categories/src/categories.rs
Applied to files:
crates/biome_cli/src/reporter/mod.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Implement `Merge` trait for rule options to support configuration inheritance
Applied to files:
crates/biome_cli/src/reporter/mod.rscrates/biome_cli/src/commands/mod.rscrates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Use `rename_all = "camelCase"` in serde derive macro for rule options
Applied to files:
crates/biome_cli/src/commands/mod.rscrates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `full_options` code block property for complete biome.json configuration snippets in documentation
Applied to files:
crates/biome_cli/src/commands/mod.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Assist rules should detect refactoring opportunities and emit code action signals
Applied to files:
crates/biome_cli/src/commands/mod.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Store type data in linear vectors instead of using recursive data structures with `Arc` for improved data locality and performance
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : No module may copy or clone data from another module in the module graph, not even behind an `Arc`
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Each invalid code example in rule documentation must emit exactly one diagnostic
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Implement the `FormattedIterExt` trait and `FormattedIter` struct in `lib.rs` to provide iterator extensions for formatting
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use the `dbg_write!` macro to debug formatter output instead of other logging methods
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/src/lib.rs : Define a type alias `<Language>Formatter<'buf>` as `Formatter<'buf, <Language>FormatContext>` in the main formatter crate
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `let else` trick when `run` function returns `Vec` to reduce code branching
Applied to files:
crates/biome_cli/src/execute/mod.rs
🧬 Code graph analysis (2)
crates/biome_cli/src/commands/mod.rs (1)
crates/biome_cli/src/execute/mod.rs (1)
cli_options(320-324)
crates/biome_cli/src/execute/mod.rs (2)
crates/biome_cli/src/reporter/junit.rs (1)
new(44-47)crates/biome_cli/src/reporter/checkstyle.rs (1)
new(35-37)
🔇 Additional comments (3)
crates/biome_cli/src/execute/mod.rs (2)
44-44: LGTM! Report mode vector changes are well-structured.The migration from single
ReportModetoVec<ReportMode>is consistently applied across initialization, mutation, and access methods. The default initialisation ensures at least one reporter is always present.Also applies to: 286-290, 303-316, 320-326, 462-469
539-550: Sensible max diagnostics logic.The conditional logic correctly lifts the diagnostic limit when only custom reporters are used, whilst respecting the user's
--max-diagnosticssetting when the default reporter is included. Nice touch.crates/biome_cli/src/cli_options.rs (1)
60-63: No changes needed – the fallback and many combinators work correctly together.The pattern
fallback(CliReporter::default()).manyis valid in bpaf 0.9.20. Themanycombinator applied to a fallback parser correctly producesVec<CliReporter>where the fallback providesvec![CliReporter::default()]when no--reporterflag is supplied, and collects multiple values into the Vec when flags are provided. This is the intended usage.
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
Thank you. Heres' a review
- The changes in the diagnostic must be reverted
- The
max_diagnosticsstill needs to be applied to the terminal reporter - The changeset needs some love
- It's a feature, so it needs to target
next - The data structure of the reporters must be updated, and accept references
Netail
force-pushed
the
feat/multi-reporters
branch
from
aed0cb4 to
25e3f43
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/biome_diagnostics/src/error.rs (1)
4-6: Documentation still describes the old thin-pointer design.The module docs describe
Box<Box<dyn Diagnostic>>for an 8-byte thin pointer, but the implementation now usesArc<dyn Diagnostic>(16 bytes). While updating the docs to explain the Arc trade-off (cloneability at the cost of pointer size) would improve clarity, this was previously raised and the maintainer chose to defer it.
🧹 Nitpick comments (3)
crates/biome_cli/src/runner/impls/finalizers/default.rs (3)
72-76: Minor style: prefer method reference over closure.Clippy will likely suggest simplifying the closure to a method reference.
🔎 Proposed fix
let report_modes: Vec<ReportMode> = cli_options .reporter .iter() - .map(|reporter| ReportMode::from(reporter)) + .map(ReportMode::from) .collect();
116-147: Consider resource cleanup on error path.If
format_filefails (line 131),close_fileis never called. Whilst the workspace likely handles cleanup implicitly, you might want to ensure the temporary file is closed even when formatting fails.
287-287: Nitpick: inconsistent braces.
Self::GitLab {}has braces whereas other unit-like variants (e.g.,Self::GitHub,Self::Junit) don't.🔎 Proposed fix
- CliReporter::GitLab => Self::GitLab {}, + CliReporter::GitLab => Self::GitLab,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/slimy-files-feel.mdcrates/biome_cli/src/cli_options.rscrates/biome_cli/src/commands/mod.rscrates/biome_cli/src/reporter/gitlab.rscrates/biome_cli/src/reporter/mod.rscrates/biome_cli/src/runner/impls/finalizers/default.rscrates/biome_diagnostics/src/error.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/biome_cli/src/reporter/mod.rs
- crates/biome_cli/src/cli_options.rs
- crates/biome_cli/src/commands/mod.rs
- .changeset/slimy-files-feel.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_cli/src/reporter/gitlab.rscrates/biome_diagnostics/src/error.rscrates/biome_cli/src/runner/impls/finalizers/default.rs
crates/biome_diagnostics/**/*.rs
📄 CodeRabbit inference engine (crates/biome_diagnostics/CONTRIBUTING.md)
crates/biome_diagnostics/**/*.rs: Implement the Diagnostic trait on types, or use the #[derive(Diagnostic)] procedural macro to implement the trait. Configure category, severity, description, message, location, and tags using the #[diagnostic] attribute
Fields with #[advice] or #[verbose_advice] attributes must implement the Advices trait to record advices on the diagnostic
Use helper types from the biome_diagnostics::v2 module (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) or implement the Advices trait yourself for custom advice handling
Use #[derive(Diagnostic)] on enums when every variant contains a type that is itself a diagnostic
Ensure the type implementing Diagnostic derives Debug
Files:
crates/biome_diagnostics/src/error.rs
🧠 Learnings (19)
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Implement the Diagnostic trait on types, or use the #[derive(Diagnostic)] procedural macro to implement the trait. Configure category, severity, description, message, location, and tags using the #[diagnostic] attribute
Applied to files:
crates/biome_cli/src/reporter/gitlab.rscrates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper types from the biome_diagnostics::v2 module (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) or implement the Advices trait yourself for custom advice handling
Applied to files:
crates/biome_cli/src/reporter/gitlab.rscrates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use #[derive(Diagnostic)] on enums when every variant contains a type that is itself a diagnostic
Applied to files:
crates/biome_cli/src/reporter/gitlab.rscrates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Fields with #[advice] or #[verbose_advice] attributes must implement the Advices trait to record advices on the diagnostic
Applied to files:
crates/biome_cli/src/reporter/gitlab.rscrates/biome_diagnostics/src/error.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Implement the `diagnostic` function to provide error messages explaining what the error is, why it is triggered, and what the user should do
Applied to files:
crates/biome_cli/src/reporter/gitlab.rscrates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:04:57.309Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:57.309Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Ensure the type implementing Diagnostic derives Debug
Applied to files:
crates/biome_cli/src/reporter/gitlab.rscrates/biome_diagnostics/src/error.rs
📚 Learning: 2025-12-31T15:34:51.717Z
Learnt from: Netail
Repo: biomejs/biome PR: 8631
File: crates/biome_cli/src/reporter/sarif.rs:106-131
Timestamp: 2025-12-31T15:34:51.717Z
Learning: In SARIF 2.1.0 reporter (crates/biome_cli/src/reporter/sarif.rs), diagnostics without location data (e.g., formatter diagnostics with empty Location) are intentionally excluded from SARIF output by returning None from to_sarif_result when to_sarif_result_location fails, rather than including them with empty or placeholder locations.
Applied to files:
crates/biome_cli/src/reporter/gitlab.rscrates/biome_cli/src/runner/impls/finalizers/default.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `rule_category!()` macro to refer to the diagnostic category instead of dynamically parsing its string name
Applied to files:
crates/biome_cli/src/reporter/gitlab.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : Store type data in linear vectors instead of using recursive data structures with `Arc` for improved data locality and performance
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Prefer using `Box<[_]>` over `Vec<_>` for signal collections to reduce memory usage
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Use `Box<[Box<str>]>` instead of `Vec<String>` for collections of strings in rule options to save memory
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Invalid code snippets in rule documentation must emit exactly one diagnostic
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : No module may copy or clone data from another module in the module graph, not even behind an `Arc`
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-12-22T09:27:13.161Z
Learnt from: ematipico
Repo: biomejs/biome PR: 8537
File: crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs:167-210
Timestamp: 2025-12-22T09:27:13.161Z
Learning: In crates/biome_analyze/**/*analyze/src/**/*.rs, the `fix_kind` field in `declare_lint_rule!` should only be specified when the rule implements the `action` function. Rules that only emit diagnostics without providing code fixes should not include `fix_kind` in their metadata.
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Implement the `FormattedIterExt` trait and `FormattedIter` struct in `lib.rs` to provide iterator extensions for formatting
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:05:27.810Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use the `dbg_write!` macro to debug formatter output instead of other logging methods
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/src/lib.rs : Define a type alias `<Language>Formatter<'buf>` as `Formatter<'buf, <Language>FormatContext>` in the main formatter crate
Applied to files:
crates/biome_diagnostics/src/error.rs
📚 Learning: 2025-12-31T15:19:03.056Z
Learnt from: Netail
Repo: biomejs/biome PR: 8631
File: crates/biome_cli/src/reporter/sarif.rs:197-218
Timestamp: 2025-12-31T15:19:03.056Z
Learning: In SARIF 2.1.0 reporter (crates/biome_cli/src/reporter/sarif.rs), when location data (source_code or span) is unavailable, omit the region field entirely using Option and #[serde(skip_serializing_if = "Option::is_none")] rather than defaulting to 0 values, as the SARIF spec states "If the region property is absent, the physicalLocation object refers to the entire artifact."
Applied to files:
crates/biome_cli/src/runner/impls/finalizers/default.rs
🧬 Code graph analysis (1)
crates/biome_cli/src/runner/impls/finalizers/default.rs (6)
crates/biome_cli/src/reporter/summary.rs (1)
from(454-456)crates/biome_cli/src/reporter/gitlab.rs (1)
new(63-68)crates/biome_cli/src/reporter/sarif.rs (1)
new(43-56)crates/biome_cli/src/reporter/checkstyle.rs (1)
new(37-39)crates/biome_cli/src/reporter/junit.rs (1)
new(46-49)crates/biome_cli/src/reporter/json.rs (1)
new(17-23)
⏰ 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: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Documentation
- GitHub Check: autofix
🔇 Additional comments (7)
crates/biome_diagnostics/src/error.rs (4)
12-12: LGTM—Arc enables efficient cloning for multi-reporter support.The move from
Box<Box<dyn Diagnostic>>toArc<dyn Diagnostic>correctly enablesClonederivation, allowing diagnostic errors to be shared efficiently across multiple reporters. Arc's atomic reference-counting is the appropriate trade-off here.Also applies to: 27-29
86-88: Correct Arc construction.The
Fromimplementation properly usesArc::new(diag)to match the new storage approach.
96-98: Correct dereference adjustment.Properly updated from
&**self.innerto&*self.innersinceArc<T>derefs directly to&T.
168-168: Size assertions correctly updated.The assertions now expect 16 bytes (u128) to match
Arc<dyn Diagnostic>'s fat pointer size, correctly reflecting the implementation change from the previous 8-byte thin pointer.Also applies to: 173-173
crates/biome_cli/src/reporter/gitlab.rs (1)
21-21: LGTM!Clean field rename to
diagnostics_payloadfor consistency with the multi-reporter pattern.Also applies to: 30-30
crates/biome_cli/src/runner/impls/finalizers/default.rs (2)
149-209: LGTM!The reporter dispatch for GitHub, GitLab, Junit, Checkstyle, RdJson, and Sarif modes is well-structured. Each mode correctly clones
diagnostics_payloadand constructs the appropriate reporter/visitor pair.
78-101: No action needed—TraversalSummaryalready derivesCopy, so using it by value across multiple reporters works fine.Likely an incorrect or invalid review comment.
Netail
force-pushed
the
feat/multi-reporters
branch
from
25e3f43 to
7414155
Compare
ematipico
force-pushed
the
feat/multi-reporters
branch
from
7414155 to
e426ae4
Compare
|
@Netail I updated the PR. I reverted the code inside the diagnostics, and the diagnostics are now passed as a reference. To fix the JSON reporter, I created a new type, because we can't use our serde diagnostics. As in turn, we broke our json reporter because we return less information, but this is fine because our JSON reporter was marked as experimental in our documentation. |
Oh sweet, thanks! A bit of procrastination on this one 😅 |
ematipico
force-pushed
the
feat/multi-reporters
branch
from
0c06790 to
953962c
Compare
Thought one of the examples of bpaf showed a non-positional argument example. That you could have multiple values for a single flag/option. In our case the type + output location (e.g. But I think you've implemented that, correct? Then only some documentation for the website missing I guess |
github-actions
bot
added
A-Parser
Merging this PR will not alter performance
Comparing 
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 2
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/json.rs (1)
215-220:⚠️ Potential issue | 🟡 MinorRemove the unused
Displayimplementation.The
Displayimpl forJsonReporterVisitor(lines 215–220) is never invoked. The only serialisation path actually used isto_json(), called directly in the finalizer. IfDisplayisn't needed, remove it; otherwise, switch to using it consistently rather than maintaining two separate serialisation approaches.
🤖 Fix all issues with AI agents
In `@crates/biome_cli/src/reporter/json.rs`:
- Around line 415-416: The two calls using
expect—source.location(span.start()).expect("Invalid span") and
source.location(span.end()).expect("Invalid span")—can panic on bad span data;
change them to handle missing locations gracefully (e.g., use .ok()? or early
return/continue) so the function returns None or skips the entry instead of
panicking, mirroring the defensive pattern used by to_location; update any
downstream code that assumes start/end to handle the Option values accordingly.
In `@crates/biome_cli/src/runner/impls/finalizers/default.rs`:
- Line 140: Remove the leftover debug statement dbg!(&root) in default.rs (the
one printing the local variable root); delete that call so the CLI no longer
emits debug output to stderr during normal runs, or if you need retained debug
visibility replace it with an appropriate logging macro (e.g., trace! or debug!)
from the project's logging facility instead of dbg!.
🧹 Nitpick comments (1)
crates/biome_cli/src/runner/impls/finalizers/default.rs (1)
330-378: Remove commented-out code.The old
ReportModeenum and its implementations are no longer needed. Commented-out code should be removed to keep the codebase tidy.
ematipico
force-pushed
the
feat/multi-reporters
branch
from
4ff184b to
9772d7c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/biome_cli/src/reporter/json.rs`:
- Around line 398-401: The code in record_log currently calls
self.suggestions.last_mut().expect("No suggestions to append to"), which can
panic when suggestions is empty while last_diagnostic_length !=
current_diagnostic_length; change this to handle the None case defensively by
using if let Some(last) = self.suggestions.last_mut() { ... } else { create or
push a new suggestion entry (or skip appending) and then proceed } so appending
logic for last_suggestion only runs when last exists and you won't panic; update
any related logic that assumes a suggestion was present (variables
last_diagnostic_length, current_diagnostic_length) accordingly.
🧹 Nitpick comments (3)
crates/biome_cli/src/reporter/mod.rs (2)
56-134: Consider extracting the JSON member creation pattern.The
json_member()method is quite verbose with repetitive boilerplate for each field. This works correctly, but you might consider a small helper macro or function to reduce the visual noise. That said, it's explicit and easy to follow.♻️ Example helper to reduce repetition
fn make_json_number_member(key: &str, value: impl ToString) -> JsonMember { json_member( AnyJsonMemberName::JsonMemberName(json_member_name(json_string_literal(key))), token(T![:]), AnyJsonValue::JsonNumberValue(json_number_value(json_number_literal(value))), ) }Then usage becomes:
let members = vec![ make_json_number_member("changed", self.changed), make_json_number_member("unchanged", self.unchanged), // ... ];
178-183: Public trait missing rustdoc.
ReporterWriteris a public trait but lacks documentation. A brief rustdoc comment explaining its purpose and the expected behaviour of each method would help future contributors.📝 Suggested documentation
+/// Abstraction for writing reporter output to different destinations. +/// +/// Implementations handle console output ([`ConsoleReporterWriter`]) or +/// file buffering ([`FileReporterWriter`]) for multi-reporter support. pub trait ReporterWriter { + /// Writes a log-level message. fn log(&mut self, message: Markup); + /// Writes an error-level message. fn error(&mut self, message: Markup); + /// Dumps the buffered content, if any. Returns `None` for unbuffered writers. fn dump(&mut self) -> Option<String>; + /// Clears any buffered content. fn clear(&mut self); }crates/biome_cli/src/runner/impls/finalizers/default.rs (1)
327-375: Remove commented-out dead code.The old
ReportModeenum and its implementations are fully commented out. If they're no longer needed, delete them rather than leaving them as comments. Version control preserves history if you ever need to reference them.
| let last_suggestion = self | ||
| .suggestions | ||
| .last_mut() | ||
| .expect("No suggestions to append to"); |
There was a problem hiding this comment.
Another potential panic in record_log.
The .expect("No suggestions to append to") at line 401 will panic if suggestions is empty when last_diagnostic_length != current_diagnostic_length. Consider using if let Some(last) = self.suggestions.last_mut() instead.
🛡️ Proposed defensive handling
if self.last_diagnostic_length != current_diagnostic_length {
- let last_suggestion = self
- .suggestions
- .last_mut()
- .expect("No suggestions to append to");
- last_suggestion.text = message;
+ if let Some(last_suggestion) = self.suggestions.last_mut() {
+ last_suggestion.text = message;
+ }
} else if let Some(current_message) = self.current_message.as_mut() {📝 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.
| let last_suggestion = self | |
| .suggestions | |
| .last_mut() | |
| .expect("No suggestions to append to"); | |
| if self.last_diagnostic_length != current_diagnostic_length { | |
| if let Some(last_suggestion) = self.suggestions.last_mut() { | |
| last_suggestion.text = message; | |
| } | |
| } else if let Some(current_message) = self.current_message.as_mut() { |
🤖 Prompt for AI Agents
In `@crates/biome_cli/src/reporter/json.rs` around lines 398 - 401, The code in
record_log currently calls self.suggestions.last_mut().expect("No suggestions to
append to"), which can panic when suggestions is empty while
last_diagnostic_length != current_diagnostic_length; change this to handle the
None case defensively by using if let Some(last) = self.suggestions.last_mut() {
... } else { create or push a new suggestion entry (or skip appending) and then
proceed } so appending logic for last_suggestion only runs when last exists and
you won't panic; update any related logic that assumes a suggestion was present
(variables last_diagnostic_length, current_diagnostic_length) accordingly.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
I am going to merge this one. If you have any feedback, please let me know |
2 participants
Summary
Support the configuration of multiple reporters, e.g. github & summary
Related #4761
Test Plan
Docs