Conversation
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Naoki Ikeguchi <me@s6n.jp> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: arendjr <533294+arendjr@users.noreply.github.com>
Co-authored-by: Hamir Mahal <hirehamir@gmail.com>
🦋 Changeset detectedLatest commit: 4804acf 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 |
github-actions
bot
added
A-CLI
WalkthroughThis PR introduces multiple enhancements across Biome: new HTML accessibility lint rules (noAccessKey, useButtonType, useHtmlLang), a TypeScript interface member sorting assist, JSX factory configuration support from tsconfig.json, improvements to the useHookAtTopLevel rule for detecting module-level hook usage, custom extension mappings for useImportExtensions, configurable file watcher options, standardised logging configuration via LogOptions, LSP project-scanning progress reporting with UUID tokens, and a complete new YAML formatter crate. Additionally, KeyValuePair's API shifts from tuple-like construction to an explicit Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 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: 3
🧹 Nitpick comments (12)
crates/biome_yaml_formatter/src/verbatim.rs (1)
97-106: Consider using.find()instead of.take_while().next().The current chain achieves the same result as
.find(), but the latter expresses intent more directly.let start_source = self .node .first_leading_trivia() .into_iter() .flat_map(|trivia| trivia.pieces()) .filter(|trivia| trivia.is_skipped()) .map(|trivia| source_range(f, trivia.text_range()).start()) - .take_while(|start| *start < trimmed_source_range.start()) - .next() + .find(|start| *start < trimmed_source_range.start()) .unwrap_or_else(|| trimmed_source_range.start());.changeset/witty-ears-hammer.md (1)
1-22: Good, rule-focused changeset with examplesNice user-facing summary: past tense for the addition, present tense for behaviour, plus clear invalid/valid examples. If there’s a public rule doc URL handy, you could optionally link it here for quicker discoverability, but it’s not required.
crates/biome_cli/tests/snap_test.rs (1)
271-303: Be careful:trim()here may eat newlines/indentationUsing
before.trim()normalises all surrounding whitespace, including leading newlines/indentation, then forces a single space before<BIOME_DIR>. That can join the placeholder onto the previous line rather than keeping it on its own/indented line.If the goal is just to avoid duplicated spaces before the placeholder while preserving line breaks and leading indentation, consider something like:
- result.push_str(before.trim()); - result.push(' '); + result.push_str(before.trim_end()); + if !before.ends_with(char::is_whitespace) { + result.push(' '); + }Or a similar
trim_end‑based approach, depending on exactly how you want the snapshots to look.crates/biome_flags/src/lib.rs (1)
104-223: Consider extracting the repetitive Display logic.The match-based formatting for each env variable is quite repetitive. A helper method on
BiomeEnvVariablecould reduce boilerplate:impl BiomeEnvVariable { fn fmt_with_padding(&self, fmt: &mut Formatter, padding: usize) -> std::io::Result<()> { match self.value() { None => KeyValuePair::new(self.name, markup! { <Dim>"unset"</Dim> }) .with_padding(padding) .fmt(fmt), Some(value) => KeyValuePair::new(self.name, markup! {{DebugDisplay(value)}}) .with_padding(padding) .fmt(fmt), } } }Then the Display impl becomes a series of
self.biome_*.fmt_with_padding(fmt, padding)?;calls..changeset/six-plants-lie.md (1)
9-17: Minor: Trailing comma in example.The JSON example has a trailing comma on line 13 that makes it invalid JSON. Consider removing it for clarity, though users will understand the intent.
Apply this diff:
"extensionMappings": { - "ts": "js", + "ts": "js" }.changeset/every-beers-sleep.md (1)
1-24: Minor wording polish for watcher option descriptionThe description is clear, but you could make it read a touch more naturally with tiny edits like:
- The option accepts the current values: + The option accepts the following values: @@ -- `none`: it doesn't enable the watcher. When the watcher is disabled, changes to files aren't recorded anymore by Biome. This might have - repercussions on some lint rules that might rely on updated types or updated paths. +- `none`: it doesn't enable the watcher. When the watcher is disabled, changes to files are no longer recorded by Biome. This might have + repercussions for some lint rules that rely on updated types or paths.Purely optional, the existing text is already understandable.
crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs (1)
80-105:extensionMappingsbehaviour matches docs; consider clarifying precedence withforceJsExtensionsThe new
extensionMappingsoption and its use inget_extensionless_importlook sound: when a mapping exists for the resolved extension, that mapped extension is suggested; otherwise you fall back to the resolved extension, preserving the existing.d.tsand sub‑extension handling.Note that
forceJsExtensionsis evaluated first, so if bothforceJsExtensions: trueandextensionMappingsare set,forceJsExtensionsalways wins and mappings are effectively ignored. If you intend mappings to be the more specific override, it might be worth either swapping the order or explicitly documenting thatforceJsExtensionstakes precedence.Also applies to: 286-295
crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/hookLocations.js (1)
1-48: Nice coverage of hook locations across contextsThis fixture exercises a good spread of cases for
useHookAtTopLevel(module scope, non‑components, components/hooks by naming convention, object/class methods, and varioustestcallback shapes includingrenderHook). The mixed valid/invalid examples in a single file are fine here; just be aware it diverges from thevalid/invalidfile‑prefix convention if you ever rely on that mechanically.crates/biome_cli/src/logging.rs (1)
1-3: Avoid drifting defaults betweenbpaffallbacks andLogOptions::default
LogOptionsnicely centralises logging configuration, but the default values are now encoded twice: once via the#[bpaf(fallback(...))]attributes and again in the manualDefaultimpl. If one side changes (e.g. log path or prefix) it’s easy for the two to diverge.Consider factoring the defaults into associated constants or a
fn default_log_options() -> LogOptionsused both by bpaf (viafallback) and by theDefaultimpl, so there’s a single source of truth.Also applies to: 15-83
crates/biome_js_analyze/src/lint/style/use_import_type.rs (1)
1-4: JSX factory detection inuseImportTypecorrectly avoids flagging runtime-only importsThe new
is_jsx_factory_bindinghelper and its use at the various import sites mean:
Under
JsxRuntime::ReactClassic, default/namespace bindings that are either:
- the standard React global, or
- match configured
jsxFactory/jsxFragmentFactoryare treated as value imports and exempt from the “only used as a type” checks.
That should eliminate spurious
import typesuggestions for React or custom JSX factory imports that are only referenced via JSX. The helper being gated onReactClassicalso matches how custom factories are used today; if you later support custom factories under other runtimes, this will be the natural place to extend.Also applies to: 195-201, 251-268, 277-282, 343-349, 1100-1129
crates/biome_html_analyze/src/lint/a11y/use_button_type.rs (1)
111-115: Minor inconsistency with sibling rule helper.The
is_button_elementhelper usesname.text()whilstis_html_elementinuse_html_lang.rsusestoken_text. Both operate onAnyHtmlElement::name(). Consider aligning the approach for consistency across the a11y rules.fn is_button_element(element: &AnyHtmlElement) -> bool { element .name() - .is_some_and(|name| name.text().eq_ignore_ascii_case("button")) + .is_some_and(|token_text| token_text.eq_ignore_ascii_case("button")) }crates/biome_cli/src/service/unix.rs (1)
56-84: LGTM! Clean consolidation of daemon spawn arguments.The refactor from individual parameters to structured
WatcherOptionsandLogOptionsimproves maintainability. The command-line argument construction properly forwards all relevant configuration.One minor observation: the parameter is named
watcher_configurationhere (line 58) butwatcher_optionsin the Windows counterpart (crates/biome_cli/src/service/windows.rs, line 190). Consider aligning the naming for consistency across platforms.fn spawn_daemon( stop_on_disconnect: bool, - watcher_configuration: WatcherOptions, + watcher_options: WatcherOptions, log_options: LogOptions, ) -> io::Result<Child> {
| "@biomejs/biome": minor | ||
| --- | ||
|
|
||
| Added support for `jsxFactory` and `jsxFragmentFactory`.Biome now respects `jsxFactory` and `jsxFragmentFactory` settings from `tsconfig.json` when using the classic JSX runtime, preventing false positive [noUnusedImports](https://biomejs.dev/linter/rules/no-unused-imports/) errors for custom JSX libraries like Preact. |
There was a problem hiding this comment.
Fix spacing in description text.
Line 5 has a missing space after the closing backtick. Should be:
-Added support for `jsxFactory` and `jsxFragmentFactory`.Biome now respects...
+Added support for `jsxFactory` and `jsxFragmentFactory`. Biome now respects...Otherwise, the changeset is well-structured with good examples and a helpful rule link. Nice work! ✓
📝 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.
| Added support for `jsxFactory` and `jsxFragmentFactory`.Biome now respects `jsxFactory` and `jsxFragmentFactory` settings from `tsconfig.json` when using the classic JSX runtime, preventing false positive [noUnusedImports](https://biomejs.dev/linter/rules/no-unused-imports/) errors for custom JSX libraries like Preact. | |
| Added support for `jsxFactory` and `jsxFragmentFactory`. Biome now respects `jsxFactory` and `jsxFragmentFactory` settings from `tsconfig.json` when using the classic JSX runtime, preventing false positive [noUnusedImports](https://biomejs.dev/linter/rules/no-unused-imports/) errors for custom JSX libraries like Preact. |
🤖 Prompt for AI Agents
In .changeset/eight-eels-pull.md around line 5, the sentence "Added support for
`jsxFactory` and `jsxFragmentFactory`.Biome now respects..." is missing a space
after the closing backtick; edit that line to insert a space after the backtick
so it reads "...`jsxFragmentFactory`. Biome now respects..." to fix the spacing.
| fn source_map(&self) -> Option<&TransformSourceMap> { | ||
| None | ||
| } |
There was a problem hiding this comment.
Bug: source_map() always returns None despite the field being set.
The with_source_map method on line 32 sets self.source_map, but this accessor ignores it and always returns None. This breaks source map functionality used in verbatim.rs (lines 60-63, 108-116).
fn source_map(&self) -> Option<&TransformSourceMap> {
- None
+ self.source_map.as_ref()
}📝 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.
| fn source_map(&self) -> Option<&TransformSourceMap> { | |
| None | |
| } | |
| fn source_map(&self) -> Option<&TransformSourceMap> { | |
| self.source_map.as_ref() | |
| } |
🤖 Prompt for AI Agents
In crates/biome_yaml_formatter/src/context.rs around lines 45-47, the
source_map() accessor currently always returns None even though with_source_map
sets self.source_map; change the method to return a reference to the stored
Option by returning self.source_map.as_ref() (or Some(&self.source_map) if the
field is not an Option) so callers receive the actual TransformSourceMap; ensure
the function signature remains fn source_map(&self) ->
Option<&TransformSourceMap> and compile.
| /// Rule for formatting an bogus nodes. | ||
| pub(crate) trait FormatBogusNodeRule<N> | ||
| where | ||
| N: AstNode<Language = YamlLanguage>, | ||
| { | ||
| fn fmt(&self, node: &N, f: &mut YamlFormatter) -> FormatResult<()> { | ||
| format_bogus_node(node.syntax()).fmt(f) | ||
| } | ||
| } |
There was a problem hiding this comment.
Minor typo in doc comment.
"an bogus nodes" → "a bogus node" or "bogus nodes".
-/// Rule for formatting an bogus nodes.
+/// Rule for formatting bogus nodes.
pub(crate) trait FormatBogusNodeRule<N>📝 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.
| /// Rule for formatting an bogus nodes. | |
| pub(crate) trait FormatBogusNodeRule<N> | |
| where | |
| N: AstNode<Language = YamlLanguage>, | |
| { | |
| fn fmt(&self, node: &N, f: &mut YamlFormatter) -> FormatResult<()> { | |
| format_bogus_node(node.syntax()).fmt(f) | |
| } | |
| } | |
| /// Rule for formatting bogus nodes. | |
| pub(crate) trait FormatBogusNodeRule<N> | |
| where | |
| N: AstNode<Language = YamlLanguage>, | |
| { | |
| fn fmt(&self, node: &N, f: &mut YamlFormatter) -> FormatResult<()> { | |
| format_bogus_node(node.syntax()).fmt(f) | |
| } | |
| } |
🤖 Prompt for AI Agents
In crates/biome_yaml_formatter/src/lib.rs around lines 234 to 242, the doc
comment for the trait reads "Rule for formatting an bogus nodes" which contains
a typo and grammatical error; update the comment to use correct article and
number, e.g. "Rule for formatting a bogus node" or "Rule for formatting bogus
nodes" to match intended meaning and keep it consistent with surrounding docs.
commit: |
Merging this PR will degrade performance by 6.06%
Performance Changes
Comparing 
Footnotes
|
Parser conformance results onjs/262
jsx/babel
symbols/microsoft
ts/babel
ts/microsoft
|
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…8908) Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Carson McManus <dyc3@users.noreply.github.com>
github-actions
bot
removed
A-Tooling
…rule diagnostics (#9060)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: dyc3 <1808807+dyc3@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Victorien Elvinger <victorien@elvinger.fr> Co-authored-by: dyc3 <1808807+dyc3@users.noreply.github.com> Co-authored-by: Conaclos <2358560+Conaclos@users.noreply.github.com> Co-authored-by: chansuke <501052+chansuke@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In @.changeset/add-e18e-rule-source.md:
- Line 5: Update the six rule mentions to include their documentation links: for
each rule name useAtIndex, useExponentiationOperator, noPrototypeBuiltins,
useDateNow, useSpread, and useObjectSpread, append or replace the plain text
with a Markdown link to the official e18e/website docs (e.g.
[useAtIndex](https://.../rules/use-at-index) style). Ensure each rule reference
points to its corresponding rule page on the public docs site so all six entries
become clickable documentation links.
In @.changeset/angry-women-accept.md:
- Around line 1-3: Update the changeset front matter to use "patch" instead of
"minor" for the "@biomejs/biome" entry so it aligns with main-branch rules;
locate the YAML front matter block in the changeset (the top lines showing
"@biomejs/biome": minor) and change the value from minor to patch, keeping the
rest of the front matter intact.
In @.changeset/odd-flies-nail.md:
- Around line 1-3: The changeset currently declares a "minor" release but the PR
targets main; update the changeset declaration in odd-flies-nail.md to "patch"
if this is a non-breaking fix for main, or explicitly confirm and document that
a "minor" release should be carried (i.e., this is a feature intended for next)
so the release type matches the branch intent; ensure the first line in the
changeset front-matter is adjusted accordingly.
In @.changeset/small-seas-laugh.md:
- Around line 1-2: Update the changeset entry that currently reads
`"@biomejs/biome": minor` to use `"patch"` since this PR targets main; locate
the .changeset file containing the changeset header and change the version bump
type from minor to patch so it conforms to the branch-specific changeset
guidance.
In @.changeset/wide-masks-mix.md:
- Around line 1-5: The changeset currently labels the release as a minor change
and uses a generic description; change the frontmatter tag from "minor" to
"patch" and update the description line to begin with the required bug-fix
format "Fixed `#NUMBER`(...): ..." (replace `#NUMBER` with the actual issue number
and include the issue link if available), ensuring both the frontmatter and the
human-readable description at the top of .changeset/wide-masks-mix.md are
updated to reflect a patch bugfix.
In `@crates/biome_analyze/src/diagnostics.rs`:
- Around line 102-136: OffsetVisitor currently only overrides record_frame,
causing other Visit methods to be dropped when applying an offset; update
OffsetVisitor (used in advices on DiagnosticKind::Raw) to implement and forward
all Visit trait methods (record_log, record_list, record_diff, record_backtrace,
record_command, record_group, record_table) to self.inner unchanged while
keeping record_frame logic that adjusts the span, so all non-frame advices are
preserved and only frame spans are offset.
In `@crates/biome_analyze/src/rule.rs`:
- Around line 287-290: The Ord/PartialOrd mapping assigns duplicate index 37 to
Self::HtmlEslint and Self::EslintE18e causing incorrect comparisons; update the
index values so each variant uses a unique integer (e.g., bump one of the
duplicates to the next unused index) in the match that feeds the ordering
implementation (the match arms for HtmlEslint, EslintE18e,
EslintBetterTailwindcss), then run tests to verify deterministic ordering of
rules.
In `@crates/biome_cli/src/commands/mod.rs`:
- Around line 689-709: The match in the log_options() function incorrectly
returns None for daemon-related variants causing Start, LspProxy, and RunServer
log options to be ignored; update the match arm to return Some(log_options) for
those variants (e.g., include Self::Start { log_options, .. } | Self::LspProxy {
log_options, .. } | Self::RunServer { log_options, .. } in the Some(...) arm) so
that log_options(), and downstream helpers like log_level() / log_kind(),
receive the user's logging preferences.
In `@crates/biome_cli/src/runner/impls/collectors/default.rs`:
- Around line 262-265: The is_error logic treats any format run as an error even
when write access is allowed; change the grouping so CI always errors but
format/check only error when write access is not required: compute is_error as
execution.is_ci() || ((execution.is_format() || execution.is_check()) &&
!execution.requires_write_access()); update the is_error assignment (and its
parentheses) to use this grouping so execution.is_format(),
execution.is_check(), and execution.requires_write_access() interact correctly.
- Around line 173-183: In the Message::Error arm (in the collector
implementation in default.rs) you only increment warnings/infos, so Error and
Fatal diagnostics never increment the error counter; update the branch to check
err.severity() for Severity::Error and Severity::Fatal and call
self.errors.fetch_add(1, Ordering::Relaxed) for those cases (after the
should_skip_diagnostic check), leaving the existing increments for Warning and
Information in place.
In `@crates/biome_cli/src/runner/impls/finalizers/default.rs`:
- Around line 111-143: The JSON branch currently always writes to
console_reporter_writer so file destinations are ignored; update the
CliReporterKind::Json / JsonPretty handling to detect
cli_reporter.is_file_report() and use file_reporter_writer (instead of
console_reporter_writer) when true: construct the JsonReporter and
JsonReporterVisitor as now, call reporter.write(...) with the chosen writer and
buffer, obtain root from buffer.to_json(), then either format/print or to_string
and call file_reporter_writer.log(...) (and ensure file_reporter_writer.dump()
is invoked where needed) so both pretty and non‑pretty JSON end up in the file
output rather than only the console.
In `@crates/biome_cli/src/runner/impls/process_file/lint_and_assist.rs`:
- Around line 183-221: open_file is called on workspace but early returns for
ignored or protected files (branches using file_features.is_ignored() and
file_features.is_protected()) never call close_file, leaving documents open;
before each return in those branches, call workspace.close_file(CloseFileParams
{ project_key, path: biome_path.clone() }) (or the crate's equivalent close_file
API) to ensure the file is closed, and do this both for the ignored branch and
for the protected branch right after logging/console operations so the
open/close pairing is preserved even on early exits.
- Around line 274-306: The code currently returns
StdinDiagnostic::new_not_formatted() whenever new_content is borrowed and
execution.requires_write_access() is false, which wrongfully errors on stdin
even when formatting would produce identical output; change the logic so we only
return the NotFormatted error if the formatter (workspace.format_file / the
computed output) would have produced a different string than original_content.
Concretely: after computing output (the formatted string) and before possibly
replacing new_content, record whether formatting_changed = output !=
original_content (or compare the formatter result or transformed code that would
be written), and in the match arm for Cow::Borrowed(original_content) only
return Err(StdinDiagnostic::new_not_formatted().into()) when
!execution.requires_write_access() && formatting_changed (or other
transformations would have changed), otherwise proceed without error. Ensure you
reference the existing variables execution, output, new_content,
original_content, workspace.format_file and StdinDiagnostic::new_not_formatted()
when making the change.
- Around line 45-62: The code always constructs Some(suppression_explanation)
and passes it to workspace_file.guard().fix_file(...), which causes suppression
metadata to be applied even when execution.suppress() is false; change the logic
to build an Option<String> that is None when suppression is not enabled and
Some(reason) only when execution.suppress() is true (use
execution.suppression_reason().map(|s| s.to_string()).or_else(|| Some("ignored
using `--suppress`".to_string())) when suppressed), then pass that Option
directly to fix_file instead of wrapping an unconditional Some(...).
In `@crates/biome_cli/src/runner/process_file.rs`:
- Around line 235-270: The code reads the file into `input` once, then in the
`else` branch re-reads `file.read_to_string(&mut input)` which runs at EOF and
yields empty content; remove the redundant second read and reuse the earlier
`input` when calling `ctx.workspace().open_file` (keep `OpenOptions`, `file`,
`input`, `FileGuard`, `ctx.workspace().file_exists`, `OpenFileParams`, and
`FileContent::from_client` as-is), ensuring the content passed to `open_file` is
the initially read `input`.
🟡 Minor comments (22)
.changeset/fast-months-move.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorFix the typo in the user-facing description.
There’s a duplicated preposition (“in inside”) in the sentence. Consider:
✍️ Suggested edit
-Fixed [`#901`](https://github.com/biomejs/biome-vscode/issues/901). Biome now allows trailing commas in inside Zed `settings.json` and VSCode `settings.json`. +Fixed [`#901`](https://github.com/biomejs/biome-vscode/issues/901). Biome now allows trailing commas inside Zed `settings.json` and VS Code `settings.json`..changeset/nine-ends-wink.md-1-5 (1)
1-5:⚠️ Potential issue | 🟡 MinorAlign changeset type with target branch.
This changeset is marked minor, but the PR targets
main. Guidelines saymainshould use patch for non‑breaking changes. If this is meant formain, please switch topatch; otherwise, confirm it’s intended fornextonly.
As per coding guidelines “Use 'patch' for bug fixes and non-breaking changes targeting the main branch, 'minor' for new features targeting the next branch, and 'major' for breaking changes targeting the next branch.”.changeset/rich-flowers-hunt.md-1-3 (1)
1-3:⚠️ Potential issue | 🟡 MinorUse a patch changeset on main.
This PR targets
main, so non‑breaking user‑facing changes should bepatch, notminor. Please update the front matter accordingly. As per coding guidelines, "Use 'patch' for bug fixes and non-breaking changes targeting the main branch, 'minor' for new features targeting the next branch, and 'major' for breaking changes targeting the next branch.".changeset/eight-bars-unite.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorTiny wording tweak: “HTML language” → “HTML”.
Saves a word and reads cleaner.
.changeset/eight-bars-unite.md-2-2 (1)
2-2:⚠️ Potential issue | 🟡 MinorConfirm the changeset type matches the target branch policy.
This PR targets
main, but the changeset is markedminor. If this merge PR is expected to carry over “next” changesets, all good; otherwise this should likely bepatchfor main. As per coding guidelines: “Use 'patch' for bug fixes and non-breaking changes targeting the main branch, 'minor' for new features targeting the next branch, and 'major' for breaking changes targeting the next branch.”.changeset/odd-flies-nail.md-7-7 (1)
7-7:⚠️ Potential issue | 🟡 MinorTighten the sentence by dropping the comma before “because”.
Minor grammar tweak for readability.
✂️ Suggested edit
-If your codebase has only `*.module.css` files, you can remove the parser feature as follows, because now Biome does it for you: +If your codebase has only `*.module.css` files, you can remove the parser feature as follows because Biome now does it for you:.changeset/angry-women-accept.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorTiny grammar fix (“will a emit”).
Quick tweak for readability in the user-facing text.
✍️ Suggested edit
-... Otherwise, Biome will a emit a parse error. +... Otherwise, Biome will emit a parse error..changeset/group-by-nesting-feature.md-44-44 (1)
44-44:⚠️ Potential issue | 🟡 MinorFix formatting: missing space before opening brace.
The example has a spacing inconsistency.
🔧 Proposed fix
-const object ={ +const object = {.changeset/ready-taxes-build.md-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorFix issue number mismatch in link.
The text references
#6625but the link points to#6623. Please correct the link or the issue number.🔗 Proposed fix
-This overhaul fixes several issues ([`#5150`](https://github.com/biomejs/biome/issues/5150), [`#6625`](https://github.com/biomejs/biome/issues/6623), [`#8437`](https://github.com/biomejs/biome/issues/8437)) around whitespace sensitivity that were causing incorrect formatting in certain scenarios that were difficult or impossible to fully address before. +This overhaul fixes several issues ([`#5150`](https://github.com/biomejs/biome/issues/5150), [`#6625`](https://github.com/biomejs/biome/issues/6625), [`#8437`](https://github.com/biomejs/biome/issues/8437)) around whitespace sensitivity that were causing incorrect formatting in certain scenarios that were difficult or impossible to fully address before..changeset/proud-ends-hear.md-11-21 (1)
11-21:⚠️ Potential issue | 🟡 MinorRefine the code example to avoid redeclaration errors.
The example declares
const athree times, which would produce syntax errors in JavaScript. Whilst the point is to demonstrate unformatted code preservation, a realistic example would be clearer and more helpful.💡 Suggested improvement
```js // biome-ignore-all format: generated -const a = [ ] +const a = [ ] -const a = [ ] +const b = { } -const a = [ ] +const c = ( )=> {}This demonstrates the feature with valid syntax whilst showing various unformatted constructs. </details> </blockquote></details> <details> <summary>crates/biome_cli/tests/cases/handle_svelte_files.rs-686-709 (1)</summary><blockquote> `686-709`: _⚠️ Potential issue_ | _🟡 Minor_ **Missing `biome.json` configuration.** This test lacks the `biome.json` setup that all other new tests include and that the Vue equivalent (`use_import_type_not_triggered_for_enum_in_template` in `handle_vue_files.rs`) has. Without enabling `experimentalFullSupportEnabled`, the HTML linter behaviour may differ from what's being tested. <details> <summary>🔧 Proposed fix</summary> ```diff fn use_import_type_not_triggered_for_enum_in_template() { let fs = MemoryFileSystem::default(); let mut console = BufferConsole::default(); + fs.insert( + "biome.json".into(), + r#"{ "html": { "linter": {"enabled": true}, "experimentalFullSupportEnabled": true } }"# + .as_bytes(), + ); + let file = Utf8Path::new("file.svelte"); fs.insert(file.into(), SVELTE_ENUM_IN_TEMPLATE.as_bytes());.changeset/silly-walls-build.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorLine 5: Change
useImportTypestouseImportType.Lint rule names follow the
use<Concept>pattern for mandated concepts, so it'suseImportType(singular), matching line 24.crates/biome_cli/src/runner/finalizer.rs-14-16 (1)
14-16:⚠️ Potential issue | 🟡 MinorMinor typo: "finalise" should be "finalize" for consistency.
The codebase uses American English spelling ("finalize") throughout. Line 15 uses "finalise".
📝 Suggested fix
/// Optional hook to run before finalization. Useful for commands that need - /// to work with the Workspace before finally finalise the command. + /// to work with the Workspace before finally finalizing the command.crates/biome_cli/src/runner/impls/process_file/format.rs-55-71 (1)
55-71:⚠️ Potential issue | 🟡 MinorAdjust diagnostics for embedded-file offsets.
For astro/vue/svelte, the reported ranges can be off without the handler offset, which makes errors point at the wrong place.Proposed fix
-use biome_diagnostics::{Diagnostic, DiagnosticExt, Error, PrintDiagnostic, Severity, category}; +use biome_diagnostics::{Diagnostic, DiagnosticExt, Error, PrintDiagnostic, Severity, category}; +use biome_rowan::TextSize; @@ - ctx.push_message(Message::Diagnostics { + let offset = if features_supported.supports_full_html_support() { + None + } else { + match workspace_file.as_extension() { + Some("vue") => VueFileHandler::start(input.as_str()), + Some("astro") => AstroFileHandler::start(input.as_str()), + Some("svelte") => SvelteFileHandler::start(input.as_str()), + _ => None, + } + }; + + ctx.push_message(Message::Diagnostics { file_path: workspace_file.path.to_string(), content: input.clone(), diagnostics: diagnostics_result .diagnostics .into_iter() + .map(|diagnostic| { + if let Some(offset) = offset { + diagnostic.with_offset(TextSize::from(offset)) + } else { + diagnostic + } + }) // Formatting is usually blocked by errors, so we want to print only diagnostics that // Have error severity .filter_map(|diagnostic| { if diagnostic.severity() >= Severity::Error { Some(Error::from(diagnostic)) } else { None } }) .collect(), skipped_diagnostics: diagnostics_result.skipped_diagnostics as u32, });.changeset/tangy-dingos-design.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorMinor grammatical tweak needed.
The sentence reads "how Biome will attempt the configuration file is" which is missing a verb. Consider:
-Added the ability to load the hidden files `.biome.json` and `.biome.jsonc`. This is the order how Biome will attempt the configuration file is: +Added the ability to load the hidden files `.biome.json` and `.biome.jsonc`. This is the order in which Biome will attempt to load the configuration file:crates/biome_cli/tests/cases/reporter_sarif.rs-172-188 (1)
172-188:⚠️ Potential issue | 🟡 MinorFunction name and command mismatch.
The function is named
reports_diagnostics_sarif_check_command_filebut actually runs theformatcommand (line 188). Either rename the function toreports_diagnostics_sarif_format_command_fileor change the command to"check".Option 1: Fix the command to match the name
[ - "format", + "check", "--reporter=sarif", "--reporter-file=file.json",crates/biome_cli/tests/cases/reporter_terminal.rs-141-142 (1)
141-142:⚠️ Potential issue | 🟡 MinorFunction name may be misleading.
The function is named
reports_diagnostics_check_write_command_filebut doesn't include the--writeflag (unlikereports_diagnostics_check_write_command_verboseabove which does). Consider renaming toreports_diagnostics_check_command_filefor clarity..changeset/all-pumas-stop.md-31-31 (1)
31-31:⚠️ Potential issue | 🟡 MinorTidy the final sentence grammar.
A separator reads better here.💬 Suggested edit
-**The `--reporter` and `--reporter-file` flags must appear next to each other, otherwise an error is thrown.** +**The `--reporter` and `--reporter-file` flags must appear next to each other; otherwise, an error is thrown.**crates/biome_cli/src/commands/migrate.rs-142-148 (1)
142-148:⚠️ Potential issue | 🟡 MinorAvoid a panic if the config path is missing.
unwrap()here can crash if call ordering ever changes. A defensive error keeps the CLI stable.Suggested fix
- let payload = crate::execute::migrate::MigratePayload { + let configuration_file_path = self.configuration_file_path.clone().ok_or_else(|| { + CliDiagnostic::MigrateError(MigrationDiagnostic { + reason: "Biome couldn't find the Biome configuration file.".to_string(), + }) + })?; + let payload = crate::execute::migrate::MigratePayload { session, project_key, write: self.should_write(), - // SAFETY: checked during get_execution - configuration_file_path: self.configuration_file_path.clone().unwrap(), + configuration_file_path, sub_command: self.sub_command.clone(), nested_configuration_files: configuration_files, };crates/biome_cli/src/runner/handler.rs-142-156 (1)
142-156:⚠️ Potential issue | 🟡 MinorCount panics so summaries don’t under-report.
On panic you emit diagnostics but don’t update counters, so totals can drift. Mirror the error branch.Suggested fix
Err(err) => { let message = match err.downcast::<String>() { Ok(msg) => format!("processing panicked: {msg}"), Err(err) => match err.downcast::<&'static str>() { Ok(msg) => format!("processing panicked: {msg}"), Err(_) => String::from("processing panicked"), }, }; + ctx.increment_unchanged(); + ctx.increment_skipped(); ctx.push_message( PanicDiagnostic { message } .with_file_path(biome_path.to_string()) .into(),crates/biome_cli/src/reporter/mod.rs-201-206 (1)
201-206:⚠️ Potential issue | 🟡 MinorDelegate
dump/clearto the wrapped console.
Stubbing these out hides output for consoles that implement them.Suggested fix
fn dump(&mut self) -> Option<String> { - None + self.0.dump() } - fn clear(&mut self) {} + fn clear(&mut self) { + self.0.clear(); + }crates/biome_cli/src/reporter/json.rs-412-429 (1)
412-429:⚠️ Potential issue | 🟡 MinorPanic risk on invalid spans.
Lines 415-416 use
expect("Invalid span")which will panic ifsource.location()fails. Whilst the guard on line 413 ensuresspanandsource_codeexist, edge cases (e.g., spans pointing beyond source bounds) could still cause panics. Consider returning early or using defaults.🛡️ Defensive alternative
fn record_frame(&mut self, location: Location<'_>) -> std::io::Result<()> { 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"); + let Some(start) = source.location(span.start()).ok() else { + return Ok(()); + }; + let Some(end) = source.location(span.end()).ok() else { + return Ok(()); + }; self.suggestions.push(JsonSuggestion {
🧹 Nitpick comments (25)
.changeset/blue-buttons-own.md (1)
1-3: Change type should bepatchfor main-branch non‑breaking updates.This PR targets
main, so the changeset should bepatchunless it’s a breaking change. Please adjust the front matter accordingly. As per coding guidelines, “Use 'patch' for bug fixes and non-breaking changes targeting the main branch.”.changeset/add-use-anchor-content-html.md (1)
5-5: Please add an invalid example snippet.Changesets for new lint rules should include an invalid-case example so users immediately understand what gets flagged. A short inline or fenced HTML example would do nicely.
As per coding guidelines: “For new lint rules in changesets, show an example of invalid case in inline code or code block”.
.changeset/short-points-tie.md (1)
5-55: Show formatting changes as diffs.This is a formatter-facing change, so the examples should be presented as diff code blocks to highlight the before/after formatting. Based on learnings: “For formatter changes in changesets, show formatting changes using diff code blocks”.
.changeset/tiny-dodos-ask.md (1)
5-5: Consider adding an issue number.As per coding guidelines, bug fix changesets should follow the format: "Fixed [
#NUMBER](issue link): ..." to help users track the original issue..github/workflows/benchmark_html.yml (2)
10-15: Redundant path patterns.Lines 12–14 are already covered by
crates/biome_html_*/**/*.rson line 15. Same applies to the push paths (lines 25–28). You could simplify by removing the explicitbiome_html_analyze,biome_html_formatter, andbiome_html_parserentries.That said, if these are kept intentionally for clarity, feel free to ignore.
🧹 Suggested simplification
paths: - 'Cargo.lock' - - 'crates/biome_html_analyze/**/*.rs' - - 'crates/biome_html_formatter/**/*.rs' - - 'crates/biome_html_parser/**/*.rs' - 'crates/biome_html_*/**/*.rs' - 'crates/biome_formatter/**/*.rs' - 'crates/biome_rowan/**/*.rs' - 'crates/biome_parser/**/*.rs'
63-63: Considernextbranch for cache base.
cache-baseis set tomain, but this workflow also runs on thenextbranch. Depending on how oftennextdiverges, builds on that branch might miss warm cache hits. If that's a concern, you could potentially use a conditional or accept the current behaviour..changeset/html-use-alt-text.md (1)
5-5: Consider simplifying to focus on impact rather than implementation details.The description includes helpful accessibility context but enumerates specific elements and attributes. Per the guidelines, changeset descriptions should explain impact and why users care, not implementation details. The documentation link already provides the technical specifics.
✨ Suggested refinement
-Added the [`useAltText`](https://biomejs.dev/linter/rules/use-alt-text/) lint rule for HTML. This rule enforces that elements requiring alternative text (`<img>`, `<area>`, `<input type="image">`, `<object>`) provide meaningful information for screen reader users via `alt`, `title` (for objects), `aria-label`, or `aria-labelledby` attributes. Elements with `aria-hidden="true"` are exempt. +Added the [`useAltText`](https://biomejs.dev/linter/rules/use-alt-text/) lint rule for HTML. This rule enforces that elements requiring alternative text provide meaningful information for screen reader users, improving accessibility.Based on learnings: "Write changeset descriptions for end users, explaining impact and why they care, not implementation details. Be concise (1-3 sentences)."
.claude/skills/prettier-compare/SKILL.md (1)
44-44: Optional: Consider adding a comma before "so".The sentence would read slightly better as: "...without an extension, so both formatters...". Totally minor though.
crates/biome_cli/tests/commands/format.rs (1)
3727-3728: Consider addingassert_file_contentsfor consistency.The JSON trailing newline tests omit file content assertions while all other language tests (JS, CSS, GraphQL, HTML) include them. If this is intentional due to JSON formatter behaviour, a brief comment explaining why would be helpful. Otherwise, consider adding:
assert_file_contents(&fs, test, "{ \"name\": \"test\" }");.claude/skills/testing-codegen/SKILL.md (1)
59-60: Consider clarifying theqtcommand on first use.The
just qtcommand is introduced here without explanation. First-time readers might not immediately recognise that "qt" stands for "quick test". Consider adding a brief note likejust qt biome_js_analyze # qt = quick testor explaining it in the surrounding text..changeset/promote-nursery-rules-to-stable.md (1)
23-24: Minor readability suggestion.Consider adding a comma after "Vue 3 setup" for clarity: "...props in Vue 3 setup, which causes reactivity loss."
✏️ Suggested fix
-- [`noVueSetupPropsReactivityLoss`](https://biomejs.dev/linter/rules/no-vue-setup-props-reactivity-loss/). The rule reports destructuring of props in Vue 3 setup which causes reactivity loss. +- [`noVueSetupPropsReactivityLoss`](https://biomejs.dev/linter/rules/no-vue-setup-props-reactivity-loss/). The rule reports destructuring of props in Vue 3 setup, which causes reactivity loss..changeset/rotten-squids-ring.md (1)
11-11: Minor grammar nit."The priority how Biome will attempt" reads a bit awkwardly. Consider: "The order in which Biome will attempt" or "The priority order for how Biome will attempt".
Suggested fix
-The priority how Biome will attempt to load the configuration files is the following: +The order in which Biome will attempt to load the configuration files is the following:crates/biome_cli/src/runner/impls/process_file/check.rs (2)
52-77: Empty if-branch is a code smell; prefer negated condition.The empty block at lines 53-54 makes the intent less clear. Consider inverting the condition to eliminate the empty branch.
♻️ Suggested refactor
if features_supported.supports_format() { - if execution.should_skip_parse_errors() && skipped_parse_error { - // Parse errors are already skipped during the analyze phase, so no need to do it here. - } else { + // Parse errors are already skipped during the analyze phase, so no need to do it here. + if !(execution.should_skip_parse_errors() && skipped_parse_error) { let format_result = FormatProcessFile::process_file(ctx, workspace_file, features_supported);
30-50: Consider extracting duplicated result-handling logic.The match arms for
analyzer_result(lines 30-50) andformat_result(lines 59-75) are nearly identical. A helper function could reduce duplication and improve maintainability.Also applies to: 59-75
crates/biome_cli/src/runner/impls/executions/summary_verb.rs (1)
20-30: DuplicatedFilesformatting logic.This
Filesstruct and itsDisplayimplementation duplicate the one incrates/biome_cli/src/reporter/terminal.rs(lines 169-176). Consider extracting to a shared location to maintain consistency and reduce duplication.crates/biome_cli/tests/commands/ci.rs (1)
987-994: Consider extracting repeated message-filtering logic.The pattern for filtering console messages by
LogLevel::Errorand content appears five times. A helper function would reduce duplication and make tests more maintainable.Also applies to: 1019-1026, 1059-1066, 1102-1109, 1146-1153
crates/biome_cli/tests/cases/reporter_checkstyle.rs (1)
190-190: Consider using.xmlextension for checkstyle output.Checkstyle format is XML, so
--reporter-file=file.xmlwould be more conventional thanfile.json. Not a blocker—just a clarity nit.crates/biome_cli/src/cli_options.rs (1)
194-204: Redundant struct-like pattern on unit variants.The
Displayimpl uses{ .. }patterns on unit enum variants (e.g.,Self::Default { .. }), but these variants have no fields. This is syntactically valid but misleading and inconsistent with the enum definition.🔧 Suggested simplification
impl Display for CliReporterKind { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - Self::Default { .. } => f.write_str("default"), - Self::Json { .. } => f.write_str("json"), - Self::JsonPretty { .. } => f.write_str("json-pretty"), - Self::Summary { .. } => f.write_str("summary"), - Self::GitHub { .. } => f.write_str("github"), - Self::Junit { .. } => f.write_str("junit"), - Self::GitLab { .. } => f.write_str("gitlab"), - Self::Checkstyle { .. } => f.write_str("checkstyle"), - Self::RdJson { .. } => f.write_str("rdjson"), - Self::Sarif { .. } => f.write_str("sarif"), + Self::Default => f.write_str("default"), + Self::Json => f.write_str("json"), + Self::JsonPretty => f.write_str("json-pretty"), + Self::Summary => f.write_str("summary"), + Self::GitHub => f.write_str("github"), + Self::Junit => f.write_str("junit"), + Self::GitLab => f.write_str("gitlab"), + Self::Checkstyle => f.write_str("checkstyle"), + Self::RdJson => f.write_str("rdjson"), + Self::Sarif => f.write_str("sarif"), } } }crates/biome_analyze/src/profiling.rs (1)
356-478: Consider extracting the cutoff calculation logic.The cutoff calculation pattern is repeated five times (lines 394-427). While readable, extracting a helper function could reduce duplication.
crates/biome_cli/src/runner/mod.rs (1)
444-444: TODO: CLI flag for force scan.There's a TODO about potentially adding a CLI flag for force scanning. Consider tracking this in an issue if it's a planned feature.
Would you like me to open an issue to track this potential feature?
crates/biome_cli/src/reporter/sarif.rs (1)
147-175: Filtering logic could be simplified.The verbose/non-verbose filtering has duplicated branches. Consider restructuring to reduce nesting.
🔧 Suggested simplification
let sarif_results: Vec<_> = payload .diagnostics .iter() .filter_map(|diagnostic| { - if diagnostic.severity() >= payload.diagnostic_level { - if diagnostic.tags().is_verbose() { - if verbose { - if let Some(driver_rule) = - to_sarif_driver_rule(diagnostic, &self.rule_descriptions) - { - sarif_rules.insert(driver_rule); - } - to_sarif_result(diagnostic, working_directory) - } else { - None - } - } else { - if let Some(driver_rule) = - to_sarif_driver_rule(diagnostic, &self.rule_descriptions) - { - sarif_rules.insert(driver_rule); - } - to_sarif_result(diagnostic, working_directory) - } - } else { - None + if diagnostic.severity() < payload.diagnostic_level { + return None; + } + if diagnostic.tags().is_verbose() && !verbose { + return None; } + if let Some(driver_rule) = + to_sarif_driver_rule(diagnostic, &self.rule_descriptions) + { + sarif_rules.insert(driver_rule); + } + to_sarif_result(diagnostic, working_directory) }) .collect();crates/biome_cli/src/reporter/github.rs (1)
56-60: Redundant condition check.The
else if diagnostic.tags().is_verbose() && verboseon line 58 is redundant—since theifon line 56 already checked!diagnostic.tags().is_verbose(), theelse ifbranch will only execute whenis_verbose()is true. Simplify toelse if verbose.✨ Suggested simplification
if diagnostic.severity() >= diagnostics_payload.diagnostic_level { if !diagnostic.tags().is_verbose() { writer.log(markup! {{PrintGitHubDiagnostic(diagnostic)}}); - } else if diagnostic.tags().is_verbose() && verbose { + } else if verbose { writer.log(markup! {{PrintGitHubDiagnostic(diagnostic)}}); } }crates/biome_cli/src/reporter/json.rs (2)
215-220: Dual serialisation paths present.The
Displayimpl usesserde_json::to_string(&self)(line 217), whilstto_json()builds aJsonRootAST. If these are meant to produce identical output, there's a risk of divergence. If they serve different purposes (e.g., one for streaming, one for formatted output), consider adding a brief doc comment clarifying when each is used.
312-321: Consider propagating errors instead of unwrap.Line 318 uses
unwrap()ondiagnostic.advices(&mut visitor). Whilst unlikely to fail, propagating the error would be more defensive.✨ Suggested improvement
fn to_advices(diagnostic: &Error) -> Vec<JsonSuggestion> { let mut visitor = SuggestionsVisitor { suggestions: vec![], last_diagnostic_length: 0, current_message: None, }; - diagnostic.advices(&mut visitor).unwrap(); + // Errors during advice collection are non-fatal; return partial results + let _ = diagnostic.advices(&mut visitor); visitor.suggestions }crates/biome_cli/src/reporter/terminal.rs (1)
249-252: Vague comments could be clarified.The comments "For now, we'll assume..." appear twice (lines 189-190 and 250-251). If this is a known limitation pending refinement, consider adding a TODO or linking to a tracking issue.
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Added e18e ESLint plugin as a recognized rule source. Six Biome rules now reference their e18e equivalents: `useAtIndex`, `useExponentiationOperator`, `noPrototypeBuiltins`, `useDateNow`, `useSpread`, and `useObjectSpread`. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add documentation links for the rules.
The six rules should include links to their documentation pages. As per coding guidelines, reference rules with links to the website documentation.
📚 Suggested improvement
-Added e18e ESLint plugin as a recognized rule source. Six Biome rules now reference their e18e equivalents: `useAtIndex`, `useExponentiationOperator`, `noPrototypeBuiltins`, `useDateNow`, `useSpread`, and `useObjectSpread`.
+Added e18e ESLint plugin as a recognised rule source. Six Biome rules now reference their e18e equivalents: [useAtIndex](https://biomejs.dev/linter/rules/use-at-index), [useExponentiationOperator](https://biomejs.dev/linter/rules/use-exponentiation-operator), [noPrototypeBuiltins](https://biomejs.dev/linter/rules/no-prototype-builtins), [useDateNow](https://biomejs.dev/linter/rules/use-date-now), [useSpread](https://biomejs.dev/linter/rules/use-spread), and [useObjectSpread](https://biomejs.dev/linter/rules/use-object-spread).📝 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.
| Added e18e ESLint plugin as a recognized rule source. Six Biome rules now reference their e18e equivalents: `useAtIndex`, `useExponentiationOperator`, `noPrototypeBuiltins`, `useDateNow`, `useSpread`, and `useObjectSpread`. | |
| Added e18e ESLint plugin as a recognised rule source. Six Biome rules now reference their e18e equivalents: [useAtIndex](https://biomejs.dev/linter/rules/use-at-index), [useExponentiationOperator](https://biomejs.dev/linter/rules/use-exponentiation-operator), [noPrototypeBuiltins](https://biomejs.dev/linter/rules/no-prototype-builtins), [useDateNow](https://biomejs.dev/linter/rules/use-date-now), [useSpread](https://biomejs.dev/linter/rules/use-spread), and [useObjectSpread](https://biomejs.dev/linter/rules/use-object-spread). |
🤖 Prompt for AI Agents
In @.changeset/add-e18e-rule-source.md at line 5, Update the six rule mentions
to include their documentation links: for each rule name useAtIndex,
useExponentiationOperator, noPrototypeBuiltins, useDateNow, useSpread, and
useObjectSpread, append or replace the plain text with a Markdown link to the
official e18e/website docs (e.g. [useAtIndex](https://.../rules/use-at-index)
style). Ensure each rule reference points to its corresponding rule page on the
public docs site so all six entries become clickable documentation links.
| --- | ||
| "@biomejs/biome": minor | ||
| --- |
There was a problem hiding this comment.
Change type should be patch for main.
This changeset is marked minor, but the guideline says main branch changesets must be patch. Please align the front matter accordingly. As per coding guidelines, “Use 'patch' for bug fixes and non-breaking changes targeting the main branch, 'minor' for new features targeting the next branch, and 'major' for breaking changes targeting the next branch.”
🔧 Suggested front matter update
"@biomejs/biome": minor
+"@biomejs/biome": patch📝 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.
| --- | |
| "@biomejs/biome": minor | |
| --- | |
| --- | |
| "@biomejs/biome": patch | |
| --- |
🤖 Prompt for AI Agents
In @.changeset/angry-women-accept.md around lines 1 - 3, Update the changeset
front matter to use "patch" instead of "minor" for the "@biomejs/biome" entry so
it aligns with main-branch rules; locate the YAML front matter block in the
changeset (the top lines showing "@biomejs/biome": minor) and change the value
from minor to patch, keeping the rest of the front matter intact.
| --- | ||
| "@biomejs/biome": minor | ||
| --- |
There was a problem hiding this comment.
Confirm the release type matches the target branch.
This PR targets main, but the changeset declares a minor release. If this is being merged into main, should this be patch instead, or is the intent to carry a next-feature changeset through the merge? A quick confirm here avoids a release bump surprise. As per coding guidelines, "Use 'patch' for bug fixes and non-breaking changes targeting the main branch, 'minor' for new features targeting the next branch, and 'major' for breaking changes targeting the next branch."
🤖 Prompt for AI Agents
In @.changeset/odd-flies-nail.md around lines 1 - 3, The changeset currently
declares a "minor" release but the PR targets main; update the changeset
declaration in odd-flies-nail.md to "patch" if this is a non-breaking fix for
main, or explicitly confirm and document that a "minor" release should be
carried (i.e., this is a feature intended for next) so the release type matches
the branch intent; ensure the first line in the changeset front-matter is
adjusted accordingly.
| --- | ||
| "@biomejs/biome": minor |
There was a problem hiding this comment.
Change type should be patch on main.
This PR targets main, so the changeset type should be patch, not minor. As it stands, it violates the branch-specific changeset guidance. Based on learnings: Target 'main' branch for bug fixes (patch changesets) and new nursery rules (patch changesets). Target 'next' branch for rule promotions from nursery (minor), new features (minor), and breaking changes (major). As per coding guidelines "Use 'patch' for bug fixes and non-breaking changes targeting the main branch, 'minor' for new features targeting the next branch, and 'major' for breaking changes targeting the next branch."
🤖 Prompt for AI Agents
In @.changeset/small-seas-laugh.md around lines 1 - 2, Update the changeset
entry that currently reads `"@biomejs/biome": minor` to use `"patch"` since this
PR targets main; locate the .changeset file containing the changeset header and
change the version bump type from minor to patch so it conforms to the
branch-specific changeset guidance.
| --- | ||
| "@biomejs/biome": minor | ||
| --- | ||
|
|
||
| Improved the rule `noUnusedVariables` in Svelte files, by correctly detecting variables defined in the JavaScript blocks, and used inside the templates. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use a patch changeset and the required “Fixed #NUMBER…” format.
This reads like a bug fix on main, so the changeset should be patch, and the description must start with “Fixed #NUMBER(…)…”. Please update both accordingly. As per coding guidelines: “Use 'patch' for bug fixes and non-breaking changes targeting the main branch… For bug fixes, start with 'Fixed [#NUMBER](issue link): ...'.”
✍️ Suggested changeset edit
---
-"@biomejs/biome": minor
+"@biomejs/biome": patch
---
-Improved the rule `noUnusedVariables` in Svelte files, by correctly detecting variables defined in the JavaScript blocks, and used inside the templates.
+Fixed [`#NUMBER`](issue link): `noUnusedVariables` in Svelte now flags variables declared in `<script>` blocks but unused in templates.Based on learnings: “Target 'main' branch for bug fixes (patch changesets) …” and as per coding guidelines: “Use 'patch' for bug fixes… For bug fixes, start with 'Fixed [#NUMBER](issue link): ...'.”
🤖 Prompt for AI Agents
In @.changeset/wide-masks-mix.md around lines 1 - 5, The changeset currently
labels the release as a minor change and uses a generic description; change the
frontmatter tag from "minor" to "patch" and update the description line to begin
with the required bug-fix format "Fixed `#NUMBER`(...): ..." (replace `#NUMBER` with
the actual issue number and include the issue link if available), ensuring both
the frontmatter and the human-readable description at the top of
.changeset/wide-masks-mix.md are updated to reflect a patch bugfix.
| CliReporterKind::Json | CliReporterKind::JsonPretty => { | ||
| console_reporter_writer.error(markup! { | ||
| <Warn>"The "<Emphasis>"--json"</Emphasis>" option is "<Underline>"unstable/experimental"</Underline>" and its output might change between patches/minor releases."</Warn> | ||
| }); | ||
| let reporter = JsonReporter { | ||
| summary, | ||
| diagnostics_payload: &diagnostics_payload, | ||
| execution, | ||
| verbose: cli_options.verbose, | ||
| working_directory: fs.working_directory().clone(), | ||
| }; | ||
| let mut buffer = JsonReporterVisitor::new(summary); | ||
| reporter.write(&mut console_reporter_writer, &mut buffer)?; | ||
| let root = buffer.to_json(); | ||
| if cli_reporter.kind == CliReporterKind::JsonPretty { | ||
| let formatted = biome_json_formatter::format_node( | ||
| JsonFormatOptions::default(), | ||
| root.syntax(), | ||
| ) | ||
| .expect("To format the JSON report") | ||
| .print() | ||
| .expect("To print the JSON report"); | ||
|
|
||
| console_reporter_writer.log(markup! { | ||
| {formatted.as_code()} | ||
| }); | ||
| } else { | ||
| let code = root.to_string(); | ||
| console_reporter_writer.log(markup! { | ||
| {code} | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Respect JSON file destinations.
When cli_reporter.is_file_report() is true, JSON output still goes to the console writer, so file_reporter_writer.dump() stays empty and the destination never gets written.
Proposed fix
- let reporter = JsonReporter {
+ let reporter = JsonReporter {
summary,
diagnostics_payload: &diagnostics_payload,
execution,
verbose: cli_options.verbose,
working_directory: fs.working_directory().clone(),
};
- let mut buffer = JsonReporterVisitor::new(summary);
- reporter.write(&mut console_reporter_writer, &mut buffer)?;
+ let mut buffer = JsonReporterVisitor::new(summary);
+ let writer: &mut dyn ReporterWriter = if cli_reporter.is_file_report() {
+ &mut file_reporter_writer
+ } else {
+ &mut console_reporter_writer
+ };
+ reporter.write(writer, &mut buffer)?;
let root = buffer.to_json();
if cli_reporter.kind == CliReporterKind::JsonPretty {
let formatted = biome_json_formatter::format_node(
JsonFormatOptions::default(),
root.syntax(),
)
.expect("To format the JSON report")
.print()
.expect("To print the JSON report");
- console_reporter_writer.log(markup! {
+ writer.log(markup! {
{formatted.as_code()}
});
} else {
let code = root.to_string();
- console_reporter_writer.log(markup! {
+ writer.log(markup! {
{code}
});
}📝 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.
| CliReporterKind::Json | CliReporterKind::JsonPretty => { | |
| console_reporter_writer.error(markup! { | |
| <Warn>"The "<Emphasis>"--json"</Emphasis>" option is "<Underline>"unstable/experimental"</Underline>" and its output might change between patches/minor releases."</Warn> | |
| }); | |
| let reporter = JsonReporter { | |
| summary, | |
| diagnostics_payload: &diagnostics_payload, | |
| execution, | |
| verbose: cli_options.verbose, | |
| working_directory: fs.working_directory().clone(), | |
| }; | |
| let mut buffer = JsonReporterVisitor::new(summary); | |
| reporter.write(&mut console_reporter_writer, &mut buffer)?; | |
| let root = buffer.to_json(); | |
| if cli_reporter.kind == CliReporterKind::JsonPretty { | |
| let formatted = biome_json_formatter::format_node( | |
| JsonFormatOptions::default(), | |
| root.syntax(), | |
| ) | |
| .expect("To format the JSON report") | |
| .print() | |
| .expect("To print the JSON report"); | |
| console_reporter_writer.log(markup! { | |
| {formatted.as_code()} | |
| }); | |
| } else { | |
| let code = root.to_string(); | |
| console_reporter_writer.log(markup! { | |
| {code} | |
| }); | |
| } | |
| } | |
| CliReporterKind::Json | CliReporterKind::JsonPretty => { | |
| console_reporter_writer.error(markup! { | |
| <Warn>"The "<Emphasis>"--json"</Emphasis>" option is "<Underline>"unstable/experimental"</Underline>" and its output might change between patches/minor releases."</Warn> | |
| }); | |
| let reporter = JsonReporter { | |
| summary, | |
| diagnostics_payload: &diagnostics_payload, | |
| execution, | |
| verbose: cli_options.verbose, | |
| working_directory: fs.working_directory().clone(), | |
| }; | |
| let mut buffer = JsonReporterVisitor::new(summary); | |
| let writer: &mut dyn ReporterWriter = if cli_reporter.is_file_report() { | |
| &mut file_reporter_writer | |
| } else { | |
| &mut console_reporter_writer | |
| }; | |
| reporter.write(writer, &mut buffer)?; | |
| let root = buffer.to_json(); | |
| if cli_reporter.kind == CliReporterKind::JsonPretty { | |
| let formatted = biome_json_formatter::format_node( | |
| JsonFormatOptions::default(), | |
| root.syntax(), | |
| ) | |
| .expect("To format the JSON report") | |
| .print() | |
| .expect("To print the JSON report"); | |
| writer.log(markup! { | |
| {formatted.as_code()} | |
| }); | |
| } else { | |
| let code = root.to_string(); | |
| writer.log(markup! { | |
| {code} | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@crates/biome_cli/src/runner/impls/finalizers/default.rs` around lines 111 -
143, The JSON branch currently always writes to console_reporter_writer so file
destinations are ignored; update the CliReporterKind::Json / JsonPretty handling
to detect cli_reporter.is_file_report() and use file_reporter_writer (instead of
console_reporter_writer) when true: construct the JsonReporter and
JsonReporterVisitor as now, call reporter.write(...) with the chosen writer and
buffer, obtain root from buffer.to_json(), then either format/print or to_string
and call file_reporter_writer.log(...) (and ensure file_reporter_writer.dump()
is invoked where needed) so both pretty and non‑pretty JSON end up in the file
output rather than only the console.
| if let Some(fix_mode) = execution.as_fix_file_mode() { | ||
| let suppression_explanation = | ||
| if execution.suppress() && execution.suppression_reason().is_none() { | ||
| "ignored using `--suppress`" | ||
| } else { | ||
| execution.suppression_reason().unwrap_or("<explanation>") | ||
| }; | ||
|
|
||
| let fix_result = workspace_file | ||
| .guard() | ||
| .fix_file( | ||
| fix_mode, | ||
| features_supported.supports_format(), | ||
| categories, | ||
| only.clone(), | ||
| skip.clone(), | ||
| Some(suppression_explanation.to_string()), | ||
| ) |
There was a problem hiding this comment.
Only pass a suppression reason when suppression is enabled.
Right now suppression_explanation is always Some, which can trigger suppression actions or embed “” even when suppression isn’t requested.
Suggested fix
- let suppression_explanation =
- if execution.suppress() && execution.suppression_reason().is_none() {
- "ignored using `--suppress`"
- } else {
- execution.suppression_reason().unwrap_or("<explanation>")
- };
+ let suppression_explanation = if execution.suppress() {
+ Some(
+ execution
+ .suppression_reason()
+ .unwrap_or("ignored using `--suppress`")
+ .to_string(),
+ )
+ } else {
+ None
+ };
...
- Some(suppression_explanation.to_string()),
+ suppression_explanation,📝 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.
| if let Some(fix_mode) = execution.as_fix_file_mode() { | |
| let suppression_explanation = | |
| if execution.suppress() && execution.suppression_reason().is_none() { | |
| "ignored using `--suppress`" | |
| } else { | |
| execution.suppression_reason().unwrap_or("<explanation>") | |
| }; | |
| let fix_result = workspace_file | |
| .guard() | |
| .fix_file( | |
| fix_mode, | |
| features_supported.supports_format(), | |
| categories, | |
| only.clone(), | |
| skip.clone(), | |
| Some(suppression_explanation.to_string()), | |
| ) | |
| if let Some(fix_mode) = execution.as_fix_file_mode() { | |
| let suppression_explanation = if execution.suppress() { | |
| Some( | |
| execution | |
| .suppression_reason() | |
| .unwrap_or("ignored using `--suppress`") | |
| .to_string(), | |
| ) | |
| } else { | |
| None | |
| }; | |
| let fix_result = workspace_file | |
| .guard() | |
| .fix_file( | |
| fix_mode, | |
| features_supported.supports_format(), | |
| categories, | |
| only.clone(), | |
| skip.clone(), | |
| suppression_explanation, | |
| ) |
🤖 Prompt for AI Agents
In `@crates/biome_cli/src/runner/impls/process_file/lint_and_assist.rs` around
lines 45 - 62, The code always constructs Some(suppression_explanation) and
passes it to workspace_file.guard().fix_file(...), which causes suppression
metadata to be applied even when execution.suppress() is false; change the logic
to build an Option<String> that is None when suppression is not enabled and
Some(reason) only when execution.suppress() is true (use
execution.suppression_reason().map(|s| s.to_string()).or_else(|| Some("ignored
using `--suppress`".to_string())) when suppressed), then pass that Option
directly to fix_file instead of wrapping an unconditional Some(...).
| workspace.open_file(OpenFileParams { | ||
| project_key, | ||
| path: biome_path.clone(), | ||
| content: FileContent::from_client(content), | ||
| document_file_source: None, | ||
| persist_node_cache: false, | ||
| inline_config: None, | ||
| })?; | ||
|
|
||
| // apply fix file of the linter | ||
| let FileFeaturesResult { | ||
| features_supported: file_features, | ||
| } = workspace.file_features(SupportsFeatureParams { | ||
| project_key, | ||
| path: biome_path.clone(), | ||
| features: payload.execution.wanted_features(), | ||
| inline_config: None, | ||
| skip_ignore_check, | ||
| not_requested_features: payload.execution.not_requested_features(), | ||
| })?; | ||
|
|
||
| if file_features.is_ignored() { | ||
| console.append(markup! {{content}}); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| if file_features.is_protected() { | ||
| let protected_diagnostic = WorkspaceError::protected_file(biome_path.to_string()); | ||
| if protected_diagnostic.tags().is_verbose() { | ||
| if cli_options.verbose { | ||
| console.error(markup! {{PrintDiagnostic::verbose(&protected_diagnostic)}}) | ||
| } | ||
| } else { | ||
| console.error(markup! {{PrintDiagnostic::simple(&protected_diagnostic)}}) | ||
| } | ||
| console.append(markup! {{content}}); | ||
|
|
||
| return Ok(()); | ||
| }; |
There was a problem hiding this comment.
Close the workspace file on early exits.
open_file is called, but ignored/protected paths return without close_file, leaving an open document behind.
Suggested fix
workspace.open_file(OpenFileParams {
project_key,
path: biome_path.clone(),
content: FileContent::from_client(content),
document_file_source: None,
persist_node_cache: false,
inline_config: None,
})?;
+ let close = || {
+ let _ = workspace.close_file(CloseFileParams {
+ project_key,
+ path: biome_path.clone(),
+ });
+ };
...
if file_features.is_ignored() {
console.append(markup! {{content}});
+ close();
return Ok(());
}
...
if file_features.is_protected() {
let protected_diagnostic = WorkspaceError::protected_file(biome_path.to_string());
if protected_diagnostic.tags().is_verbose() {
if cli_options.verbose {
console.error(markup! {{PrintDiagnostic::verbose(&protected_diagnostic)}})
}
} else {
console.error(markup! {{PrintDiagnostic::simple(&protected_diagnostic)}})
}
console.append(markup! {{content}});
+ close();
return Ok(());
};🤖 Prompt for AI Agents
In `@crates/biome_cli/src/runner/impls/process_file/lint_and_assist.rs` around
lines 183 - 221, open_file is called on workspace but early returns for ignored
or protected files (branches using file_features.is_ignored() and
file_features.is_protected()) never call close_file, leaving documents open;
before each return in those branches, call workspace.close_file(CloseFileParams
{ project_key, path: biome_path.clone() }) (or the crate's equivalent close_file
API) to ensure the file is closed, and do this both for the ignored branch and
for the protected branch right after logging/console operations so the
open/close pairing is preserved even on early exits.
| if file_features.supports_format() && execution.is_check() { | ||
| let printed = workspace.format_file(FormatFileParams { | ||
| project_key, | ||
| path: biome_path.clone(), | ||
| inline_config: None, | ||
| })?; | ||
| let code = printed.into_code(); | ||
| let output = if !file_features.supports_full_html_support() { | ||
| match biome_path.extension() { | ||
| Some("astro") => AstroFileHandler::output(&new_content, code.as_str()), | ||
| Some("vue") => VueFileHandler::output(&new_content, code.as_str()), | ||
| Some("svelte") => SvelteFileHandler::output(&new_content, code.as_str()), | ||
| _ => code, | ||
| } | ||
| } else { | ||
| code | ||
| }; | ||
| if (execution.is_safe_fixes_enabled() || execution.is_safe_and_unsafe_fixes_enabled()) | ||
| && output != new_content | ||
| { | ||
| new_content = Cow::Owned(output); | ||
| } | ||
| } | ||
|
|
||
| match new_content { | ||
| Cow::Borrowed(original_content) => { | ||
| console.append(markup! { | ||
| {original_content} | ||
| }); | ||
|
|
||
| if !execution.requires_write_access() { | ||
| return Err(StdinDiagnostic::new_not_formatted().into()); | ||
| } |
There was a problem hiding this comment.
Don’t error on stdin when formatting didn’t change.
NotFormatted is returned whenever content is unchanged and write isn’t allowed, even if formatting would produce identical output.
Suggested fix
- if file_features.supports_format() && execution.is_check() {
+ let mut format_changed = false;
+ if file_features.supports_format() && execution.is_check() {
let printed = workspace.format_file(FormatFileParams {
project_key,
path: biome_path.clone(),
inline_config: None,
})?;
let code = printed.into_code();
let output = if !file_features.supports_full_html_support() {
match biome_path.extension() {
Some("astro") => AstroFileHandler::output(&new_content, code.as_str()),
Some("vue") => VueFileHandler::output(&new_content, code.as_str()),
Some("svelte") => SvelteFileHandler::output(&new_content, code.as_str()),
_ => code,
}
} else {
code
};
+ if output != new_content {
+ format_changed = true;
+ }
if (execution.is_safe_fixes_enabled() || execution.is_safe_and_unsafe_fixes_enabled())
&& output != new_content
{
new_content = Cow::Owned(output);
}
}
...
Cow::Borrowed(original_content) => {
console.append(markup! {
{original_content}
});
- if !execution.requires_write_access() {
+ if format_changed && !execution.requires_write_access() {
return Err(StdinDiagnostic::new_not_formatted().into());
}
}🤖 Prompt for AI Agents
In `@crates/biome_cli/src/runner/impls/process_file/lint_and_assist.rs` around
lines 274 - 306, The code currently returns StdinDiagnostic::new_not_formatted()
whenever new_content is borrowed and execution.requires_write_access() is false,
which wrongfully errors on stdin even when formatting would produce identical
output; change the logic so we only return the NotFormatted error if the
formatter (workspace.format_file / the computed output) would have produced a
different string than original_content. Concretely: after computing output (the
formatted string) and before possibly replacing new_content, record whether
formatting_changed = output != original_content (or compare the formatter result
or transformed code that would be written), and in the match arm for
Cow::Borrowed(original_content) only return
Err(StdinDiagnostic::new_not_formatted().into()) when
!execution.requires_write_access() && formatting_changed (or other
transformations would have changed), otherwise proceed without error. Ensure you
reference the existing variables execution, output, new_content,
original_content, workspace.format_file and StdinDiagnostic::new_not_formatted()
when making the change.
| let open_options = OpenOptions::default() | ||
| .read(true) | ||
| .write(ctx.execution().requires_write_access()); | ||
|
|
||
| let mut file = ctx | ||
| .fs() | ||
| .open_with_options(path.as_path(), open_options) | ||
| .with_file_path(path.to_string())?; | ||
|
|
||
| let mut input = String::new(); | ||
| file.read_to_string(&mut input) | ||
| .with_file_path(path.to_string())?; | ||
|
|
||
| let guard = FileGuard::new(ctx.workspace(), ctx.project_key(), path.clone()) | ||
| .with_file_path_and_code(path.to_string(), category!("internalError/fs"))?; | ||
|
|
||
| if ctx.workspace().file_exists(FileExitsParams { | ||
| file_path: path.clone(), | ||
| })? { | ||
| Ok(Self { guard, path, file }) | ||
| } else { | ||
| let mut input = String::new(); | ||
| file.read_to_string(&mut input) | ||
| .with_file_path(path.to_string())?; | ||
|
|
||
| ctx.workspace().open_file(OpenFileParams { | ||
| project_key: ctx.project_key(), | ||
| document_file_source: None, | ||
| path: path.clone(), | ||
| content: FileContent::from_client(&input), | ||
| persist_node_cache: false, | ||
| inline_config: None, | ||
| })?; | ||
|
|
||
| Ok(Self { guard, path, file }) | ||
| } |
There was a problem hiding this comment.
Avoid reading the file twice before open_file.
The second read runs at EOF, so newly opened workspace files can be initialised with empty content.
Suggested fix
- let mut input = String::new();
- file.read_to_string(&mut input)
- .with_file_path(path.to_string())?;
-
let guard = FileGuard::new(ctx.workspace(), ctx.project_key(), path.clone())
.with_file_path_and_code(path.to_string(), category!("internalError/fs"))?;🤖 Prompt for AI Agents
In `@crates/biome_cli/src/runner/process_file.rs` around lines 235 - 270, The code
reads the file into `input` once, then in the `else` branch re-reads
`file.read_to_string(&mut input)` which runs at EOF and yields empty content;
remove the redundant second read and reuse the earlier `input` when calling
`ctx.workspace().open_file` (keep `OpenOptions`, `file`, `input`, `FileGuard`,
`ctx.workspace().file_exists`, `OpenFileParams`, and `FileContent::from_client`
as-is), ensuring the content passed to `open_file` is the initially read
`input`.
…when applying suppressions (#9059) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/biome_cli/src/reporter/gitlab.rs (1)
160-166:⚠️ Potential issue | 🟡 MinorVerbose filter appears inverted.
When
verboseis true, this only includes diagnostics with the verbose tag, excluding non-verbose ones. Typically verbose mode should show more diagnostics (all including verbose-tagged), not fewer.Expected behaviour
.filter(|d| { - if *verbose { - d.tags().is_verbose() - } else { - true - } + // Include all diagnostics, but skip verbose-tagged ones unless verbose mode is on + !d.tags().is_verbose() || *verbose })crates/biome_cli/src/reporter/summary.rs (1)
170-198:⚠️ Potential issue | 🟠 MajorAvoid double-counting verbose lint diagnostics. When
--verboseis on, verbose lint diagnostics are inserted in the verbose branch and again in the general lint block, inflating counts. Suggest gating once and letting the lint block handle the insertion.🔧 Suggested fix
- if diagnostic.tags().is_verbose() { - if verbose { - if let Some(category) = category - && should_report_lint_diagnostic(category) - { - files_to_diagnostics.insert_rule_for_file( - category.name(), - severity, - location, - ); - } - } else { - continue; - } - } + if diagnostic.tags().is_verbose() && !verbose { + continue; + }- if (execution.is_check() || execution.is_lint() || execution.is_ci()) - && let Some(category) = category - && should_report_lint_diagnostic(category) - { + if (execution.is_check() || execution.is_lint() || execution.is_ci()) + && let Some(category) = category + && should_report_lint_diagnostic(category) + { files_to_diagnostics.insert_rule_for_file(category.name(), severity, location); }
🤖 Fix all issues with AI agents
In @.changeset/bright-foxes-glow.md:
- Around line 1-3: The changeset front matter currently marks the release type
as "minor" which conflicts with main-branch guidance; update the front matter in
.changeset/bright-foxes-glow.md to use "patch" if this PR is targeting main, or
keep "minor" only if you confirm this changeset will remain on the next release
branch; ensure the YAML key "@biomejs/biome": minor is changed to
"@biomejs/biome": patch when targeting main and commit that change.
In @.changeset/tricky-masks-cry.md:
- Around line 1-5: The changeset for "@biomejs/biome" declares a "minor" release
but the PR targets main; either update the changeset change type from "minor" to
"patch" or retarget this PR to the next branch; confirm whether the change is a
user-facing new feature (keep "minor" and move the branch to next) or a
non-breaking bugfix (change the header to "patch") and update the changeset
accordingly so it matches the repository release policy.
In `@crates/biome_cli/src/reporter/json.rs`:
- Around line 387-407: The bug is that SuggestionsVisitor::record_log overwrites
previous suggestion text because self.last_diagnostic_length is never updated;
after you append to the last suggestion or otherwise mutate suggestions you
should set self.last_diagnostic_length to the current number of diagnostics so
future logs branch correctly. In the record_log method
(SuggestionsVisitor::record_log) update self.last_diagnostic_length =
current_diagnostic_length immediately after the block that mutates
self.suggestions (i.e., after the last_suggestion.text = message assignment),
and ensure any code path that creates a new suggestion or consumes
current_message also updates last_diagnostic_length accordingly so subsequent
calls don't clobber earlier entries.
In `@crates/biome_cli/src/reporter/terminal.rs`:
- Around line 154-159: The code currently falls through and prints
verbose-tagged diagnostics even when verbose is false; update the conditional in
the loop that checks diagnostic.severity() so that verbose-tagged diagnostics
are only printed when verbose is true: if diagnostic.tags().is_verbose() then
print PrintDiagnostic::verbose(diagnostic) when verbose is true and otherwise
skip that diagnostic (do not fall through to PrintDiagnostic::simple), else
print PrintDiagnostic::simple(diagnostic). Locate the branch around
diagnostic.severity(), diagnostic.tags().is_verbose(), writer.error and
PrintDiagnostic::verbose/simple and implement the explicit skip (e.g., continue)
for non-verbose context.
In `@crates/biome_cli/src/runner/mod.rs`:
- Around line 264-303: The code reads stdin twice which can consume the stream
and make the second read empty; update configure_workspace/ConfiguredWorkspace
to capture and return an optional Stdin payload (the result of
self.get_stdin/Console::read) so the workspace carries the already-read stdin,
then change the branch that currently calls self.get_stdin(console,
execution.as_ref()) to use the stored stdin from ConfiguredWorkspace; pass that
stored stdin into ProcessFile::process_std_in via ProcessStdinFilePayload (and
still call ProcessFile::should_skip_ignore_check with the biome_path and
workspace) so stdin is only read once and reused for scan-kind derivation and
processing.
🟡 Minor comments (20)
.claude/skills/prettier-compare/SKILL.md-44-44 (1)
44-44:⚠️ Potential issue | 🟡 MinorAdd a comma for readability.
Suggested tweak: “Use
-l/--languagewhen formatting code without an extension, so both formatters pick the correct parser.”.changeset/ninety-rice-like.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorChangeset description must follow the “Fixed [
#NUMBER](issue link): …” format.This is a bug fix, so please prefix the description accordingly. Short and user‑focused is already on the right track.
As per coding guidelines: “For bug fixes, start with 'Fixed [#NUMBER](issue link): ...'.”.changeset/group-by-nesting-feature.md-44-44 (1)
44-44:⚠️ Potential issue | 🟡 MinorFix the syntax error in the code example.
Missing space before the opening brace.
🔧 Proposed fix
-const object ={ +const object = {crates/biome_cli/tests/cases/editorconfig.rs-702-738 (1)
702-738:⚠️ Potential issue | 🟡 MinorExpectation mismatch: “should_support” yet asserts error.
With
insert_final_newline = false, formatting should succeed (and avoid adding a newline) rather than erroring. Please align the assertion (or rename the test if the failure is intentional).Suggested fix
- assert!(result.is_err(), "run_cli returned {result:?}"); + assert!(result.is_ok(), "run_cli returned {result:?}");.changeset/silly-walls-build.md-7-8 (1)
7-8:⚠️ Potential issue | 🟡 MinorAdd a comma after “Now” for readability.
Reads more naturally as “Now, variables and components…”..changeset/silly-walls-build.md-5-11 (1)
5-11:⚠️ Potential issue | 🟡 MinorChange
useImportTypestouseImportType.The canonical rule name is
useImportType(singular), notuseImportTypes. Update line 6 to match..changeset/proud-ends-hear.md-11-21 (1)
11-21:⚠️ Potential issue | 🟡 MinorInvalid JavaScript in the example code.
The example declares
const athree times, which would cause a redeclaration error in JavaScript. Consider using unique variable names to demonstrate the suppression feature with valid code.📝 Proposed fix with unique variable names
```js // biome-ignore-all format: generated -const a = [ ] +const arr1 = [ ] -const a = [ ] +const arr2 = [ ] -const a = [ ] +const arr3 = [ ]</details> </blockquote></details> <details> <summary>.changeset/angry-women-accept.md-5-5 (1)</summary><blockquote> `5-5`: _⚠️ Potential issue_ | _🟡 Minor_ **Fix grammar: "will a emit" → "will emit an".** The phrase "will a emit" is grammatically incorrect. <details> <summary>📝 Proposed fix</summary> ```diff -The Biome CSS parser is now able to parse Vue SFC syntax such as `:slotted` and `:deep`. These pseudo functions are only correctly parsed when the CSS is defined inside `.vue` components. Otherwise, Biome will a emit a parse error. +The Biome CSS parser is now able to parse Vue SFC syntax such as `:slotted` and `:deep`. These pseudo functions are only correctly parsed when the CSS is defined inside `.vue` components. Otherwise, Biome will emit an parse error..claude/skills/README.md-30-30 (1)
30-30:⚠️ Potential issue | 🟡 MinorLine count estimate seems a bit generous.
The entry claims
~320lines, but the actual file is 265 lines. Might want to adjust to~265for accuracy.📏 Suggested correction
-| **[biome-developer](./biome-developer/SKILL.md)** | General development best practices, common gotchas, Biome-specific patterns | Any agent | ~320 | +| **[biome-developer](./biome-developer/SKILL.md)** | General development best practices, common gotchas, Biome-specific patterns | Any agent | ~265 |.changeset/rich-flowers-hunt.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorFix the documentation URL—it includes "selector" when it shouldn't.
The URL should be
no-unknown-pseudo-element, notno-unknown-selector-pseudo-element. See: https://biomejs.dev/linter/rules/no-unknown-pseudo-element/crates/biome_cli/src/panic.rs-35-43 (1)
35-43:⚠️ Potential issue | 🟡 MinorTidy up a small grammar hiccup.
User-facing copy reads a bit off.✏️ Suggested fix
- "This is a bug in Biome, not an error in your code, and we would appreciate it if you could report it to https://github.com/biomejs/biome/issues/ along with the following information to help us fixing the issue." + "This is a bug in Biome, not an error in your code, and we would appreciate it if you could report it to https://github.com/biomejs/biome/issues/ along with the following information to help us fix the issue.".changeset/chubby-buttons-lie.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorFix the wording about exporting the env var.
You exportRUST_BACKTRACEin your shell environment, not$PATH.✍️ Proposed wording
-It's now possible to provide the stacktrace for a fatal error. The stacktrace is only available when the environment variable `RUST_BACKTRACE=1` is set, either via the CLI or exported `$PATH`. This is useful when providing detailed information for debugging purposes: +It's now possible to provide the stacktrace for a fatal error. The stacktrace is only available when the environment variable `RUST_BACKTRACE=1` is set, either inline on the CLI or exported in your shell environment. This is useful when providing detailed information for debugging purposes:.changeset/full-berries-follow.md-5-41 (1)
5-41:⚠️ Potential issue | 🟡 MinorInclude a before/after diff snippet for the formatting change.
That makes the formatter impact obvious at a glance.✍️ Suggested diff-style example
+```diff -const PeopleCountQuery = gql`query PeopleCount { allPeople { totalCount } }`; +const PeopleCountQuery = gql` + query PeopleCount { + allPeople { + totalCount + } + } +`; +```Based on learnings, "For formatter changes in changesets, show formatting changes using diff code blocks".
.changeset/tiny-dodos-ask.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorUse the required bug-fix prefix with an issue link.
Please follow the standard format for bug fixes.✍️ Template update (replace with the real issue)
-Fixed an issue where some info diagnostics weren't tracked by the final summary. +Fixed [`#NNNN`](https://github.com/biomejs/biome/issues/NNNN): some info diagnostics weren't tracked by the final summary.As per coding guidelines, "For bug fixes, start with 'Fixed [
#NUMBER](issue link): ...'"..changeset/eight-bars-unite.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorAdd an invalid example for the new lint rule.
Please include a short invalid-case snippet so users immediately see what triggers the rule.✍️ Proposed addition
Added the rule [`useValidLang`](https://biomejs.dev/linter/rules/use-valid-lang) to the HTML language. + +**Invalid example** + +```html +<html lang="invalid"></html> +```Based on learnings, "For new lint rules in changesets, show an example of invalid case in inline code or code block".
.changeset/tangy-dingos-design.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorMinor grammatical correction needed.
The phrasing "the order how Biome will attempt the configuration file is" reads awkwardly.
📝 Suggested fix
-Added the ability to load the hidden files `.biome.json` and `.biome.jsonc`. This is the order how Biome will attempt the configuration file is: +Added the ability to load the hidden files `.biome.json` and `.biome.jsonc`. The order in which Biome attempts to load configuration files is:crates/biome_cli/tests/cases/reporter_sarif.rs-172-195 (1)
172-195:⚠️ Potential issue | 🟡 MinorTest name/command mismatch. The case says “check command file” but invokes
format, so check+reporter‑file isn’t actually covered. I’d switch the CLI verb tocheck(or rename the test if format was intended).✏️ Proposed fix
- "format", + "check",crates/biome_analyze/src/shared/class_dedup.rs-25-63 (1)
25-63:⚠️ Potential issue | 🟡 MinorType mismatch between
find_duplicate_classesandduplicate_classes_diagnostic.
find_duplicate_classesreturnsduplicates: Vec<&'a str>butduplicate_classes_diagnosticexpectsduplicates: &[Box<str>]. Callers will need to convert the borrowed slices to boxed strings before calling the diagnostic function, which seems like unnecessary allocation.Consider aligning the signatures:
Option A: Accept borrowed slices in diagnostic
pub fn duplicate_classes_diagnostic( category: &'static Category, span: impl AsSpan, - duplicates: &[Box<str>], + duplicates: &[&str], ) -> RuleDiagnostic {Option B: Use a generic bound
pub fn duplicate_classes_diagnostic( category: &'static Category, span: impl AsSpan, - duplicates: &[Box<str>], + duplicates: &[impl AsRef<str>], ) -> RuleDiagnostic {crates/biome_cli/src/runner/impls/commands/traversal.rs-120-120 (1)
120-120:⚠️ Potential issue | 🟡 MinorTypo in doc comment.
CommanderRunnershould beCommandRunner.📝 Suggested fix
- /// Alias of [CommanderRunner::get_files_to_process] + /// Alias of [CommandRunner::get_files_to_process]crates/biome_cli/src/runner/impls/commands/traversal.rs-65-69 (1)
65-69:⚠️ Potential issue | 🟡 MinorDoc comment doesn't match the item.
The comment states "A trait that returns a [TraverseResult]" but this is a
struct, not a trait.📝 Suggested fix
-/// A trait that returns a [TraverseResult] from a traversal command. +/// A wrapper struct that adapts a [TraversalCommand] to [CommandRunner]. pub(crate) struct TraversalCommandImpl<C, P>(pub C)
🧹 Nitpick comments (28)
.changeset/smooth-dots-swim.md (2)
5-16: Make the changeset description more user‑focused.Right now it reads like internal plumbing. Consider framing it around user impact (what to enable, why they care) in 1–3 sentences.
Suggested rewrite
-Added the new linter domain `types`. This is a domain that enables all rules that require the type inference engine. - -As opposed to the `project` domain, which only enables rules that require the module graph to function. - -The following **nursery** rules have been moved to the `types` domain: +Added a new `types` domain so you can opt into type‑inference‑based rules only when you want them. Several **nursery** rules now live under `types`, so enable that domain to run them: - `useArraySortCompare` - `useAwaitThenable` - `useFind` - `useRegexpExec` - `noUnnecessaryConditions` - `noMisusedPromises` - `noFloatingPromises`
9-16: If any moved rules rely on default severity, mention it here.If these rules don’t explicitly set severity, please add a short note that the default is
Severity::Informationto avoid confusion. Based on learnings "In changeset markdown files (e.g., .changeset/*), document that when a lint rule does not explicitly set the severity in declare_lint_rule!, the default severity is Severity::Information (RuleMetadata::default()), not Severity::Error.".changeset/fix-diagnostic-offset-in-embedded-files.md (1)
5-5: Consider splitting the description for improved readability.Whilst technically compliant, the 82-word sentence could be split into two for easier scanning. For example:
"Fixed
#9057: Incorrect diagnostic spans for suppression comments and other raw diagnostics in HTML-ish files (Vue, Svelte, Astro). Previously, diagnostics like "unused suppression" pointed to the wrong location due to the diagnostic offset not being applied."As per coding guidelines, changeset descriptions should be concise (1-3 sentences) and user-focused.
.changeset/bright-foxes-glow.md (1)
5-30: Trim the changeset to 1–3 user‑focused sentences.This reads like mini‑docs (bullets + examples). The guideline asks for concise, end‑user impact, not how‑to content. As per coding guidelines, “Write changeset descriptions for end users, explaining impact and why they care, not implementation details. Be concise (1–3 sentences).”
.changeset/solid-maps-relax.md (1)
5-8: Consider rephrasing for a more user-focused description.The description is clear and concise, but could be slightly more user-focused. Consider: "The
noSvgWithoutTitlerule now supportsgraphics-documentandgraphics-symbolroles, as well as multiple role specifications." This emphasises the benefit to users rather than the implementation action.✍️ Suggested rewording
-Added two new behaviors to the `noSvgWithoutTitle` rule. +The `noSvgWithoutTitle` rule now supports additional accessibility roles and configurations: -- Support for `graphics-document` and `graphics-symbol` roles. -- Support for multiple role specifications. +- `graphics-document` and `graphics-symbol` roles +- Multiple role specificationsAs per coding guidelines: changeset descriptions should be written for end users, explaining impact and why they care, not implementation details.
.changeset/witty-coats-serve.md (1)
5-5: Consider mentioning accessibility benefits.The description is concise and clear, but could be more user-focused by explaining why this rule matters. SVG title elements improve accessibility for screen reader users.
✨ Suggested enhancement
-Added the [`noSvgWithoutTitle`](https://biomejs.dev/linter/rules/no-svg-without-title/) lint rule to HTML. The rule enforces the usage of the `title` element for the `svg` element. +Added the [`noSvgWithoutTitle`](https://biomejs.dev/linter/rules/no-svg-without-title/) lint rule to HTML. The rule enforces the usage of the `title` element for the `svg` element to improve accessibility for screen reader users.As per coding guidelines: "Write changeset descriptions for end users, explaining impact and why they care, not implementation details."
.gitignore (1)
19-19: LGTM – consider broader pattern for consistency.The pattern works, though it's more specific than similar entries. If you want to ignore the entire
.claude/directory (like.vscode/*and.zed/*above), consider/.claude/*instead..changeset/calm-goats-talk.md (1)
5-5: Make the description explicitly user‑focused (why it matters).Consider adding a short impact statement, e.g. accessibility benefits or clearer embeds, rather than just “adds rule”.
As per coding guidelines: “Write changeset descriptions for end users, explaining impact and why they care.”.changeset/petite-mice-say.md (1)
5-5: Minor formatting: Remove backticks from the issue reference.The issue reference uses backticks inside the markdown link:
[#7912]. Standard changeset format is[#7912]without the backticks.Suggested adjustment
-Fixed [`#7912`](https://github.com/biomejs/biome/issues/7912), where Biome incorrectly added a leading newline to the code contained inside the Astro frontmatter. +Fixed [`#7912`](https://github.com/biomejs/biome/issues/7912), where Biome incorrectly added a leading newline to the code contained inside the Astro frontmatter.As per coding guidelines: Create changeset files with YAML front matter and user-focused descriptions. For bug fixes, start with 'Fixed [
#NUMBER](issue link): ...'..changeset/proud-ends-hear.md (1)
11-21: Consider using a diff block to show the formatting suppression.Based on learnings, formatter changes in changesets should demonstrate formatting changes using diff code blocks. A before/after comparison would more clearly illustrate how the suppression prevents formatting.
♻️ Example with diff showing suppressed formatting
Replace the current code block with:
Without the suppression comment, Biome would format the code like this: ```diff -const arr1 = [ ] +const arr1 = [] -const arr2 = [ ] +const arr2 = [] ``` With `biome-ignore-all format: generated`, the original spacing is preserved: ```js // biome-ignore-all format: generated const arr1 = [ ] const arr2 = [ ] ```.changeset/no-duplicate-classes-rule.md (1)
5-17: Consider condensing the description slightly.The changeset is clear and informative, but the guidelines recommend 1-3 sentences for conciseness. The current version has four sentences plus a code example. As per coding guidelines, changeset descriptions should be concise whilst explaining impact.
You might condense it to something like:
Added the [`noDuplicateClasses`](https://biomejs.dev/assist/actions/no-duplicate-classes/) assist action to detect and remove duplicate CSS classes. Supports JSX (`class`, `className`, and utility functions like `clsx`, `cn`, `cva`) and HTML (`class` attributes) — the first assist action for HTML. ```jsx // Before <div class="flex p-4 flex" />; // After <div class="flex p-4" />;That said, your current version provides valuable context about the scope across JSX and HTML, so this is more of a style refinement. </blockquote></details> <details> <summary>.changeset/angry-women-accept.md (1)</summary><blockquote> `5-5`: **Consider "can parse" instead of "is now able to parse" for brevity.** As a minor style improvement, "can now parse" is more concise than "is now able to parse". <details> <summary>✍️ Optional style improvement</summary> ```diff -The Biome CSS parser is now able to parse Vue SFC syntax such as `:slotted` and `:deep`. +The Biome CSS parser can now parse Vue SFC syntax such as `:slotted` and `:deep`.
crates/biome_cli/src/runner/process_file.rs (1)
64-69: Consider removing or documenting commented-out code.If
DiagnosticsWithActionsis planned for future use, aTODOcomment would clarify intent. Otherwise, removing dead code keeps the codebase tidy.
.changeset/smooth-clocks-change.md (1)
5-5: Consider highlighting the user benefit.The description is functional, but could emphasise why this matters to users—improved accessibility, better UX, or alignment with modern web standards.
✨ Optional enhancement
-Added the `noDistractingElements` lint rule for HTML. The rule enforces that no distracting elements like `<marquee>` or `<blink>` are used. +Added the `noDistractingElements` lint rule for HTML to improve accessibility and user experience by disallowing distracting elements like `<marquee>` and `<blink>`.
.changeset/silent-turtles-walk.md (1)
5-5: Add documentation link for the rule.As per coding guidelines, rules should include links to their documentation. Consider adding the link like other changesets in this PR.
📝 Suggested fix
-Added the `useAriaPropsForRole` lint rule for HTML. The rule enforces that elements with ARIA roles must have all required ARIA attributes for that role. +Added the [`useAriaPropsForRole`](https://biomejs.dev/linter/rules/use-aria-props-for-role/) lint rule for HTML. The rule enforces that elements with ARIA roles must have all required ARIA attributes for that role.
.github/workflows/benchmark_html.yml (1)
10-18: Minor path filter redundancy.The glob pattern
crates/biome_html_*/**/*.rs(lines 15, 28) already covers the specific paths forbiome_html_analyze,biome_html_formatter, andbiome_html_parserlisted above it. These explicit paths are technically redundant.That said, keeping them explicit may aid readability by showing exactly which crates are benchmarked. Your call on whether to simplify.
Also applies to: 23-31
crates/biome_cli/src/runner/handler.rs (1)
142-155: Consider counting panics as skipped files.
The panic branch emits a diagnostic but doesn’t update skipped/unchanged counters, so summary stats can under‑report. Mirroring the Err path would keep totals consistent.💡 Suggested tweak
Err(err) => { let message = match err.downcast::<String>() { Ok(msg) => format!("processing panicked: {msg}"), Err(err) => match err.downcast::<&'static str>() { Ok(msg) => format!("processing panicked: {msg}"), Err(_) => String::from("processing panicked"), }, }; ctx.push_message( PanicDiagnostic { message } .with_file_path(biome_path.to_string()) .into(), ); + ctx.increment_unchanged(); + ctx.increment_skipped(); }
.changeset/all-pumas-stop.md (1)
31-31: Minor grammar nit.The word "otherwise" is an adverb and reads better when separated from the sentence. Consider:
Suggested fix
-**The `--reporter` and `--reporter-file` flags must appear next to each other, otherwise an error is thrown.** +**The `--reporter` and `--reporter-file` flags must appear next to each other; otherwise, an error is thrown.**
crates/biome_analyze/src/analyzer_plugin.rs (1)
105-109: Consider using the plugin's actual name for profiling.The timer label
"plugin"is generic. For more useful profiling data, consider extracting the plugin's name or identifier. However, if all plugin rules should aggregate under one label, this is fine as-is.
crates/biome_cli/tests/cases/reporter_terminal.rs (1)
141-142: Function name may be misleading.The test is named
reports_diagnostics_check_write_command_filebut doesn't use the--writeflag (unlikereports_diagnostics_check_write_command_verboseabove it). Perhapsreports_diagnostics_check_command_reporter_fileor similar would better reflect what's being tested?
crates/biome_cli/src/runner/run.rs (1)
6-14: Consider simplifying the mutable rebinding.The
let command = &mut command;on line 12 is redundant sincecommandis already declared asmut. You could callrundirectly:♻️ Optional simplification
pub(crate) fn run_command( session: CliSession, log_options: &LogOptions, cli_options: &CliOptions, mut command: impl CommandRunner, ) -> Result<(), CliDiagnostic> { - let command = &mut command; command.run(session, log_options, cli_options) }
crates/biome_cli/src/runner/impls/crawlers/default.rs (1)
12-60: Consider usingPhantomDatainstead of storing the value.
DefaultCrawler<P>(P)stores a value of typeP, but it's never accessed -Pis only used at the type level for theProcessFileassociated type. This could beDefaultCrawler<P>(std::marker::PhantomData<P>)to make the intent clearer and avoid storing an unused value.That said, this is a minor point and doesn't affect correctness.
Optional refactor
+use std::marker::PhantomData; + -pub(crate) struct DefaultCrawler<P>(P); +pub(crate) struct DefaultCrawler<P>(PhantomData<P>);
crates/biome_cli/src/runner/impls/commands/traversal.rs (1)
35-39: Editorconfig diagnostics are silently discarded.The
_editorconfig_diagnosticsvariable is captured but never used. If these diagnostics contain parsing warnings or errors, users won't see them.Consider logging or propagating these diagnostics if they're meaningful.
crates/biome_cli/src/runner/impls/process_file/format.rs (2)
40-42: TODO comment left in code.The comment
// NOTE: probably to revisiton thefalseparameter suggests uncertainty. Consider clarifying what this boolean controls and whether it needs revisiting.Would you like me to open an issue to track this for follow-up?
81-104: Consider extracting the HTML handler output logic.The Astro/Vue/Svelte output transformation is duplicated in both
process_file(lines 81-104) andprocess_std_in(lines 183-192). A helper function could reduce this duplication.
crates/biome_cli/src/commands/migrate.rs (1)
146-147: Clarify the comment - this isn't a Rust SAFETY invariant.The
// SAFETY:convention in Rust typically documents whyunsafecode is sound. Here it's documenting logical correctness (thatget_executionchecks forSome). Consider rephrasing:📝 Suggested fix
- // SAFETY: checked during get_execution + // Invariant: get_execution returns Err if configuration_file_path is None configuration_file_path: self.configuration_file_path.clone().unwrap(),
crates/biome_cli/src/commands/search.rs (1)
152-153: Consider defensive handling for search_pattern.The
// SAFETYcomment is reasonable, but relying on the caller always beingSearchExecutionis fragile if refactoring occurs. A debug assertion orexpect()with a descriptive message would make failures more diagnosable.Suggested improvement
- // SAFETY: search_pattern is implemented in this file - let pattern = execution.search_pattern().unwrap(); + let pattern = execution + .search_pattern() + .expect("SearchProcessFile requires an execution with a search pattern");
crates/biome_cli/src/reporter/github.rs (1)
54-62: Simplify redundant condition.In the else branch,
diagnostic.tags().is_verbose()is guaranteed to be true (you wouldn't reach the else otherwise), so the&& verbosecheck can be simplified.Proposed simplification
for diagnostic in &diagnostics_payload.diagnostics { if diagnostic.severity() >= diagnostics_payload.diagnostic_level { - if !diagnostic.tags().is_verbose() { - writer.log(markup! {{PrintGitHubDiagnostic(diagnostic)}}); - } else if diagnostic.tags().is_verbose() && verbose { + let is_verbose_diag = diagnostic.tags().is_verbose(); + if !is_verbose_diag || verbose { writer.log(markup! {{PrintGitHubDiagnostic(diagnostic)}}); } } }
| --- | ||
| "@biomejs/biome": minor | ||
| --- |
There was a problem hiding this comment.
Use a patch changeset on main (unless this is staying on next).
The front matter marks this as minor, which conflicts with the guidance for changesets targeting main. If this merge PR is landing on main, please switch to patch, or confirm this changeset remains only on next. As per coding guidelines, “Use 'patch' for bug fixes and non-breaking changes targeting the main branch, 'minor' for new features targeting the next branch, and 'major' for breaking changes targeting the next branch.”
🤖 Prompt for AI Agents
In @.changeset/bright-foxes-glow.md around lines 1 - 3, The changeset front
matter currently marks the release type as "minor" which conflicts with
main-branch guidance; update the front matter in .changeset/bright-foxes-glow.md
to use "patch" if this PR is targeting main, or keep "minor" only if you confirm
this changeset will remain on the next release branch; ensure the YAML key
"@biomejs/biome": minor is changed to "@biomejs/biome": patch when targeting
main and commit that change.
| --- | ||
| "@biomejs/biome": minor | ||
| --- | ||
|
|
||
| Formatting is now applied when applying safe/unsafe fixes via `biome check`. |
There was a problem hiding this comment.
Please confirm the change type versus target branch.
This PR targets main, yet the changeset declares a minor release. Our guidance says minor is for new features targeting next, while main should be patch only. If this is a user-facing feature (it reads as such), consider keeping it in next or confirming the release policy before landing on main. As per coding guidelines: “Use 'patch' for bug fixes and non-breaking changes targeting the main branch, 'minor' for new features targeting the next branch, and 'major' for breaking changes targeting the next branch.”
🤖 Prompt for AI Agents
In @.changeset/tricky-masks-cry.md around lines 1 - 5, The changeset for
"@biomejs/biome" declares a "minor" release but the PR targets main; either
update the changeset change type from "minor" to "patch" or retarget this PR to
the next branch; confirm whether the change is a user-facing new feature (keep
"minor" and move the branch to next) or a non-breaking bugfix (change the header
to "patch") and update the changeset accordingly so it matches the repository
release policy.
| impl Visit for SuggestionsVisitor { | ||
| fn record_log(&mut self, _category: LogCategory, text: &dyn Display) -> std::io::Result<()> { | ||
| let message = { | ||
| let mut message = MarkupBuf::default(); | ||
| let mut fmt = Formatter::new(&mut message); | ||
| fmt.write_markup(markup!({ { text } }))?; | ||
| markup_to_string(&message).expect("Invalid markup") | ||
| }; | ||
| let current_diagnostic_length = self.suggestions.len(); | ||
|
|
||
| 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; | ||
| } else if let Some(current_message) = self.current_message.as_mut() { | ||
| current_message.push_str(&message); | ||
| } else { | ||
| self.current_message = Some(message); | ||
| } |
There was a problem hiding this comment.
Suggestion logs can overwrite each other. last_diagnostic_length never updates, so any log after the first frame keeps clobbering the last suggestion text and loses earlier messages.
🔧 Suggested fix
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;
+ self.last_diagnostic_length = current_diagnostic_length;
} else if let Some(current_message) = self.current_message.as_mut() {
current_message.push_str(&message);
} else {
self.current_message = Some(message);
} 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");
self.suggestions.push(JsonSuggestion {
@@
text: self.current_message.take().unwrap_or_default(),
})
+ self.last_diagnostic_length = self.suggestions.len();
}📝 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.
| impl Visit for SuggestionsVisitor { | |
| fn record_log(&mut self, _category: LogCategory, text: &dyn Display) -> std::io::Result<()> { | |
| let message = { | |
| let mut message = MarkupBuf::default(); | |
| let mut fmt = Formatter::new(&mut message); | |
| fmt.write_markup(markup!({ { text } }))?; | |
| markup_to_string(&message).expect("Invalid markup") | |
| }; | |
| let current_diagnostic_length = self.suggestions.len(); | |
| 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; | |
| } else if let Some(current_message) = self.current_message.as_mut() { | |
| current_message.push_str(&message); | |
| } else { | |
| self.current_message = Some(message); | |
| } | |
| impl Visit for SuggestionsVisitor { | |
| fn record_log(&mut self, _category: LogCategory, text: &dyn Display) -> std::io::Result<()> { | |
| let message = { | |
| let mut message = MarkupBuf::default(); | |
| let mut fmt = Formatter::new(&mut message); | |
| fmt.write_markup(markup!({ { text } }))?; | |
| markup_to_string(&message).expect("Invalid markup") | |
| }; | |
| let current_diagnostic_length = self.suggestions.len(); | |
| 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; | |
| self.last_diagnostic_length = current_diagnostic_length; | |
| } else if let Some(current_message) = self.current_message.as_mut() { | |
| current_message.push_str(&message); | |
| } else { | |
| self.current_message = Some(message); | |
| } |
🤖 Prompt for AI Agents
In `@crates/biome_cli/src/reporter/json.rs` around lines 387 - 407, The bug is
that SuggestionsVisitor::record_log overwrites previous suggestion text because
self.last_diagnostic_length is never updated; after you append to the last
suggestion or otherwise mutate suggestions you should set
self.last_diagnostic_length to the current number of diagnostics so future logs
branch correctly. In the record_log method (SuggestionsVisitor::record_log)
update self.last_diagnostic_length = current_diagnostic_length immediately after
the block that mutates self.suggestions (i.e., after the last_suggestion.text =
message assignment), and ensure any code path that creates a new suggestion or
consumes current_message also updates last_diagnostic_length accordingly so
subsequent calls don't clobber earlier entries.
| if diagnostic.severity() >= diagnostics_payload.diagnostic_level { | ||
| if diagnostic.tags().is_verbose() && verbose { | ||
| self.0 | ||
| .error(markup! {{PrintDiagnostic::verbose(diagnostic)}}); | ||
| writer.error(markup! {{PrintDiagnostic::verbose(diagnostic)}}); | ||
| } else { | ||
| self.0 | ||
| .error(markup! {{PrintDiagnostic::simple(diagnostic)}}); | ||
| writer.error(markup! {{PrintDiagnostic::simple(diagnostic)}}); | ||
| } |
There was a problem hiding this comment.
Verbose-only diagnostics are always printed. The current branch prints verbose-tagged diagnostics even when verbose is false (falls through to simple). That changes CLI behaviour.
🔧 Suggested fix
- if diagnostic.tags().is_verbose() && verbose {
- writer.error(markup! {{PrintDiagnostic::verbose(diagnostic)}});
- } else {
- writer.error(markup! {{PrintDiagnostic::simple(diagnostic)}});
- }
+ if diagnostic.tags().is_verbose() {
+ if verbose {
+ writer.error(markup! {{PrintDiagnostic::verbose(diagnostic)}});
+ } else {
+ continue;
+ }
+ } else {
+ writer.error(markup! {{PrintDiagnostic::simple(diagnostic)}});
+ }📝 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.
| if diagnostic.severity() >= diagnostics_payload.diagnostic_level { | |
| if diagnostic.tags().is_verbose() && verbose { | |
| self.0 | |
| .error(markup! {{PrintDiagnostic::verbose(diagnostic)}}); | |
| writer.error(markup! {{PrintDiagnostic::verbose(diagnostic)}}); | |
| } else { | |
| self.0 | |
| .error(markup! {{PrintDiagnostic::simple(diagnostic)}}); | |
| writer.error(markup! {{PrintDiagnostic::simple(diagnostic)}}); | |
| } | |
| if diagnostic.tags().is_verbose() { | |
| if verbose { | |
| writer.error(markup! {{PrintDiagnostic::verbose(diagnostic)}}); | |
| } else { | |
| continue; | |
| } | |
| } else { | |
| writer.error(markup! {{PrintDiagnostic::simple(diagnostic)}}); | |
| } |
🤖 Prompt for AI Agents
In `@crates/biome_cli/src/reporter/terminal.rs` around lines 154 - 159, The code
currently falls through and prints verbose-tagged diagnostics even when verbose
is false; update the conditional in the loop that checks diagnostic.severity()
so that verbose-tagged diagnostics are only printed when verbose is true: if
diagnostic.tags().is_verbose() then print PrintDiagnostic::verbose(diagnostic)
when verbose is true and otherwise skip that diagnostic (do not fall through to
PrintDiagnostic::simple), else print PrintDiagnostic::simple(diagnostic). Locate
the branch around diagnostic.severity(), diagnostic.tags().is_verbose(),
writer.error and PrintDiagnostic::verbose/simple and implement the explicit skip
(e.g., continue) for non-verbose context.
| let configured_workspace = self.configure_workspace(fs, console, workspace, cli_options)?; | ||
|
|
||
| // Commands that don't require crawling can implement custom execution logic | ||
| if !self.requires_crawling() { | ||
| return self.execute_without_crawling(session, configured_workspace); | ||
| } | ||
|
|
||
| let ConfiguredWorkspace { | ||
| execution, | ||
| paths, | ||
| duration, | ||
| configuration_files: _, | ||
| project_key, | ||
| } = configured_workspace; | ||
|
|
||
| if let Some(stdin) = self.get_stdin(console, execution.as_ref())? { | ||
| let biome_path = BiomePath::new(stdin.as_path()); | ||
| if biome_path.extension().is_none() { | ||
| console.error(markup! { | ||
| {PrintDiagnostic::simple(&CliDiagnostic::from(StdinDiagnostic::new_no_extension()))} | ||
| }); | ||
| console.append(markup! {{stdin.as_content()}}); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| return Self::ProcessFile::process_std_in(ProcessStdinFilePayload { | ||
| biome_path: &biome_path, | ||
| project_key, | ||
| workspace, | ||
| execution: execution.as_ref(), | ||
| content: stdin.as_content(), | ||
| cli_options, | ||
| console, | ||
| skip_ignore_check: Self::ProcessFile::should_skip_ignore_check( | ||
| &biome_path, | ||
| workspace, | ||
| ), | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Stdin is read twice, so the second read may be empty. Line 405 reads stdin for scan-kind derivation, then Line 279 reads it again for processing. If Console::read() consumes the stream, stdin handling silently skips. Consider reading once and threading it through.
🔧 Suggested fix (store stdin in ConfiguredWorkspace)
- let ConfiguredWorkspace {
+ let ConfiguredWorkspace {
execution,
paths,
duration,
configuration_files: _,
project_key,
+ stdin,
} = configured_workspace;
- if let Some(stdin) = self.get_stdin(console, execution.as_ref())? {
+ if let Some(stdin) = stdin {
let biome_path = BiomePath::new(stdin.as_path());
if biome_path.extension().is_none() {
console.error(markup! {
{PrintDiagnostic::simple(&CliDiagnostic::from(StdinDiagnostic::new_no_extension()))}
}); pub(crate) struct ConfiguredWorkspace {
@@
/// The unique identifier of the project
pub project_key: ProjectKey,
+ /// Captured stdin (if any)
+ pub stdin: Option<Stdin>,
}- let stdin = self.get_stdin(console, execution.as_ref())?;
+ let stdin = self.get_stdin(console, execution.as_ref())?;
@@
- Ok(ConfiguredWorkspace {
+ Ok(ConfiguredWorkspace {
execution,
paths,
duration: Some(result.duration),
configuration_files: result.configuration_files,
project_key: open_project_result.project_key,
+ stdin,
})📝 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 configured_workspace = self.configure_workspace(fs, console, workspace, cli_options)?; | |
| // Commands that don't require crawling can implement custom execution logic | |
| if !self.requires_crawling() { | |
| return self.execute_without_crawling(session, configured_workspace); | |
| } | |
| let ConfiguredWorkspace { | |
| execution, | |
| paths, | |
| duration, | |
| configuration_files: _, | |
| project_key, | |
| } = configured_workspace; | |
| if let Some(stdin) = self.get_stdin(console, execution.as_ref())? { | |
| let biome_path = BiomePath::new(stdin.as_path()); | |
| if biome_path.extension().is_none() { | |
| console.error(markup! { | |
| {PrintDiagnostic::simple(&CliDiagnostic::from(StdinDiagnostic::new_no_extension()))} | |
| }); | |
| console.append(markup! {{stdin.as_content()}}); | |
| return Ok(()); | |
| } | |
| return Self::ProcessFile::process_std_in(ProcessStdinFilePayload { | |
| biome_path: &biome_path, | |
| project_key, | |
| workspace, | |
| execution: execution.as_ref(), | |
| content: stdin.as_content(), | |
| cli_options, | |
| console, | |
| skip_ignore_check: Self::ProcessFile::should_skip_ignore_check( | |
| &biome_path, | |
| workspace, | |
| ), | |
| }); | |
| } | |
| let configured_workspace = self.configure_workspace(fs, console, workspace, cli_options)?; | |
| // Commands that don't require crawling can implement custom execution logic | |
| if !self.requires_crawling() { | |
| return self.execute_without_crawling(session, configured_workspace); | |
| } | |
| let ConfiguredWorkspace { | |
| execution, | |
| paths, | |
| duration, | |
| configuration_files: _, | |
| project_key, | |
| stdin, | |
| } = configured_workspace; | |
| if let Some(stdin) = stdin { | |
| let biome_path = BiomePath::new(stdin.as_path()); | |
| if biome_path.extension().is_none() { | |
| console.error(markup! { | |
| {PrintDiagnostic::simple(&CliDiagnostic::from(StdinDiagnostic::new_no_extension()))} | |
| }); | |
| console.append(markup! {{stdin.as_content()}}); | |
| return Ok(()); | |
| } | |
| return Self::ProcessFile::process_std_in(ProcessStdinFilePayload { | |
| biome_path: &biome_path, | |
| project_key, | |
| workspace, | |
| execution: execution.as_ref(), | |
| content: stdin.as_content(), | |
| cli_options, | |
| console, | |
| skip_ignore_check: Self::ProcessFile::should_skip_ignore_check( | |
| &biome_path, | |
| workspace, | |
| ), | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@crates/biome_cli/src/runner/mod.rs` around lines 264 - 303, The code reads
stdin twice which can consume the stream and make the second read empty; update
configure_workspace/ConfiguredWorkspace to capture and return an optional Stdin
payload (the result of self.get_stdin/Console::read) so the workspace carries
the already-read stdin, then change the branch that currently calls
self.get_stdin(console, execution.as_ref()) to use the stored stdin from
ConfiguredWorkspace; pass that stored stdin into ProcessFile::process_std_in via
ProcessStdinFilePayload (and still call ProcessFile::should_skip_ignore_check
with the biome_path and workspace) so stdin is only read once and reused for
scan-kind derivation and processing.
20 participants
Summary
Caution
main->next.Test Plan
Docs