feat(biome-js-analyze): add useConsistentBooleanProps rule#8120
feat(biome-js-analyze): add useConsistentBooleanProps rule#8120vladimir-ivanov wants to merge 11 commits intobiomejs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 13e833c The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-Project
vladimir-ivanov
changed the title
WalkthroughAdds a nursery JSX lint rule Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 0
🧹 Nitpick comments (2)
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.jsx (1)
1-3: Consider adding more test cases.The current coverage handles comments well but could be more comprehensive. Consider adding:
- Multiple implicit props on the same element
- Nested components with implicit props
- Implicit props in fragments
Example additions:
<input disabled readOnly required />; <MyComponent disabled someProp />; <> <input disabled /> <button accept /> </>;crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (1)
207-213: Use AST node type checking instead of text comparison.The current implementation uses
text_trimmed() == "true", which could theoretically match non-boolean-literal expressions with that text. A more robust approach would verify it's actually aJsBooleanLiteralExpressionwith valuetrue.-fn is_true_literal(jsx_attribute_initializer_clause: &JsxAttributeInitializerClause) -> bool { - if let Ok(expr) = jsx_attribute_initializer_clause.value() - && let Some(lit_expr) = expr.as_jsx_expression_attribute_value() - && let Ok(expression) = lit_expr.expression() - { - return expression.syntax().text_trimmed() == "true"; - } - - false +fn is_true_literal(jsx_attribute_initializer_clause: &JsxAttributeInitializerClause) -> bool { + if let Ok(expr) = jsx_attribute_initializer_clause.value() + && let Some(lit_expr) = expr.as_jsx_expression_attribute_value() + && let Ok(expression) = lit_expr.expression() + && let Some(lit) = expression.as_any_js_literal_expression() + && let Some(bool_lit) = lit.as_js_boolean_literal_expression() + { + return bool_lit.value_token().ok().map_or(false, |t| t.text_trimmed() == "true"); + } + false }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.jsx.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (12)
.changeset/use-consistent-boolean-props.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.options.json(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/use_consistent_boolean_props.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
Repo: biomejs/biome PR: 7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.options.jsoncrates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.options.jsoncrates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.options.jsoncrates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.options.json
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.options.jsoncrates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.options.jsoncrates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.options.jsoncrates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.options.json
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (1)
crates/biome_analyze/src/rule.rs (2)
sources(617-620)inspired(254-259)
⏰ 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). (14)
- GitHub Check: Check JS Files
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: End-to-end tests
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: autofix
🔇 Additional comments (11)
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.jsx (1)
1-41: Test coverage looks solid.The test cases comprehensively cover explicit boolean prop scenarios including edge cases with falsy values, expressions, and nested structures. Well done.
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.options.json (1)
1-15: Configuration looks correct.The JSON structure properly configures the rule for explicit mode testing.
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.options.json (1)
1-15: Configuration looks correct.crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.options.json (1)
1-15: Configuration looks correct.crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.options.json (1)
1-15: Configuration looks correct..changeset/use-consistent-boolean-props.md (1)
1-30: Documentation is clear and helpful.The changeset provides a concise explanation with good examples illustrating both modes.
crates/biome_rule_options/src/lib.rs (1)
1-1: Module declaration verified as auto-generated.Line 273 shows
pub mod use_consistent_boolean_props;in alphabetically-ordered sequence with other rule options modules. This matches the pattern produced by theregister_analyzer_rule_options()function inxtask/codegen/src/generate_analyzer_rule_options.rs, which automatically registers new modules in lib.rs. The corresponding rule options file exists at the expected location. No manual editing required.crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.jsx (1)
1-3: Test cases cover the essentials.The three test cases appropriately exercise explicit
{true}usage (which should be flagged in implicit mode) and include comments in different positions to validate trivia handling.crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.jsx (1)
1-25: Comprehensive test coverage for valid implicit mode cases.The test cases appropriately cover bare attributes (line 3) and various non-
{true}values (falsy values, strings, numbers, expressions) that should not trigger diagnostics in implicit mode.crates/biome_rule_options/src/use_consistent_boolean_props.rs (1)
1-22: Well-structured options module.The options structure follows Biome conventions with appropriate derives for serialisation, schema generation, and the
Implicitdefault mode aligns with the rule documentation.crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (1)
142-148: No issues found—trivia preservation is working correctly.The test snapshots confirm that trailing comments and whitespace are being preserved. For example, test case 2 shows
accept /** some comment */being correctly transformed toaccept={true} /** some comment */. The trivia handling in lines 142–148 is sound.
CodSpeed Performance ReportMerging #8120 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_rule_options/src/use_consistent_boolean_props.rs (2)
4-9: Consider adding documentation.The struct is correctly implemented with appropriate derives and serde attributes.
deny_unknown_fieldsis a nice touch for catching configuration errors. However, consider adding doc comments to explain the purpose and usage of this public API.Example:
+/// Configuration options for the `useConsistentBooleanProps` rule. +/// +/// Controls whether boolean props in JSX should use implicit (`<Foo bar />`) +/// or explicit (`<Foo bar={true} />`) syntax. #[derive(Default, Clone, Debug, Deserialize, Deserializable, Merge, Eq, PartialEq, Serialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields, default)] pub struct UseConsistentBooleanPropsOptions { + /// The boolean prop mode to enforce. pub mode: BooleanPropMode, }
11-18: Good choice of default; consider adding docs.
Implicitas the default aligns well with the linked issue's preference. The derives and serde configuration are spot on. Documentation comments would help users understand the behaviour of each mode.Example:
+/// Determines how boolean props in JSX should be formatted. #[derive(Default, Clone, Debug, Deserialize, Deserializable, Merge, Eq, PartialEq, Serialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase")] pub enum BooleanPropMode { + /// Enforce implicit boolean props: `<Foo bar />` (default). #[default] Implicit, + /// Enforce explicit boolean props: `<Foo bar={true} />`. Explicit, }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (1)
crates/biome_rule_options/src/use_consistent_boolean_props.rs(1 hunks)
⏰ 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). (13)
- GitHub Check: Test Node.js API
- GitHub Check: End-to-end tests
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Check JS Files
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
🔇 Additional comments (1)
crates/biome_rule_options/src/use_consistent_boolean_props.rs (1)
1-2: LGTM!The imports are correct and all are used in the derives below.
vladimir-ivanov
force-pushed
the
feat/use_consistent_boolean_props-2
branch
from
03a9d43 to
43dc1f9
Compare
vladimir-ivanov
assigned
dyc3,
ematipico and
siketyan and unassigned
ematipico,
dyc3 and
siketyan
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (1)
crates/biome_analyze/src/rule.rs (2)
sources(617-620)inspired(254-259)
⏰ 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). (14)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: End-to-end tests
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Check JS Files
- GitHub Check: Bench (biome_configuration)
- GitHub Check: autofix
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_rule_options/src/use_consistent_boolean_props.rs (1)
7-18: Consider adding documentation for the public API.Adding doc comments would help users understand the purpose of each mode:
+/// Configuration options for the useConsistentBooleanProps rule. pub struct UseConsistentBooleanPropsOptions { + /// The style to enforce for boolean props. pub mode: BooleanPropMode, } +/// The style to use for boolean props in JSX. pub enum BooleanPropMode { + /// Enforce implicit boolean props (e.g., `<input disabled />`). #[default] Implicit, + /// Enforce explicit boolean props (e.g., `<input disabled={true} />`). Explicit, }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (1)
crates/biome_rule_options/src/use_consistent_boolean_props.rs(1 hunks)
⏰ 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). (10)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test Node.js API
- GitHub Check: Check JS Files
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_js_parser)
🔇 Additional comments (2)
crates/biome_rule_options/src/use_consistent_boolean_props.rs (2)
1-18: Clean implementation!The options module is well-structured and follows Biome's patterns. The derives are comprehensive, the default behaviour (Implicit mode) aligns with the PR objectives, and the schema feature gating is consistent.
14-18: Ensure the oldnoImplicitBooleanrule is properly deprecated before release.The verification confirms both rules currently coexist—
noImplicitBooleanremains active in the style group without deprecation markers, whilstuseConsistentBooleanPropsis added to nursery. This creates overlapping functionality and user confusion.Before merging, either:
- Add deprecation notices to
NoImplicitBooleanwith migration guidance pointing touseConsistentBooleanProps- Remove
NoImplicitBooleanentirely if this is a replacement
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
We should deprecate the old rule (maybe in the next minor?) and provide a migrator for it in biome_migrate. That should probably go in a new PR against next, or we could move this PR onto next and include it here.
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
Show resolved
Hide resolved
vladimir-ivanov
force-pushed
the
feat/use_consistent_boolean_props-2
branch
from
68af37e to
68b8444
Compare
let me know if any action is required for this pr? @dyc3 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (1)
112-121: Consider enhancing diagnostic clarity per the rule pillars.The current message states what should be done but could better explain the context. With
action()implemented, you need at least two messages: one describing the issue and one suggesting the fix. The current single message somewhat covers both, but consider making it clearer—for example, stating that the current form doesn't match the configured mode.Based on learnings
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
📚 Learning: 2025-10-25T07:02:26.457Z
Learnt from: ematipico
Repo: biomejs/biome PR: 7856
File: .changeset/yellow-crews-guess.md:1-5
Timestamp: 2025-10-25T07:02:26.457Z
Learning: The Biome documentation website uses kebab-case URL slugs for rule pages (e.g., `/linter/rules/no-continue/`), not camelCase.
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (1)
crates/biome_analyze/src/rule.rs (2)
sources(617-620)inspired(254-259)
🔇 Additional comments (2)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (2)
92-106: LGTM!The detection logic correctly identifies violations in both modes and properly delegates to the
is_true_literalhelper.
196-207: Well-structured helper function.This correctly checks for
{true}literals by examining the trimmed expression content. Make sure to use it consistently throughout (see comment on lines 163-172).
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
Show resolved
Hide resolved
|
Do you want to do the deprecation in this PR or a new one on |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (1)
199-210: Consider a more type-safe check.The string comparison works, but checking the AST node type first would be more robust and avoid any ambiguity.
You could refactor to:
fn is_true_literal(jsx_attribute_initializer_clause: &JsxAttributeInitializerClause) -> bool { if let Ok(expr) = jsx_attribute_initializer_clause.value() && let Some(lit_expr) = expr.as_jsx_expression_attribute_value() && let Ok(expression) = lit_expr.expression() && let Some(literal) = expression.as_any_js_literal_expression() && let Some(bool_literal) = literal.as_js_boolean_literal_expression() { return bool_literal.value_token().is_ok_and(|t| t.text_trimmed() == "true"); } false }This ensures you're checking a boolean literal node before examining its text.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.jsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
📚 Learning: 2025-10-25T07:02:26.457Z
Learnt from: ematipico
Repo: biomejs/biome PR: 7856
File: .changeset/yellow-crews-guess.md:1-5
Timestamp: 2025-10-25T07:02:26.457Z
Learning: The Biome documentation website uses kebab-case URL slugs for rule pages (e.g., `/linter/rules/no-continue/`), not camelCase.
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (1)
crates/biome_analyze/src/rule.rs (1)
inspired(254-259)
🔇 Additional comments (5)
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.jsx (1)
1-4: LGTM! Good test coverage.The test cases appropriately cover explicit boolean props with various whitespace and comment placements that should be flagged in implicit mode.
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (4)
17-84: LGTM! Clear documentation and appropriate metadata.The rule declaration provides clear examples for both modes, and the metadata is correctly configured for a nursery rule.
92-107: LGTM! Logic correctly identifies violations.The run method properly uses the
is_true_literalhelper and correctly flags violations for both modes.
109-129: LGTM! Diagnostic messages are clear and actionable.The messages appropriately describe the issue and what the user should do.
138-160: LGTM! Explicit mode fix correctly adds={true}.The implementation properly preserves the existing attribute node whilst adding the initialiser, avoiding trivia issues.
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
Show resolved
Hide resolved
vladimir-ivanov
force-pushed
the
feat/use_consistent_boolean_props-2
branch
from
3362a95 to
d830f02
Compare
|
please review again |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (2)
152-161: Preserve attribute trivia when dropping the initialiserIn the implicit branch you rebuild the attribute from just the name:
let next_attr = make::jsx_attribute(jsx_attribute.name().ok()?).build();This risks losing any trivia/comments that were attached to the original attribute outside the
{true}expression. To mirror the explicit branch and better preserve formatting and comments, you can clear the initialiser in place instead:- if let Some(init) = jsx_attribute.initializer() - && is_true_literal(&init) - { - // Remove initializer - let next_attr = make::jsx_attribute(jsx_attribute.name().ok()?).build(); - mutation_has_changes = true; - mutation.replace_node(jsx_attribute.clone(), next_attr); - } + if let Some(init) = jsx_attribute.initializer() + && is_true_literal(&init) + { + // Remove initializer while preserving attribute trivia + let next_attr = jsx_attribute.clone().with_initializer(None); + mutation_has_changes = true; + mutation.replace_node(jsx_attribute.clone(), next_attr); + }That keeps the existing node and just drops the
{true}part, which should be safer for trivia.
170-173: Tiny nit: make the fix hint match the actual code (={true})The explicit‑mode action message currently says:
"Add `= {true}` to the attribute."but the fixer produces
={true}without a space. For consistency with the rule docs and the produced code, consider:- "Add `= {true}` to the attribute." + "Add `={true}` to the attribute."crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.jsx (1)
1-25: Add an explicitfalsecase and clarify the heading commentThis spec is great for non‑
truevalues, but it is missing the canonicaldisabled={false}example that the issue description calls out as valid for implicit mode. The heading comment also says “Explicit true/false” while only exercising the implicit form.Something along these lines would tighten it up:
-// Explicit true/false -<input disabled />; +// Implicit true and explicit false +<input disabled />; +<input disabled={false} />;That both documents the intent and gives you direct coverage that
{false}remains untouched in implicit mode.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.jsx.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (12)
.changeset/use-consistent-boolean-props.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.options.json(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/use_consistent_boolean_props.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.options.json
🚧 Files skipped from review as they are similar to previous changes (8)
- crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validImplicit.options.json
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.options.json
- crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidImplicit.jsx
- crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.jsx
- .changeset/use-consistent-boolean-props.md
- crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/validExplicit.jsx
- crates/biome_rule_options/src/use_consistent_boolean_props.rs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
Repo: biomejs/biome PR: 7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.options.json
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.options.jsoncrates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
📚 Learning: 2025-10-25T07:02:26.457Z
Learnt from: ematipico
Repo: biomejs/biome PR: 7856
File: .changeset/yellow-crews-guess.md:1-5
Timestamp: 2025-10-25T07:02:26.457Z
Learning: The Biome documentation website uses kebab-case URL slugs for rule pages (e.g., `/linter/rules/no-continue/`), not camelCase.
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (1)
crates/biome_analyze/src/rule.rs (3)
sources(617-620)inspired(254-259)recommended(602-605)
⏰ 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). (13)
- GitHub Check: Check JS Files
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Test Node.js API
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: autofix
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
🔇 Additional comments (2)
crates/biome_js_analyze/tests/specs/nursery/useConsistentBooleanProps/invalidExplicit.options.json (1)
1-15: Config wiring for explicit mode looks goodSchema path and
linter.rules.nursery.useConsistentBooleanPropsoptions match the intended"explicit"mode and align with the other spec option files.crates/biome_js_analyze/src/lint/nursery/use_consistent_boolean_props.rs (1)
76-204: Overall rule shape andis_true_literalhelper look solidNice separation of concerns:
runonly signals on the exact{true}/ missing‑initialiser cases,diagnostictailors the message by mode, andactionmirrors that with safe, mode‑specific fixes using the sharedis_true_literalhelper. This aligns well with the linked ESLint rule behaviour and the Biome rule pillars.
|
Review for what? You haven't answered my question. |
vladimir-ivanov
force-pushed
the
feat/use_consistent_boolean_props-2
branch
from
3c77d40 to
2892987
Compare
|
Tell you what, @vladimir-ivanov let's just go ahead and do the one PR.
|
|
The |
4 participants
Summary
Added
useConsistentBooleanPropsrule.This rule enforces consistent usage of boolean props in JSX.
Depending on the selected mode, the rule reports when boolean props are written implicitly or explicitly in a way that does not match the configuration.
• mode: "implicit" — encourages
<input disabled />• mode: "explicit" — encourages
<input disabled={true} />Examples
Valid (mode: implicit)
Valid (mode: explicit)
closes #5095