fix(useOptionalChain): support Yoda expressions#7387
Conversation
🦋 Changeset detectedLatest commit: f2296b3 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 |
WalkthroughIntroduces a new JsBinaryExpression API Assessment against linked issues
Suggested reviewers
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (8)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
bot
added
A-Linter
![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 (10)
.changeset/big-keys-play.md (1)
5-5: Polish the changeset (tense split, final full stop, canonical docs URL, add example).Bring it in line with our changeset conventions: past tense for the action, present tense for current behaviour, sentences ending with a full stop, and include a representative example. Also swap the Japanese docs URL for the canonical one.
-Fixed [#7381](https://github.com/biomejs/biome/issues/7381), now the [`useOptionalChain`](https://biomejs.dev/ja/linter/rules/use-optional-chain/) rule recognizes optional chaining using Yoda expressions (e.g., `undefined !== foo && foo.bar`) +Fixed [#7381](https://github.com/biomejs/biome/issues/7381). +The [`useOptionalChain`](https://biomejs.dev/linter/rules/use-optional-chain/) rule now recognises optional chaining when written as Yoda expressions. + +#### Example +```js +// Before +undefined !== foo && foo.bar +// After +foo?.bar +```crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases5.js (1)
56-66: Add ‘void 0’ Yoda variants to exercise undefined aliasing.Many codebases still use
void 0forundefined. Adding a couple of lines ensures the extractor treats it equivalently.// chained calls undefined != foo && undefined != foo.bar && undefined != foo.bar.baz && foo.bar.baz.buzz() undefined != foo && undefined != foo.bar && undefined != foo.bar.baz && undefined != foo.bar.baz.buzz && foo.bar.baz.buzz() undefined != foo.bar && undefined != foo.bar.baz && undefined != foo.bar.baz.buzz && foo.bar.baz.buzz() + +// 'void 0' variants (alias of undefined) +void 0 != foo && foo.bar +void 0 != foo.bar && foo.bar.baz +void 0 != foo && foo()crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases4.js (1)
48-55: Consider a couple of parenthesised Yoda forms and ‘void 0’ strict variants.Parentheses and
void 0are cheap extra edges that have bitten normalisers before.undefined !== foo && undefined !== foo.bar && undefined !== foo.bar.baz && foo.bar.baz.buzz undefined !== foo.bar && undefined !== foo.bar.baz && foo.bar.baz.buzz + +// Parenthesised expressions +(null !== (foo)) && (foo).bar +(undefined !== (foo?.bar)) && (foo?.bar).baz + +// 'void 0' strict variants +void 0 !== foo && foo.bar +void 0 !== foo.bar && foo.bar.bazcrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases6.jsx (1)
21-27: Private fields outside a declaring class may trigger early errors—please confirm parser mode.Access like
foo.#baris an early error unless the private names are declared in the containing class. If our test harness disables early errors, we're fine; otherwise, consider wrapping these in a minimal class to keep fixtures parseable.Happy to propose a wrapped variant if needed.
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_validCases.ts (3)
36-39: Remove duplicated valid case.
foo["some long"] && foo["some long string"].baz;appears twice (Lines 36 and 38). Drop one to keep the fixture lean.Apply:
-foo["some long"] && foo["some long string"].baz; foo[`some long`] && foo[`some long string`].baz; -foo["some long"] && foo["some long string"].baz;
46-47: Track or enable the commented-out case.The FIXME for
(new foo() || {}).barsuggests an expected no-diagnostic. Either enable it (if now supported) or link a tracking issue in a comment for future work.Happy to raise a follow-up issue and add a focused test once confirmed.
1-1: Test filename convention.Our snapshots typically use files prefixed with
valid*/invalid*. Consider renaming this to start withvalid(e.g.,valid.yoda_expressions.ts) for consistency with the harness.crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases3.js (1)
26-62: Add non‑strict andundefinedYoda coverage.Great breadth. To fully exercise the extractor, add a couple of
!=cases andundefined !== …variants, e.g.:
undefined != foo && foo.barundefined !== foo.bar && foo.bar.bazcrates/biome_js_syntax/src/expr_ext.rs (1)
345-369: Recognisevoid …as undefined (and considerglobalThis.undefined).
is_nullishonly checks literalnulland identifierundefined. Many codebases usevoid 0(orvoid any) asundefined. Optionally recognise it; likewiseglobalThis.undefined/window.undefinedif you wish to be generous.Apply minimal support for
void:fn is_nullish(expression: &AnyJsExpression) -> bool { - expression - .as_static_value() - .is_some_and(|x| x.is_null_or_undefined()) + expression + .as_static_value() + .is_some_and(|x| x.is_null_or_undefined()) + || matches!( + expression, + AnyJsExpression::JsUnaryExpression(u) + if u.operator().is_ok_and(|op| matches!(op, JsUnaryOperator::Void)) + ) }If you also want
globalThis.undefined:- fn is_nullish(expression: &AnyJsExpression) -> bool { + fn is_nullish(expression: &AnyJsExpression) -> bool { // existing checks … + || crate::global_identifier(expression) + .is_some_and(|(_, name)| matches!(name, StaticValue::String(tok) if tok.text_trimmed() == "undefined")) }crates/biome_js_analyze/src/lint/complexity/use_optional_chain.rs (1)
564-567: Minor readability tweak for nested??.
ok()??is concise but slightly opaque. Consider expanding for clarity.-AnyJsExpression::JsBinaryExpression(expression) => { - expression.extract_optional_chain_like().ok()?? -} +AnyJsExpression::JsBinaryExpression(expression) => { + match expression.extract_optional_chain_like() { + Ok(Some(expr)) => expr, + _ => return None, + } +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases3.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases4.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases5.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases6.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_validCases.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (8)
.changeset/big-keys-play.md(1 hunks)crates/biome_js_analyze/src/lint/complexity/use_optional_chain.rs(2 hunks)crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases3.js(1 hunks)crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases4.js(1 hunks)crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases5.js(1 hunks)crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases6.jsx(1 hunks)crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_validCases.ts(1 hunks)crates/biome_js_syntax/src/expr_ext.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
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/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases6.jsxcrates/biome_js_analyze/src/lint/complexity/use_optional_chain.rscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_validCases.tscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases3.jscrates/biome_js_syntax/src/expr_ext.rscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases4.jscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases5.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases6.jsxcrates/biome_js_analyze/src/lint/complexity/use_optional_chain.rscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_validCases.tscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases3.jscrates/biome_js_syntax/src/expr_ext.rscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases4.jscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases5.js
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases6.jsxcrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_validCases.tscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases3.jscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases4.jscrates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases5.js
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: Create changesets withjust new-changeset; store them in.changeset/with correct frontmatter (package keys and change type).
In changeset descriptions, follow content conventions: user-facing changes only; past tense for what you did; present tense for current behavior; link issues for fixes; link rules/assists; include representative code blocks; end every sentence with a period.
When adding headers in a changeset, only use #### or ##### levels.
Files:
.changeset/big-keys-play.md
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_js_analyze/src/lint/complexity/use_optional_chain.rscrates/biome_js_syntax/src/expr_ext.rs
🧠 Learnings (2)
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Avoid `unwrap`/`expect`; use `?`, `ok()?`, and combinators (`map`, `and_then`, `filter`) to handle `Result`/`Option` when navigating the CST
Applied to files:
crates/biome_js_analyze/src/lint/complexity/use_optional_chain.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`
Applied to files:
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_validCases.ts
⏰ 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). (23)
- GitHub Check: autofix
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Test Node.js API
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Parser conformance
🔇 Additional comments (6)
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases5.js (1)
1-66: Solid coverage of loose/nullish Yoda cases.Nice spread of chained members, element access, and calls. This will catch regressions quickly.
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases4.js (1)
1-57: Good breadth on strict inequality forms.Covers the important shapes (dot/bracket, calls, partial optional chains). Looks great.
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/yoda_expressions_logicalAndCases6.jsx (1)
36-63: Nice touch on preserving whitespace, JSX, and comments.These tend to regress; lovely to see them covered.
crates/biome_js_syntax/src/expr_ext.rs (2)
323-343: Nice normalisation: Yoda expressions now handled.Returning the non-nullish side via
Option<AnyJsExpression>is a clean API for callers.
323-343: All internal references tois_optional_chain_likehave been removed; onlyextract_optional_chain_likeremains—no semver break within this workspace.crates/biome_js_analyze/src/lint/complexity/use_optional_chain.rs (1)
266-270: Good switch to the extractor.Normalising with
extract_optional_chain_like()?simplifies the flow and covers Yoda cases.
CodSpeed Performance ReportMerging #7387 will not alter performanceComparing Summary
|
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
Snapshots look good. Nice! Please fix the lint :)
|
Whoops — I'm ready now. |
|
benchmark tests failed... 😞 |
3 participants
Summary
Closes #7381
Now the
useOptionalChainrule recognizes optional chaining using Yoda expressions (e.g.,undefined !== foo && foo.bar).Test Plan
Added snap tests.
Using the shell script below, Yoda expressions versions of the existing tests were generated, and the snapshots of the Yoda versions were compared with the originals to confirm identical results.
Docs
changeset only