Skip to content

fix(biome-js-analyze): add a check for if/ else and ternary operators…#6524

Merged
vladimir-ivanov merged 1 commit intobiomejs:mainfrom
vladimir-ivanov:fix/useReadonlyClassProperties-set-ignored
Jun 24, 2025
Merged

fix(biome-js-analyze): add a check for if/ else and ternary operators…#6524
vladimir-ivanov merged 1 commit intobiomejs:mainfrom
vladimir-ivanov:fix/useReadonlyClassProperties-set-ignored

Conversation

@vladimir-ivanov
Copy link
Copy Markdown
Contributor

@vladimir-ivanov vladimir-ivanov commented Jun 24, 2025 •
edited
Loading

Summary

Fixed an issue where class properties that are set in the constructor were not being marked as readonly when they
should be if found in if/ else condition.

Example:

class Price {
 #price: string;

 @Input()
 set some(value: string | number) {
   if (value === undefined || value === null || value === 'undefined' || value === 'null' || Number.isNaN(value)) {
     this.#price = '';
   } else {
     this.#price = '' + value;
   }
 }
}

closes #6500

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 24, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 7ac70fa

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 Jun 24, 2025
vladimir-ivanov
vladimir-ivanov commented Jun 24, 2025
View reviewed changes
Comment thread crates/biome_js_analyze/src/lint/nursery/use_readonly_class_properties.rs Outdated
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 24, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #6524 will not alter performance

Comparing vladimir-ivanov:fix/useReadonlyClassProperties-set-ignored (7ac70fa) with main (a3a3715)

Summary

✅ 115 untouched benchmarks

ematipico
ematipico reviewed Jun 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

I left some questions around the code, to have more clarity. Also left some comments around the changeset

Comment thread .changeset/use_readonly_class_properties_setter_fix.md Outdated
Comment thread .changeset/use_readonly_class_properties_setter_fix.md Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/use_readonly_class_properties.rs Outdated
Comment thread crates/biome_js_analyze/src/lint/nursery/use_readonly_class_properties.rs Outdated
ematipico
ematipico approved these changes Jun 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you! 👏

@vladimir-ivanov vladimir-ivanov force-pushed the fix/useReadonlyClassProperties-set-ignored branch from 4eb012c to 62e0a7b Compare June 24, 2025 13:22
@github-actions github-actions Bot added A-CLI Area: CLI A-Project Area: project A-Parser Area: parser A-Formatter Area: formatter L-CSS Language: CSS and super languages L-JSON Language: JSON and super languages A-Diagnostic Area: diagnostocis L-Grit Language: GritQL A-Resolver Area: resolver labels Jun 24, 2025
@vladimir-ivanov vladimir-ivanov force-pushed the fix/useReadonlyClassProperties-set-ignored branch from 62e0a7b to aece21b Compare June 24, 2025 13:26
@github-actions github-actions Bot removed A-Project Area: project A-Parser Area: parser A-Formatter Area: formatter L-CSS Language: CSS and super languages A-Diagnostic Area: diagnostocis L-Grit Language: GritQL A-Resolver Area: resolver labels Jun 24, 2025
@vladimir-ivanov vladimir-ivanov force-pushed the fix/useReadonlyClassProperties-set-ignored branch from dfc5c45 to 7ac70fa Compare June 24, 2025 14:34
@vladimir-ivanov vladimir-ivanov merged commit a27b825 into biomejs:main Jun 24, 2025
28 checks passed
@vladimir-ivanov vladimir-ivanov deleted the fix/useReadonlyClassProperties-set-ignored branch June 24, 2025 15:07
@github-actions github-actions Bot mentioned this pull request Jun 24, 2025
Merged
@burka
Copy link
Copy Markdown

burka commented Jun 25, 2025

Yeah great! That works, I tested it on my code :-) Thanks @vladimir-ivanov for that fast fix!

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

Reviewers

@ematipico ematipico ematipico approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

useReadonlyClassProperties ignores accessor writes (typescript)

3 participants

@vladimir-ivanov @burka @ematipico