-
-
Notifications
You must be signed in to change notification settings - Fork 790
chore: expose preferred indentation #7540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughAdds PreferredIndentation (Tab | Spaces(u8)) to analyser options and threads it through configuration, registry, signals, and RuleContext. RuleContext now stores PreferredQuote and PreferredJsxQuote by value and exposes preferred_indentation(), preferred_quote(), and preferred_jsx_quote() value-returning accessors. Options and service map formatter settings to these analyser preferences; JavaScript handler now sets preferred_indentation and refactors quote-style mapping. Multiple JS lint rules switch from as_preferred_quote()/as_preferred_jsx_quote() to the new accessors. Removes an empty public Rules::get_rule method in configuration. Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/biome_analyze/src/options.rs (2)
203-223: Avoid inherentto_string(); sidestep MSRV pitfalls.
- An inherent method named
to_string()returningCow<str>is surprising; callers will expectString. Preferas_str()(returningCow<str>) orDisplayimpl.std::iter::repeat_nmay exceed your MSRV;" ".repeat(n)is clearer and stable.Proposed tweak:
impl PreferredIndentation { - /// Returns the indentation in its string form. - pub fn to_string(self) -> Cow<'static, str> { + /// Returns the indentation in its string form without allocating for tabs. + pub fn as_str(self) -> Cow<'static, str> { match self { Self::Tab => Cow::Borrowed("\t"), - Self::Spaces(tab_width) => { - Cow::Owned(std::iter::repeat_n(' ', tab_width as usize).collect()) - } + Self::Spaces(tab_width) => Cow::Owned(" ".repeat(tab_width as usize)), } } }Optionally, also implement
Display(non‑blocking):impl std::fmt::Display for PreferredIndentation { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::Tab => f.write_str("\t"), Self::Spaces(w) => f.write_str(&" ".repeat(*w as usize)), } } }
66-71: Minor visibility nit: keep configuration fields consistent.
preferred_jsx_quoteispubwhereaspreferred_quoteandpreferred_indentationare private. Consider aligning visibility (all private with accessors).crates/biome_service/src/file_handlers/javascript.rs (1)
263-266: Clean refactor of quote style mapping.The match expression is more explicit and maintainable than the previous conditional logic. Good improvement.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
crates/biome_analyze/src/context.rs(5 hunks)crates/biome_analyze/src/options.rs(4 hunks)crates/biome_analyze/src/registry.rs(2 hunks)crates/biome_analyze/src/signals.rs(4 hunks)crates/biome_configuration/src/analyzer/linter/mod.rs(0 hunks)crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rs(1 hunks)crates/biome_js_analyze/src/lint/correctness/no_string_case_mismatch.rs(1 hunks)crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs(1 hunks)crates/biome_js_analyze/src/lint/correctness/use_valid_typeof.rs(1 hunks)crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs(3 hunks)crates/biome_js_analyze/src/lint/style/no_unused_template_literal.rs(1 hunks)crates/biome_js_analyze/src/lint/style/use_enum_initializers.rs(1 hunks)crates/biome_js_analyze/src/lint/suspicious/use_strict_mode.rs(1 hunks)crates/biome_service/src/file_handlers/javascript.rs(3 hunks)
💤 Files with no reviewable changes (1)
- crates/biome_configuration/src/analyzer/linter/mod.rs
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rscrates/biome_js_analyze/src/lint/correctness/use_valid_typeof.rscrates/biome_js_analyze/src/lint/correctness/use_import_extensions.rscrates/biome_js_analyze/src/lint/suspicious/use_strict_mode.rscrates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/src/lint/style/no_unused_template_literal.rscrates/biome_js_analyze/src/lint/style/use_enum_initializers.rscrates/biome_js_analyze/src/lint/correctness/no_string_case_mismatch.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rscrates/biome_js_analyze/src/lint/correctness/use_valid_typeof.rscrates/biome_js_analyze/src/lint/correctness/use_import_extensions.rscrates/biome_js_analyze/src/lint/suspicious/use_strict_mode.rscrates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_analyze/src/registry.rscrates/biome_js_analyze/src/lint/style/no_unused_template_literal.rscrates/biome_js_analyze/src/lint/style/use_enum_initializers.rscrates/biome_analyze/src/signals.rscrates/biome_analyze/src/context.rscrates/biome_js_analyze/src/lint/correctness/no_string_case_mismatch.rscrates/biome_analyze/src/options.rscrates/biome_service/src/file_handlers/javascript.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rscrates/biome_js_analyze/src/lint/correctness/use_valid_typeof.rscrates/biome_js_analyze/src/lint/correctness/use_import_extensions.rscrates/biome_js_analyze/src/lint/suspicious/use_strict_mode.rscrates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_analyze/src/registry.rscrates/biome_js_analyze/src/lint/style/no_unused_template_literal.rscrates/biome_js_analyze/src/lint/style/use_enum_initializers.rscrates/biome_analyze/src/signals.rscrates/biome_analyze/src/context.rscrates/biome_js_analyze/src/lint/correctness/no_string_case_mismatch.rscrates/biome_analyze/src/options.rscrates/biome_service/src/file_handlers/javascript.rs
🧠 Learnings (12)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Avoid avoidable string allocations: prefer comparing `&str` or using `TokenText` over calling `to_string()`
Applied to files:
crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rscrates/biome_js_analyze/src/lint/correctness/use_valid_typeof.rscrates/biome_js_analyze/src/lint/style/no_unused_template_literal.rscrates/biome_js_analyze/src/lint/correctness/no_string_case_mismatch.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings
Applied to files:
crates/biome_js_analyze/src/lint/correctness/use_valid_typeof.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : If a rule returns a code action (implements `action`), add `fix_kind` in `declare_lint_rule!` and use `ctx.metadata().applicability()` when building the action
Applied to files:
crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rscrates/biome_analyze/src/registry.rscrates/biome_analyze/src/signals.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options types in `biome_rule_options` crate (one file per rule, e.g., `lib/use_my_rule.rs`)
Applied to files:
crates/biome_analyze/src/registry.rscrates/biome_analyze/src/signals.rscrates/biome_analyze/src/context.rscrates/biome_analyze/src/options.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_analyze/src/registry.rscrates/biome_analyze/src/signals.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Avoid deep indentation and panics; prefer `?`, `ok()?`, and combinators (`map`, `and_then`, `filter`) over nested `if let`/`unwrap`/`expect`
Applied to files:
crates/biome_js_analyze/src/lint/style/no_unused_template_literal.rscrates/biome_analyze/src/signals.rscrates/biome_service/src/file_handlers/javascript.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Code blocks in rule docs must specify language; invalid snippets require `expect_diagnostic`; use `options`/`full_options`/`use_options` markers as appropriate
Applied to files:
crates/biome_analyze/src/signals.rscrates/biome_analyze/src/context.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper advice types from biome_diagnostics::v2 (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) when suitable
Applied to files:
crates/biome_analyze/src/signals.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Recommended rules with domains are enabled only when users enable the matching domains; use `domains` metadata judiciously
Applied to files:
crates/biome_analyze/src/signals.rscrates/biome_analyze/src/context.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : When emitting multiple signals, prefer `Box<[_]>` over `Vec<_>` for lower memory usage
Applied to files:
crates/biome_analyze/src/signals.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Follow rule naming conventions: use `no<Concept>` to forbid and `use<Concept>` to mandate; prefer consistent prefixes (e.g., `noDuplicate<Concept>`, `useConsistent<Concept>`)
Applied to files:
crates/biome_analyze/src/signals.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Options types must implement serialization/deserialization and schema support using derives: `Serialize`, `Deserialize`, `Deserializable`, and `#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]`
Applied to files:
crates/biome_analyze/src/context.rs
🧬 Code graph analysis (5)
crates/biome_analyze/src/registry.rs (2)
crates/biome_analyze/src/context.rs (1)
preferred_indentation(186-188)crates/biome_analyze/src/options.rs (1)
preferred_indentation(194-196)
crates/biome_analyze/src/signals.rs (2)
crates/biome_analyze/src/context.rs (1)
preferred_indentation(186-188)crates/biome_analyze/src/options.rs (1)
preferred_indentation(194-196)
crates/biome_analyze/src/context.rs (1)
crates/biome_analyze/src/options.rs (3)
preferred_quote(186-188)preferred_jsx_quote(190-192)preferred_indentation(194-196)
crates/biome_analyze/src/options.rs (2)
crates/biome_analyze/src/context.rs (3)
preferred_indentation(186-188)preferred_quote(176-178)preferred_jsx_quote(181-183)crates/biome_js_formatter/src/context.rs (1)
tab_width(377-379)
crates/biome_service/src/file_handlers/javascript.rs (5)
crates/biome_analyze/src/context.rs (3)
options(149-151)preferred_jsx_quote(181-183)preferred_indentation(186-188)crates/biome_js_type_info/src/format_type_info.rs (2)
options(56-58)indent_style(25-27)crates/biome_js_semantic/src/format_semantic_model.rs (2)
options(45-47)indent_style(14-16)crates/biome_js_formatter/src/context.rs (1)
quote_style(353-355)crates/biome_analyze/src/options.rs (2)
preferred_jsx_quote(190-192)preferred_indentation(194-196)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: autofix
🔇 Additional comments (28)
crates/biome_analyze/src/options.rs (3)
72-74: Good call: threading indentation through configuration.Private field with a sensible default via
Defaulton the enum. No concerns.
108-115: Builder API consistency: 👍
with_preferred_indentation(...)mirrors the existing builders cleanly.
186-196: Approve — by‑value accessors OK; no stale as_preferred_ usages found.*rg shows callers in crates/biome_analyze/{registry.rs, signals.rs, context.rs} use preferred_*() by value; rust toolchain channel is 1.89.0.
crates/biome_js_analyze/src/lint/complexity/use_literal_keys.rs (1)
177-181: API migration topreferred_quote()looks good.No needless allocations; sticking to
TokenTextand factory helpers is spot on.crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs (1)
172-176: LGTM on quote preference usage.Action still respects
ctx.metadata().applicability()andfix_kind: Safe. All good.crates/biome_js_analyze/src/lint/style/no_unused_template_literal.rs (1)
101-105: Consistent quote handling: ✅Switch to
preferred_quote()matches the new API; no behavioural change otherwise.crates/biome_js_analyze/src/lint/correctness/use_valid_typeof.rs (1)
194-198: Looks tidy.Quote selection now relies on the owned enum; replacement remains minimal and safe.
crates/biome_js_analyze/src/lint/correctness/no_string_case_mismatch.rs (1)
105-109: All set on the accessor change.No extra churn; still builds the correct literal variant.
crates/biome_analyze/src/signals.rs (2)
360-372: Plumbing indentation intoRuleContext: nice.Propagation through diagnostics path is correct and mirrors quotes/JSX quotes.
408-409: Consistency across actions/transformations: good coverage.Both paths now receive the indentation preference; no gaps spotted.
Also applies to: 473-474
crates/biome_js_analyze/src/lint/suspicious/use_strict_mode.rs (1)
95-98: Quote choice wired to new API.Small, precise change; behaviour remains as expected.
crates/biome_js_analyze/src/lint/style/use_enum_initializers.rs (1)
166-166: LGTM! Quote preference accessor updated correctly.The change from
ctx.as_preferred_quote()toctx.preferred_quote()aligns with the broader refactor where quote preferences are now returned by value rather than by reference.crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs (3)
217-217: LGTM! Quote preference accessor updated correctly.The change from
ctx.as_preferred_quote().is_double()toctx.preferred_quote().is_double()follows the new API where preferences are returned by value.
226-226: LGTM! Quote preference accessor updated correctly.Updated to use the new
preferred_quote()accessor that returns by value.
237-237: LGTM! JSX quote preference accessor updated correctly.The change from
ctx.as_preferred_jsx_quote().is_double()toctx.preferred_jsx_quote().is_double()aligns with the new API design.crates/biome_analyze/src/registry.rs (2)
407-407: LGTM! Indentation preference added to rule execution.The new
preferred_indentationextraction from options follows the established pattern for other preferences and correctly threads indentation settings into the rule context.
420-420: LGTM! Indentation preference passed to RuleContext.The
preferred_indentationparameter is correctly passed toRuleContext::new, enabling rules to access indentation preferences for code generation.crates/biome_service/src/file_handlers/javascript.rs (4)
21-21: LGTM! Import updated for new preference types.Added
PreferredIndentationto the import alongside the existingPreferredQuote, maintaining consistency with the new indentation preference system.
273-276: Consistent JSX quote style mapping.Applies the same clean pattern as the regular quote style mapping above.
278-292: Well-implemented indentation preference mapping.The logic correctly maps
IndentStyle::TabtoPreferredIndentation::TabandIndentStyle::SpacetoPreferredIndentation::Spaces(u8)with proper fallback to default indent width. The implementation handles the type conversion appropriately by calling.value()on the indent width.
361-362: LGTM! Indentation preference threaded through configuration.The
with_preferred_indentation(preferred_indentation)call properly adds indentation preferences to the analyzer configuration, completing the integration with the existing preference system.crates/biome_analyze/src/context.rs (7)
1-1: LGTM! Import updated for new preference type.Added
PreferredIndentationto the imports, maintaining consistency with the new indentation preference system.
19-21: Good design: preferences stored by value.Storing
preferred_quote,preferred_jsx_quote, andpreferred_indentationas owned values rather than references simplifies the API and eliminates lifetime complexities. This is a sensible design choice.
38-40: Constructor signature updated correctly.The
newmethod now accepts ownedPreferredQuoteandPreferredIndentationvalues, maintaining consistency with the field types.
55-55: Field initialization consistent with new design.The
preferred_indentationfield is correctly initialised with the owned value.
176-177: Accessor returns by value as intended.The
preferred_quote()method now returnsPreferredQuoteby value, which is consistent with the new owned field design.
181-182: JSX quote accessor returns by value.Consistent with the quote preference accessor above.
185-188: New indentation preference accessor.The new
preferred_indentation()method provides access to the indentation setting, completing the preference API. The documentation is clear and follows the existing pattern.
CodSpeed Performance ReportMerging #7540 will not alter performanceComparing Summary
|
Summary
Exposes preferred indentation to the analyzer, for use in #7457.
Test Plan
No tests added yet, since the functionality is not used yet. We can test it with #7457.
Docs
N/A