feat(lint): new rule - use nullish coalescing #8089
Conversation
Adds a new lint rule to enforce the use of nullish coalescing operator (??) over logical OR (||) when dealing with nullable values. Includes migration support from TypeScript ESLint's prefer-nullish-coalescing rule.
🦋 Changeset detectedLatest commit: 3d2dd04 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-CLI
matanshavit
changed the title
…qual utility - Replace text-based expression comparison with semantic is_node_equal utility - Simplify is_in_test_position logic with functional style - Remove unnecessary RuleState struct from useNullishCoalescing - Update test snapshots to include new nullish coalescing rules
matanshavit
force-pushed
the
feat/8043-use-nullish-coalescing
branch
from
098ae36 to
ede06d1
Compare
Fix duplicate match arm and mark all 4 nullish coalescing rules as equivalent to @typescript-eslint/prefer-nullish-coalescing. The 4 Biome rules collectively provide the same functionality as the single ESLint rule: - useNullishCoalescing: || -> ?? - useNullishCoalescingAssignment: ||= -> ??= - useNullishCoalescingInTernary: ternary patterns - useIfAsNullishCoalescingAssignment: if statement patterns Changes: - Remove duplicate match arm that caused compilation error - Enable all 4 rules when migrating from ESLint - Mark rules as .same() instead of .inspired()
WalkthroughAdds four new JS/TS style lint rules to biome_js_analyze: useNullishCoalescing, useNullishCoalescingInTernary, useNullishCoalescingAssignment, and useIfAsNullishCoalescingAssignment. Each rule includes AST pattern matching, diagnostics, and safe automatic fixes where applicable. Corresponding rule option structs and pub modules were added to crates/biome_rule_options, the Style lint group was updated to register the new rules, and test specs (valid/invalid) were added for each rule. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
🧹 Nitpick comments (1)
.changeset/nullish-coalescing-rules.md (1)
7-10: Optional: Vary the sentence structure.All four bullet points begin with "Added the rule", which is a bit repetitive. Consider varying the structure for better readability, though the content is perfectly clear as-is.
For example:
-- Added the rule [`useNullishCoalescing`](https://biomejs.dev/linter/rules/use-nullish-coalescing/) to enforce using `??` instead of `||` for default values. -- Added the rule [`useNullishCoalescingInTernary`](https://biomejs.dev/linter/rules/use-nullish-coalescing-in-ternary/) to simplify ternary expressions that check for null/undefined. -- Added the rule [`useNullishCoalescingAssignment`](https://biomejs.dev/linter/rules/use-nullish-coalescing-assignment/) to prefer `??=` over `||=` for assignments. -- Added the rule [`useIfAsNullishCoalescingAssignment`](https://biomejs.dev/linter/rules/use-if-as-nullish-coalescing-assignment/) to convert if-statement patterns to nullish coalescing assignments. +- The rule [`useNullishCoalescing`](https://biomejs.dev/linter/rules/use-nullish-coalescing/) enforces using `??` instead of `||` for default values. +- [`useNullishCoalescingInTernary`](https://biomejs.dev/linter/rules/use-nullish-coalescing-in-ternary/) simplifies ternary expressions that check for null/undefined. +- [`useNullishCoalescingAssignment`](https://biomejs.dev/linter/rules/use-nullish-coalescing-assignment/) prefers `??=` over `||=` for assignments. +- [`useIfAsNullishCoalescingAssignment`](https://biomejs.dev/linter/rules/use-if-as-nullish-coalescing-assignment/) converts if-statement patterns to nullish coalescing assignments.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_cli/tests/snapshots/main_commands_lint/linter_shows_the_default_severity_of_rule_on.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_configuration/incorrect_rule_name.snapis excluded by!**/*.snapand included by**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/tests/specs/style/useIfAsNullishCoalescingAssignment/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/style/useIfAsNullishCoalescingAssignment/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/style/useNullishCoalescingAssignment/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/style/useNullishCoalescingAssignment/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/valid.js.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 (19)
.changeset/nullish-coalescing-rules.md(1 hunks)crates/biome_js_analyze/src/lint/style.rs(2 hunks)crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs(1 hunks)crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs(1 hunks)crates/biome_js_analyze/src/lint/style/use_nullish_coalescing_assignment.rs(1 hunks)crates/biome_js_analyze/src/lint/style/use_nullish_coalescing_in_ternary.rs(1 hunks)crates/biome_js_analyze/tests/specs/style/useIfAsNullishCoalescingAssignment/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/style/useIfAsNullishCoalescingAssignment/valid.js(1 hunks)crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/valid.js(1 hunks)crates/biome_js_analyze/tests/specs/style/useNullishCoalescingAssignment/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/style/useNullishCoalescingAssignment/valid.js(1 hunks)crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/valid.js(1 hunks)crates/biome_rule_options/src/lib.rs(2 hunks)crates/biome_rule_options/src/use_if_as_nullish_coalescing_assignment.rs(1 hunks)crates/biome_rule_options/src/use_nullish_coalescing.rs(1 hunks)crates/biome_rule_options/src/use_nullish_coalescing_assignment.rs(1 hunks)crates/biome_rule_options/src/use_nullish_coalescing_in_ternary.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-10-15T09:23:33.055Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:23:33.055Z
Learning: Applies to crates/biome_js_type_info/src/{type_info,local_inference,resolver,flattening}.rs : Avoid recursive type structures and cross-module Arcs; represent links between types using TypeReference and TypeData::Reference.
Applied to files:
crates/biome_rule_options/src/use_nullish_coalescing_in_ternary.rscrates/biome_rule_options/src/use_nullish_coalescing_assignment.rscrates/biome_rule_options/src/use_nullish_coalescing.rs
📚 Learning: 2025-10-15T09:22:46.002Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:46.002Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to fix code; if a mandatory token/node is missing, return `None` instead
Applied to files:
crates/biome_rule_options/src/use_nullish_coalescing_in_ternary.rs
📚 Learning: 2025-10-15T09:23:33.055Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:23:33.055Z
Learning: Applies to crates/biome_js_type_info/src/type_info.rs : Add new TypeScript type support by extending the TypeData enum rather than introducing parallel structures.
Applied to files:
crates/biome_rule_options/src/use_nullish_coalescing_in_ternary.rscrates/biome_rule_options/src/use_nullish_coalescing_assignment.rscrates/biome_rule_options/src/use_nullish_coalescing.rscrates/biome_rule_options/src/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-10-15T09:23:33.055Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:23:33.055Z
Learning: Applies to crates/biome_js_type_info/src/{type_info,local_inference}.rs : When inference is missing or cannot determine a type, use TypeData::Unknown; use TypeData::UnknownKeyword only when the user explicitly wrote unknown.
Applied to files:
crates/biome_rule_options/src/use_nullish_coalescing_in_ternary.rscrates/biome_rule_options/src/use_nullish_coalescing.rs
📚 Learning: 2025-10-15T09:22:46.002Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:46.002Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : For non-mandatory tokens, use the provided helper constructors (e.g., `token`, `space_token`, `dynamic_token`)
Applied to files:
crates/biome_rule_options/src/use_nullish_coalescing_in_ternary.rs
📚 Learning: 2025-10-15T09:22:46.002Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:46.002Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import and use the `FormatNode` trait for AST nodes
Applied to files:
crates/biome_rule_options/src/use_nullish_coalescing_in_ternary.rscrates/biome_js_analyze/src/lint/style.rs
📚 Learning: 2025-10-15T09:22:15.851Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-10-15T09:22:15.851Z
Learning: Applies to crates/biome_formatter/src/**/*.rs : After generation, remove usages of `format_verbatim_node` and implement real formatting with biome_formatter utilities
Applied to files:
crates/biome_rule_options/src/use_nullish_coalescing_in_ternary.rs
🧬 Code graph analysis (8)
crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/valid.js (1)
crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/invalid.js (2)
value(2-2)value2(5-5)
crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/invalid.js (1)
crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/valid.js (2)
value(4-4)result(20-20)
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs (3)
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs (1)
action(325-359)crates/biome_js_analyze/src/lint/style/use_nullish_coalescing_in_ternary.rs (1)
action(87-124)crates/biome_rowan/src/ast/mod.rs (1)
cast_ref(143-152)
crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/invalid.js (1)
crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/valid.js (5)
a(4-4)b(7-7)c(8-8)d(11-11)e(14-14)
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing_in_ternary.rs (4)
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs (6)
run(300-307)diagnostic(309-323)action(325-359)expressions_equivalent(290-292)is_null_or_undefined(279-282)is_null(272-276)crates/biome_js_factory/src/make.rs (1)
token_decorated_with_space(91-102)crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/invalid.js (2)
a(2-2)b(3-3)crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/valid.js (2)
a(4-4)b(7-7)
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs (2)
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing_in_ternary.rs (8)
is_loose_null_check(315-334)is_strict_nullish_check(266-285)is_null(343-346)expressions_equivalent(349-351)is_null_or_undefined(337-340)run(60-69)diagnostic(71-85)action(87-124)crates/biome_js_analyze/src/lint/style/use_nullish_coalescing_assignment.rs (3)
run(60-79)diagnostic(81-95)action(97-122)
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing_assignment.rs (2)
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs (3)
run(300-307)diagnostic(309-323)action(325-359)crates/biome_js_analyze/src/lint/style/use_nullish_coalescing_in_ternary.rs (3)
run(60-69)diagnostic(71-85)action(87-124)
crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/valid.js (1)
crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/invalid.js (5)
a(2-2)b(3-3)c(6-6)d(7-7)e(10-10)
🪛 LanguageTool
.changeset/nullish-coalescing-rules.md
[style] ~8-~8: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...instead of|| for default values. - Added the rule [useNullishCoalescingInTernar...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~9-~9: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ssions that check for null/undefined. - Added the rule [`useNullishCoalescingAssignme...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~10-~10: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...fer ??= over ||= for assignments. - Added the rule [`useIfAsNullishCoalescingAssi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (7)
crates/biome_rule_options/src/use_nullish_coalescing_assignment.rs (1)
1-6: LGTM!Standard options boilerplate – empty struct is fine when no configuration is needed. The derives and attributes follow the established pattern from the other nullish coalescing option structs in this PR.
crates/biome_js_analyze/tests/specs/style/useNullishCoalescingAssignment/invalid.js (1)
1-27: Comprehensive test coverage!The test cases effectively cover all the major patterns where
||=should be replaced with??=: basic assignments, object properties, array elements, nested access, functions, and computed properties. Well done.crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/invalid.js (1)
1-28: Solid test coverage!The invalid cases appropriately demonstrate where
||should be replaced with??across different contexts: assignments, functions, objects, arrays, and nested expressions. The coverage is thorough.crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/valid.js (1)
1-26: Well-structured valid cases!The test cases correctly demonstrate scenarios that should not trigger diagnostics: already using
??,||in conditional test positions (respecting theignoreConditionalTestsoption), and unrelated operators. Good coverage.crates/biome_js_analyze/tests/specs/style/useNullishCoalescingAssignment/valid.js (1)
1-14: LGTM!Valid cases appropriately demonstrate patterns that should not trigger this rule: already using
??=, regular assignments (delegated touseNullishCoalescing), and unrelated assignment operators. The rule separation is well thought out.crates/biome_js_analyze/tests/specs/style/useNullishCoalescingInTernary/valid.js (1)
1-14: Excellent coverage of edge cases!The valid cases correctly identify patterns that should not be converted: already using
??, non-nullish checks (checking for0ortrue), ternaries with different variables, and complex conditions. These demonstrate good understanding of when the transformation is not applicable.crates/biome_js_analyze/tests/specs/style/useIfAsNullishCoalescingAssignment/invalid.js (1)
1-54: Thorough pattern coverage!The test cases comprehensively cover all the if-statement patterns that can be converted to
??=: negation checks, explicit null/undefined checks, loose equality, various assignment targets (properties, arrays, computed), and different syntactic forms. The clear pattern labels make the test intent obvious. Excellent work!
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| impl Rule for UseNullishCoalescing { | ||
| type Query = biome_analyze::Ast<JsLogicalExpression>; |
There was a problem hiding this comment.
You can combine this all into one rule if you use declare_node_union! to query multiple node types. Then you can just branch into different functions based on the query.
There was a problem hiding this comment.
Since this is a new rule, it must go in the nursery group.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs (1)
277-280: Consider structural comparison instead of text matching.Whilst text comparison works for simple cases, a more robust approach would extract the expression from the assignment pattern and use structural comparison (
is_node_equal). This avoids potential edge cases with formatting variations.
📜 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/style/useIfAsNullishCoalescingAssignment/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/style/useIfAsNullishCoalescingAssignment/valid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs(1 hunks)crates/biome_js_analyze/tests/specs/style/useIfAsNullishCoalescingAssignment/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/style/useIfAsNullishCoalescingAssignment/valid.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_js_analyze/tests/specs/style/useIfAsNullishCoalescingAssignment/valid.js
- crates/biome_js_analyze/tests/specs/style/useIfAsNullishCoalescingAssignment/invalid.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
🧠 Learnings (16)
📓 Common learnings
Learnt from: dyc3
Repo: biomejs/biome PR: 8171
File: crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs:125-137
Timestamp: 2025-11-21T01:10:53.059Z
Learning: In the Biome codebase, each lint rule has its own options type declaration (e.g., `type Options = RuleNameOptions`) as part of the codegen process, even if the options struct is empty or unused. This is standard practice and should not be flagged as an issue.
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.077Z
Learning: Applies to .changeset/**/*.md : For new lint rules, show an example of an invalid case in an inline code snippet or code block; for rule changes, demonstrate what is now invalid or valid; for formatter changes, use a `diff` code block
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use language tags in documentation code blocks (js, ts, tsx, json, css) and order properties consistently as: language, then `expect_diagnostic`, then options modifiers, then `ignore`, then `file=path`
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Set the `language` field in `declare_lint_rule!` to the primary language (js for JavaScript, jsx for JSX, ts for TypeScript, tsx for TSX) that the rule applies to
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `useShorthand` prefix for rules that report syntax rewritable using equivalent compact syntax (e.g., `useShorthandAssign`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : When navigating CST nodes that return `Result`, use the try operator `?` to convert to `Option`, or use `let else` pattern for `Vec` return types in rule run functions
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnsafe` prefix for rules that report code leading to runtime failures (e.g., `noUnsafeOptionalChaining`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noEmpty` prefix for rules that report empty code (e.g., `noEmptyBlockStatements`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnused` prefix for rules that report unused entities (e.g., `noUnusedVariables`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUseless` prefix for rules that report unnecessary code that could be removed or simplified (e.g., `noUselessConstructor`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUndeclared` prefix for rules that report undefined entities (e.g., `noUndeclaredVariables`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Framework-specific rules should be named using the `use` or `no` prefix followed by the framework name (e.g., `noVueReservedProps`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Set the `language` field in `declare_lint_rule!` to the primary language (js for JavaScript, jsx for JSX, ts for TypeScript, tsx for TSX) that the rule applies to
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules must be implemented using the `Rule` trait with type parameters: `Query` (node type to analyze), `State` (information for signals), `Signals` (return type from run function), and `Options` (rule configuration)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Check if a variable is global before banning it to avoid false positives when the variable is redeclared in local scope; use the semantic model to verify global scope
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Code actions must specify a `fix_kind` field in the `declare_lint_rule!` macro as either `FixKind::Safe` or `FixKind::Unsafe` to indicate whether fixes always preserve program behavior
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noInvalid` prefix for rules that report runtime errors from mistyping (e.g., `noInvalidConstructorSuper`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : The `action` function must return a `JsRuleAction` (or equivalent language-specific action type) with category `ctx.action_category(ctx.category(), ctx.group())` and applicability from `ctx.metadata().applicability()`
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use `Semantic<T>` query type instead of `Ast<T>` when a rule needs to access the semantic model for binding references and scope information
Applied to files:
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs (1)
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing_in_ternary.rs (5)
is_loose_null_check(315-334)is_strict_nullish_check(266-285)is_null(343-346)expressions_equivalent(349-351)is_null_or_undefined(337-340)
🔇 Additional comments (6)
crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs (6)
1-12: LGTM!Standard imports for a Biome lint rule, all necessary and properly organised.
14-66: LGTM!Rule declaration follows all conventions: correct "use" prefix, language set to "js", safe fix kind, and comprehensive documentation with clear examples showing the boundaries of the rule (notably excluding
!foopatterns).
72-114: LGTM!The pattern-matching logic is sound and well-documented, correctly validating all preconditions for safe transformation to
??=.
116-151: LGTM!Properly handles both block and naked expression statements, with appropriate validation for single-statement blocks.
153-182: LGTM!The nullish check detection is now correct, properly excluding the
!variablepattern (which checks all falsy values) and only matching true nullish checks (== nullor=== null || === undefined). The documentation clearly explains why the negation pattern is excluded.
287-353: LGTM!The
Ruletrait implementation follows all conventions correctly: proper query type, clear diagnostic message, and a well-structured action that safely transforms the if statement to??=assignment with appropriate metadata.
| /// Checks for pattern: `variable == null` | ||
| /// | ||
| /// Loose equality check matches both null and undefined due to JavaScript's | ||
| /// type coercion rules. | ||
| fn is_loose_null_check(expr: &AnyJsExpression) -> Option<AnyJsExpression> { | ||
| let binary = expr.as_js_binary_expression()?; | ||
| let op = binary.operator().ok()?; | ||
|
|
||
| if !matches!(op, JsBinaryOperator::Equality) { | ||
| return None; | ||
| } | ||
|
|
||
| let left = binary.left().ok()?; | ||
| let right = binary.right().ok()?; | ||
|
|
||
| // Check if either side is null | ||
| if is_null(&right) { | ||
| return Some(left); | ||
| } | ||
| if is_null(&left) { | ||
| return Some(right); | ||
| } | ||
|
|
||
| None | ||
| } | ||
|
|
||
| /// Checks for pattern: `variable === null || variable === undefined` | ||
| /// | ||
| /// Returns the variable if both sides of the OR check the same variable | ||
| /// for strict equality against null or undefined. | ||
| fn is_strict_nullish_check(expr: &AnyJsExpression) -> Option<AnyJsExpression> { | ||
| let logical = expr.as_js_logical_expression()?; | ||
| let op = logical.operator().ok()?; | ||
|
|
||
| // Must be OR operator | ||
| if !matches!(op, JsLogicalOperator::LogicalOr) { | ||
| return None; | ||
| } | ||
|
|
||
| let left = logical.left().ok()?; | ||
| let right = logical.right().ok()?; | ||
|
|
||
| // Check if both sides are strict equality checks | ||
| let left_checked = is_strict_equality_check(&left)?; | ||
| let right_checked = is_strict_equality_check(&right)?; | ||
|
|
||
| // Both sides must check the same variable | ||
| if !expressions_equivalent(&left_checked, &right_checked) { | ||
| return None; | ||
| } | ||
|
|
||
| Some(left_checked) | ||
| } | ||
|
|
||
| /// Checks for pattern: `variable === null` or `variable === undefined` | ||
| /// | ||
| /// Returns the checked variable if the binary expression uses strict equality | ||
| /// to compare against null or undefined. | ||
| fn is_strict_equality_check(expr: &AnyJsExpression) -> Option<AnyJsExpression> { | ||
| let binary = expr.as_js_binary_expression()?; | ||
| let op = binary.operator().ok()?; | ||
|
|
||
| if !matches!(op, JsBinaryOperator::StrictEquality) { | ||
| return None; | ||
| } | ||
|
|
||
| let left = binary.left().ok()?; | ||
| let right = binary.right().ok()?; | ||
|
|
||
| // Check if either side is null or undefined | ||
| if is_null_or_undefined(&right) { | ||
| return Some(left); | ||
| } | ||
| if is_null_or_undefined(&left) { | ||
| return Some(right); | ||
| } | ||
|
|
||
| None | ||
| } | ||
|
|
||
| /// Checks if an expression is a `null` literal | ||
| fn is_null(expr: &AnyJsExpression) -> bool { | ||
| use biome_js_syntax::static_value::StaticValue; | ||
| expr.as_static_value() | ||
| .is_some_and(|v| matches!(v, StaticValue::Null(_))) | ||
| } | ||
|
|
||
| /// Checks if an expression is `null` or `undefined` literal | ||
| fn is_null_or_undefined(expr: &AnyJsExpression) -> bool { | ||
| expr.as_static_value() | ||
| .is_some_and(|v| v.is_null_or_undefined()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated helper functions to a shared utility module.
These helper functions (is_loose_null_check, is_strict_nullish_check, is_null, is_null_or_undefined) are duplicated from use_nullish_coalescing_in_ternary.rs (lines 265-345). To adhere to DRY principles and simplify maintenance, extract them to a common utility module that both rules can import.
Only report || operators when the right-hand side is a "safe" default value
that won't cause unexpected behavior when replaced with ??:
- Object/array literals: {}, []
- Non-empty string literals: 'default'
- Function/class expressions
- new expressions
- Member access expressions
Skip reporting in:
- Boolean coercion contexts: Boolean(), !!, !
- Mixed logical chains: a || b && c
- Conditional test positions (existing behavior)
This prevents unsafe auto-fixes where legitimate falsy values like 0, '',
or false would behave differently with ?? vs ||.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs (2)
16-103: Reconsider marking this asFixKind::SafeThis rule deliberately changes the semantics of
lhs || rhstolhs ?? rhsin default-value patterns; even with the current heuristics, that can still alter observable behaviour when callers rely on0,''orfalsebeing replaced. Given CONTRIBUTING definesFixKind::Safeas “always preserve program behaviour”, it might be more accurate (and safer for auto-apply defaults) to classify this asFixKind::Unsafeand let users opt in to the transformation.Something like:
- fix_kind: FixKind::Safe, + fix_kind: FixKind::Unsafe,would align better with that definition, unless the project has explicitly decided to treat these as “safe bugfixes”.
Please double‑check against the latest CONTRIBUTING notes and how other semantics‑changing rules set
fix_kind.
228-300: Template literal handling may be a bit too optimistic
JsTemplateExpressionis currently treated as “safe” wheneverelements().iter().next().is_some(). That will also flag cases like:const value = input || `${maybeFalsy}`;where the fallback can still legitimately be
""or"0"depending onmaybeFalsy. If you want to be conservative, you could:
- Drop template literals from the “safe” set for now, or
- Tighten the heuristic (e.g. require at least one non‑empty chunk, or avoid single‑expression templates).
Not a blocker, but worth considering if you want to minimise false positives further.
Might be worth checking how
@typescript-eslint/prefer-nullish-coalescingtreats template literals in its default‑value heuristics.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/valid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs(1 hunks)crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/valid.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/valid.js
- crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/invalid.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnsafe` prefix for rules that report code leading to runtime failures (e.g., `noUnsafeOptionalChaining`)
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use language tags in documentation code blocks (js, ts, tsx, json, css) and order properties consistently as: language, then `expect_diagnostic`, then options modifiers, then `ignore`, then `file=path`
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `useShorthand` prefix for rules that report syntax rewritable using equivalent compact syntax (e.g., `useShorthandAssign`)
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Set the `language` field in `declare_lint_rule!` to the primary language (js for JavaScript, jsx for JSX, ts for TypeScript, tsx for TSX) that the rule applies to
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noEmpty` prefix for rules that report empty code (e.g., `noEmptyBlockStatements`)
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `useConsistent` prefix for rules that ensure consistency across the codebase (e.g., `useConsistentArrayType`)
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noRedundant` prefix for rules that report redundant code (e.g., `noRedundantUseStrict`)
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noInvalid` prefix for rules that report runtime errors from mistyping (e.g., `noInvalidConstructorSuper`)
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnknown` prefix for rules that report mistyped entities in CSS (e.g., `noUnknownUnit`)
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUseless` prefix for rules that report unnecessary code that could be removed or simplified (e.g., `noUselessConstructor`)
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnsafe` prefix for rules that report code leading to runtime failures (e.g., `noUnsafeOptionalChaining`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUseless` prefix for rules that report unnecessary code that could be removed or simplified (e.g., `noUselessConstructor`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noEmpty` prefix for rules that report empty code (e.g., `noEmptyBlockStatements`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Set the `language` field in `declare_lint_rule!` to the primary language (js for JavaScript, jsx for JSX, ts for TypeScript, tsx for TSX) that the rule applies to
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : When navigating CST nodes that return `Result`, use the try operator `?` to convert to `Option`, or use `let else` pattern for `Vec` return types in rule run functions
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnused` prefix for rules that report unused entities (e.g., `noUnusedVariables`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use language tags in documentation code blocks (js, ts, tsx, json, css) and order properties consistently as: language, then `expect_diagnostic`, then options modifiers, then `ignore`, then `file=path`
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `useShorthand` prefix for rules that report syntax rewritable using equivalent compact syntax (e.g., `useShorthandAssign`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Deprecate rules by adding a `deprecated` field to the `declare_lint_rule!` macro with a message explaining the reason for deprecation (e.g., 'Use the rule noAnotherVar')
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noRedundant` prefix for rules that report redundant code (e.g., `noRedundantUseStrict`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `useValid` prefix for rules that report code that always evaluates to a constant (e.g., `useValidTypeof`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-09-23T21:33:00.843Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7571
File: crates/biome_service/src/file_handlers/html.rs:360-368
Timestamp: 2025-09-23T21:33:00.843Z
Learning: Don't flag potential move/borrow issues in Rust code reviews since the Rust compiler catches these at compile time. Focus on logic, design, and issues that the toolchain doesn't automatically detect.
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noInvalid` prefix for rules that report runtime errors from mistyping (e.g., `noInvalidConstructorSuper`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Code actions must specify a `fix_kind` field in the `declare_lint_rule!` macro as either `FixKind::Safe` or `FixKind::Unsafe` to indicate whether fixes always preserve program behavior
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noMisleading` prefix for rules that report valid but potentially misleading code (e.g., `noMisleadingCharacterClass`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noConstant` prefix for rules that report computations always evaluated to the same value (e.g., `noConstantMathMinMaxClamp`)
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `declare_node_union!` macro to query multiple node types at once by joining them into an enum with `Any*Like` naming convention
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : New rules must be placed inside the `nursery` group as an incubation space exempt from semantic versioning, with promotion to appropriate groups done during minor/major releases
Applied to files:
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs (5)
crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/invalid.js (1)
name(12-12)crates/biome_js_analyze/src/lint/style/use_if_as_nullish_coalescing_assignment.rs (2)
run(293-300)diagnostic(302-316)crates/biome_js_analyze/src/lint/style/use_nullish_coalescing_in_ternary.rs (2)
run(60-69)diagnostic(71-85)crates/biome_js_analyze/src/lint/complexity/use_optional_chain.rs (1)
logical(988-988)crates/biome_js_analyze/tests/specs/style/useNullishCoalescing/valid.js (1)
text(37-37)
🔇 Additional comments (3)
crates/biome_js_analyze/src/lint/style/use_nullish_coalescing.rs (3)
105-143: Heuristics inrunlook well‑layeredNice job sequencing the filters here: operator check, conditional‑test option, boolean context, mixed logical chains, then the safe‑default guard. This should address the earlier “blanket
||” concern while keeping the rule logic readable and easy to extend.
337-374: Mixed logical‑chain guard looks solidThe parent/child checks for non‑
||logical operators are a neat way to avoid??suggestions ina || b && cand similar precedence‑sensitive chains. This should keep the fixes from wandering into the “needs extra parentheses” territory.
302-335: I'm unable to run shell scripts due to repository cloning issues. However, I can analyze the code snippet provided in your review comment directly, which shows the actual implementation.Analysis of the provided code snippet:
The code you provided (lines 302-335) shows:
fn is_in_boolean_context(logical: &JsLogicalExpression) -> bool { let Some(parent) = logical.syntax().parent() else { return false; }; // Check for !(x || y) or !!(x || y) if let Some(unary) = JsUnaryExpression::cast(parent.clone()) && matches!(unary.operator(), Ok(JsUnaryOperator::LogicalNot)) { return true; } // ... rest of checks }This implementation does only check the immediate parent, exactly as your review claims. The comment documents intent to handle
!(x || y)and!!(x || y), but:
- In typical Biome ASTs, a parenthesized expression like
!(x || y)would haveJsParenthesizedExpressionbetween the logical and unary operators- The direct
parent()check followed byJsUnaryExpression::cast()would fail when aJsParenthesizedExpressionintercedes- The same applies to
Boolean(x || y)with nested wrappersYour proposed fix using ancestor walking with a stopping condition at the next logical expression is sound.
However, I cannot verify:
- Whether tests exist for these specific patterns
- The exact AST structure to confirm parenthesization assumptions
- Whether the fix's stopping condition at
JsLogicalExpressionis completeGiven the code inspection confirms the core issue but I cannot fully verify test coverage and empirical behavior:
is_in_boolean_contextdoesn't catch!(x || y)/!!(x || y)when parenthesizedThe implementation only checks the immediate parent, but typical AST structures insert
JsParenthesizedExpressionbetween the logical and unary operators. This means negation and boolean-coercion cases slip through unreported.Your proposed ancestor-walking fix addresses this. However, verify:
- Do tests exist for
const b = !!(x || {});andconst b = Boolean(x || {});?- Does the stopping condition at
JsLogicalExpressioncorrectly bound the search scope?
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
I'll be honest here. I'm not sure I want to accept the PR as it is:
- four big rules with non-trivial logic in one single PR. That's a bit too much for me (can't speak for others).
- all rules are marked as
.same()the original rule while clearly they aren't. - rules must go in nursery, you know that (not your first contribution with new rules)
It hasn't been explained why we had to breakdown the rule (the issue doesn't mention it). We usually breakdown rules if there are business logic reasons, but the discussion was mainly around coding, which isn't a factor for these kinds of decisions.
There was a problem hiding this comment.
We want a changeset for each rule. Doesn't matter if you decided to implement 4 rules in one pr
3 participants
AI Assistance Notice
The PR was primarily created by Claude Code.
Summary
Implements #8043. Added four new lint rules that enforce the nullish coalescing operator (
??) over logical OR (||) for nullable values:useNullishCoalescinguseNullishCoalescingInTernaryuseNullishCoalescingAssignmentuseIfAsNullishCoalescingAssignmentThese rules detect patterns where
??is safer than||(avoiding false positives with0,"",false, etc.).Example:
Includes ESLint migration mapping for
@typescript-eslint/prefer-nullish-coalescing.Test Plan
Added comprehensive test cases for valid and invalid patterns for each rule.