Skip to content

fix(noUnnecessararyConditions): no false positive on optional param with fallback#8775

Merged
ematipico merged 3 commits intobiomejs:mainfrom
igas:optional-fallback
Jan 29, 2026
Merged

fix(noUnnecessararyConditions): no false positive on optional param with fallback#8775
ematipico merged 3 commits intobiomejs:mainfrom
igas:optional-fallback

Conversation

@igas
Copy link
Copy Markdown
Contributor

@igas igas commented Jan 16, 2026

Wrote initial test myself, planning to ask claude code to fix it.

Summary

The noUnnecessararyConditions rule says that optional parameter can never be falsy, which is incorrect.

Test Plan

New test case covers the fix.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jan 16, 2026

🦋 Changeset detected

Latest commit: f7108e8

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

This PR includes changesets to release 14 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
@biomejs/prettier-compare 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 Jan 16, 2026
@github-actions github-actions Bot added A-Project Area: project A-Type-Inference Area: type inference labels Jan 16, 2026
@igas igas marked this pull request as ready for review January 16, 2026 03:46
Comment thread crates/biome_js_type_info/src/local_inference.rs
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jan 16, 2026

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 95 skipped benchmarks1


Comparing igas:optional-fallback (f7108e8) with main (4ee3bda)

Open in CodSpeed

Footnotes

  1. 95 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.

@igas igas requested a review from ematipico January 20, 2026 13:27
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 20, 2026

Walkthrough

This patch release fixes the noUnnecessaryConditions rule to correctly handle optional fallback patterns. The changes include adding test cases for optional parameter destructuring with default values, and updating the type inference engine to properly account for optional parameters by including undefined in the parameter type. The type propagation for parameter bindings has been adjusted to reflect optionality throughout the inference flow.

Suggested reviewers

  • ematipico
  • dyc3
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: preventing false positives in the noUnnecessararyConditions rule for optional parameters with fallback patterns.
Description check ✅ Passed The description relates to the changeset by explaining the bug fix and confirming test coverage, though it's somewhat brief about technical details.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

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

Copy link
Copy Markdown
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.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @.changeset/purple-snakes-sleep.md:
- Line 5: The rule identifier "noUnnecessararyConditions" contains a typo and
should be renamed to "noUnnecessaryConditions"; update the rule's exported
name/string in the rule definition (where "noUnnecessararyConditions" is
declared), rename any references in the rule registry/map, tests, docs,
changelog, and any metadata or configuration keys, and update import/exports so
all occurrences use "noUnnecessaryConditions" (ensure tests and rule lookup
still pass after renaming).

Comment thread .changeset/purple-snakes-sleep.md
@igas
Copy link
Copy Markdown
Contributor Author

igas commented Jan 20, 2026

Oh wow, there was a typo all along should be Unnecessary

@ematipico ematipico merged commit 7ea71cd into biomejs:main Jan 29, 2026
17 checks passed
@github-actions github-actions Bot mentioned this pull request Jan 29, 2026
IxxyDev added a commit to IxxyDev/biome that referenced this pull request Apr 24, 2026
…fined`

Adds unit tests asserting `ConditionalType::from_resolved_data` does not
classify the type of an optional non-primitive parameter (`user?: { name: string }`,
`args?: Args`, `value?: object`) as truthy.

These cases were reported as false positives of `noUnnecessaryConditions`
in biomejs#6611. They were actually fixed upstream by biomejs#8775 (commit 7ea71cd),
which wraps optional-param types in `Union([ty, undefined])`. The tests
land as regression guards so future refactors of the optional-parameter
path can't silently re-break `noUnnecessaryConditions` (and other
type-aware rules) for non-primitive annotations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Linter Area: linter A-Project Area: project A-Type-Inference Area: type inference L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants