Skip to content

fix: widen mutable property literal types - #11297

Open
harshmathurx wants to merge 1 commit into
biomejs:mainfrom
harshmathurx:fix/no-unnecessary-conditions-mutable-property
Open

fix: widen mutable property literal types#11297
harshmathurx wants to merge 1 commit into
biomejs:mainfrom
harshmathurx:fix/no-unnecessary-conditions-mutable-property

Conversation

@harshmathurx

Copy link
Copy Markdown

Fixes #11174.

This widens primitive literal initializers for mutable object/class properties, so noUnnecessaryConditions does not treat checks like guard.current after { current: false } as permanently falsy.

Added regression coverage for object and class properties.

@ematipico this follows up on the confirmed issue.

AI assistance disclosure: I used AI assistance to inspect the relevant inference path and draft the patch; I reviewed the changes and ran the focused checks locally.

@changeset-bot

changeset-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e38cd48

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

Copy link
Copy Markdown
Contributor

✅ Organic activity

No automation signals detected in the analyzed events.

View full analysis →

This is an automated analysis by AgentScan

@github-actions github-actions Bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Type-Inference Area: type inference labels Aug 10, 2026
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Property initialisers now widen literal bigint, boolean, number, string, and template values to their primitive types. Object and regular-expression literals retain literal types. Class and object property inference uses this widening. Valid cases cover mutable object properties and private class fields. A patch changeset documents the noUnnecessaryConditions correction.

Possibly related PRs

Suggested labels: A-Linter, L-JavaScript, A-Type-Inference

Suggested reviewers: ematipico, dyc3

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarises the main change: widening mutable property literal types.
Description check ✅ Passed The description directly explains the fix, affected rule, regression coverage, and linked issue.
Linked Issues check ✅ Passed The changes widen primitive literals for mutable object and class properties and add regression tests, matching issue #11174.
Out of Scope Changes check ✅ Passed The changes are limited to type widening, targeted regression tests, and the related changeset.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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_type_info/src/local_inference.rs (1)

1904-1930: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add rustdoc for the widening contract.

Document both helpers with /// comments. State which primitive literal types widen, and why object and regular-expression literals remain literal types.

As per coding guidelines, use rustdoc for new rule changes and document contracts for future contributors.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/biome_js_type_info/src/local_inference.rs` around lines 1904 - 1930,
Add rustdoc comments to reference_to_widened_property_value and
widen_property_value_type_data describing that bigint, boolean, number, string,
and template literals widen to their primitive types, while object and
regular-expression literals remain literal types to preserve their structural or
identity-specific information.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/biome_js_type_info/src/local_inference.rs`:
- Around line 1904-1930: Add rustdoc comments to
reference_to_widened_property_value and widen_property_value_type_data
describing that bigint, boolean, number, string, and template literals widen to
their primitive types, while object and regular-expression literals remain
literal types to preserve their structural or identity-specific information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c060588d-fa3b-4da9-a0f3-9cc243559eb5

📥 Commits

Reviewing files that changed from the base of the PR and between db36fb4 and e38cd48.

⛔ Files ignored due to path filters (1)
  • crates/biome_js_analyze/tests/specs/suspicious/noUnnecessaryConditions/valid.ts.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (3)
  • .changeset/witty-badgers-widen.md
  • crates/biome_js_analyze/tests/specs/suspicious/noUnnecessaryConditions/valid.ts
  • crates/biome_js_type_info/src/local_inference.rs

@codspeed-hq

codspeed-hq Bot commented Aug 15, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 14.63%

❌ 1 regressed benchmark
✅ 61 untouched benchmarks
⏩ 237 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
js_analyzer[index_3894593175024091846.js] 73.9 ms 86.5 ms -14.63%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing harshmathurx:fix/no-unnecessary-conditions-mutable-property (e38cd48) with main (f34e15c)2

Open in CodSpeed

Footnotes

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

  2. No successful run was found on main (db36fb4) during the generation of this report, so f34e15c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

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

Labels

A-Linter Area: linter 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.

💅 noUnnecessaryConditions: false positive when a literal-initialised object property is mutated

1 participant