Skip to content

fix(useOptionalChain): correctly track prefix expression#9345

Merged
ematipico merged 1 commit intomainfrom
fix/use-optional-chain
Mar 5, 2026
Merged

fix(useOptionalChain): correctly track prefix expression#9345
ematipico merged 1 commit intomainfrom
fix/use-optional-chain

Conversation

@ematipico
Copy link
Member

Summary

Closes #7214

I used Claude Code to fix the issue. It's very simple!

Test Plan

Added various tests

Docs

@ematipico ematipico requested review from a team March 5, 2026 14:00
@changeset-bot
Copy link

changeset-bot bot commented Mar 5, 2026

🦋 Changeset detected

Latest commit: 627dc0d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

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 github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Mar 5, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 5, 2026

Walkthrough

This PR enhances the useOptionalChain rule to detect and fix optional chain patterns within logical AND expressions that don't start at the beginning (e.g., bar && foo && foo.lengthbar && foo?.length). A new LogicalAndChainNodes struct encapsulates chain nodes alongside an optional prefix expression, enabling the rule to preserve non-chain operands whilst transforming the identifiable chain portion.

Possibly related PRs

Suggested labels

A-Linter, L-JavaScript

Suggested reviewers

  • dyc3
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing the useOptionalChain rule to correctly track prefix expressions in logical AND chains.
Description check ✅ Passed The description clearly relates to the changeset, references the relevant issue (#7214), and provides context about the fix and testing approach.
Linked Issues check ✅ Passed The PR fully addresses issue #7214 by implementing prefix tracking in the useOptionalChain rule to handle chains not at the start of expressions, with added test coverage.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the useOptionalChain rule: a changeset documenting the fix, rule implementation updates, and test cases validating the new behaviour.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/use-optional-chain

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/invalidLogicalAndCases7.js (1)

27-28: Minor: Duplicate test case.

Line 28 (bar && foo && foo.length;) is identical to line 4. If the intent is purely documentation (referencing the original issue), consider adding it as a comment alongside line 4 rather than a separate test case that produces duplicate diagnostics.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/invalidLogicalAndCases7.js`
around lines 27 - 28, The test file contains a duplicate test case "bar && foo
&& foo.length;" (same as the earlier instance) which creates redundant
diagnostics; remove the second occurrence from
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/invalidLogicalAndCases7.js
or convert it to a comment referencing the original instance so it doesn't
produce a duplicate test—locate the duplicate string "bar && foo && foo.length;"
and either delete that line or prefix it with comment markers to keep it as
documentation only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/invalidLogicalAndCases7.js`:
- Around line 27-28: The test file contains a duplicate test case "bar && foo &&
foo.length;" (same as the earlier instance) which creates redundant diagnostics;
remove the second occurrence from
crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/invalidLogicalAndCases7.js
or convert it to a comment referencing the original instance so it doesn't
produce a duplicate test—locate the duplicate string "bar && foo && foo.length;"
and either delete that line or prefix it with comment markers to keep it as
documentation only.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 042ba863-3f52-422b-abab-da8bac24ac2b

📥 Commits

Reviewing files that changed from the base of the PR and between cabc56c and 627dc0d.

⛔ Files ignored due to path filters (1)
  • crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/invalidLogicalAndCases7.js.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (3)
  • .changeset/fix-use-optional-chain-prefix.md
  • crates/biome_js_analyze/src/lint/complexity/use_optional_chain.rs
  • crates/biome_js_analyze/tests/specs/complexity/useOptionalChain/invalidLogicalAndCases7.js

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 5, 2026

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 156 skipped benchmarks1


Comparing fix/use-optional-chain (627dc0d) with main (b535f60)

Open in CodSpeed

Footnotes

  1. 156 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Contributor

@denbezrukov denbezrukov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😯

@ematipico ematipico merged commit 70c2d4e into main Mar 5, 2026
18 checks passed
@ematipico ematipico deleted the fix/use-optional-chain branch March 5, 2026 15:07
@github-actions github-actions bot mentioned this pull request Mar 5, 2026
@eMerzh
Copy link
Contributor

eMerzh commented Mar 10, 2026

hum... checking the preview build, this seems to have open a few new false positive...

for example :


  ⚠ Change to an optional chain.

    20 │ }) => {
    21 │   const handleChange = (inView: boolean) => {
  > 22 │        if (hasMore && !isLoading && inView) {
       │           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    23 │       onEnter()
    24 │     }

or

    218 │      const height = String(theme.variables.assets.main.height)
  > 219 │         const html = (embed?.extract?.media && 'html' in embed.extract.media && embed.extract?.media.html) || ''
        │                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    220 │
    221 │      return (

(this one , you need to make sure it's not undef to check for in

@ematipico
Copy link
Member Author

@eMerzh

Open a new issue with a playground link

Commenting on closed issues or PRs is rarely visible

@eMerzh
Copy link
Contributor

eMerzh commented Mar 10, 2026

my bad, i've created it here #9428

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Linter Area: linter L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

💅 useOptionalChain doesn't work when expected chain isn't at the start of expression

3 participants