feat(lint/js): add noUnsafePlusOperands#9695
Conversation
🦋 Changeset detectedLatest commit: 3495957 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 |
Merging this PR will not alter performance
Comparing 
Footnotes
|
1d3eff3 to
791c763
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new nursery lint rule 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.changeset/purple-pants-ask.md (1)
5-5: Worth calling out the default severity here.
noUnsafePlusOperandsdoes not overrideseverityin its rule declaration, so this lands as an information-level nursery diagnostic by default. A short note here would set expectations for folks skimming the release notes. Based on learnings, when a lint rule does not explicitly set the severity indeclare_lint_rule!, the changeset markdown should mention that the default severity isSeverity::Information.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/purple-pants-ask.md at line 5, The release note should explicitly state the default diagnostic severity for the new nursery rule noUnsafePlusOperands; update the changeset text to mention that because the rule declaration does not set severity in declare_lint_rule!, it will be reported at Severity::Information by default, so readers understand the rule lands as an information-level diagnostic unless the rule's severity is later changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rs`:
- Around line 397-417: analyze_binary_operands currently collects the first
number and bigint from the whole expression tree which misflags valid
left-to-right string-concat chains; change the logic in analyze_binary_operands
to perform a left-to-right evaluation of the binary + chain by walking the
binary expression nodes in evaluation order (using the existing
analyze_expression helper to inspect each operand sequentially and updating
first_number_range/first_bigint_range as you encounter operands) and only emit
MixedBigIntAndNumber when a number and bigint are actually combined in
evaluation order; also add regression tests for examples like "1" + 1n + 1 and 1
+ "1" + 1n to valid.ts and snapshot tests under tests/specs/{group}/{rule}/ to
ensure these no longer produce diagnostics (refer to analyze_binary_operands,
analyze_expression, and NoUnsafePlusOperandsState to locate the change).
- Around line 198-240: The rule skips member assignments because
type_of_assignment returns None for AnyJsAssignment::JsComputedMemberAssignment
and ::JsStaticMemberAssignment; update type_of_assignment to resolve and return
the member's type so compound assignments like obj.total += 1n reach
analyse_pair. Specifically, add match arms for JsStaticMemberAssignment and
JsComputedMemberAssignment that extract the object expression and property/key
(for static use the property name token, for computed use the expression/key),
then call the appropriate type resolution on the context (similar to
ctx.type_of_named_value) to return the member's Type for the given
assignment.range(); keep existing recursive handling style used in
type_of_assignment for parenthesized/ts-* cases and ensure analyse_pair will
receive the resolved Type.
---
Nitpick comments:
In @.changeset/purple-pants-ask.md:
- Line 5: The release note should explicitly state the default diagnostic
severity for the new nursery rule noUnsafePlusOperands; update the changeset
text to mention that because the rule declaration does not set severity in
declare_lint_rule!, it will be reported at Severity::Information by default, so
readers understand the rule lands as an information-level diagnostic unless the
rule's severity is later changed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e03b12a2-9a62-4e19-a093-801c1ab55fd8
⛔ Files ignored due to path filters (9)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/domain_selector.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/invalid.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/valid.ts.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (6)
.changeset/purple-pants-ask.mdcrates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rscrates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/invalid.tscrates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/valid.tscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_unsafe_plus_operands.rs
| fn type_of_assignment_target( | ||
| ctx: &RuleContext<NoUnsafePlusOperands>, | ||
| assignment: &JsAssignmentExpression, | ||
| left: &AnyJsAssignmentPattern, | ||
| ) -> Option<Type> { | ||
| match left { | ||
| AnyJsAssignmentPattern::AnyJsAssignment(assignment_target) => { | ||
| type_of_assignment(ctx, assignment, assignment_target) | ||
| } | ||
| AnyJsAssignmentPattern::JsArrayAssignmentPattern(_) | ||
| | AnyJsAssignmentPattern::JsObjectAssignmentPattern(_) => None, | ||
| } | ||
| } | ||
|
|
||
| fn type_of_assignment( | ||
| ctx: &RuleContext<NoUnsafePlusOperands>, | ||
| assignment: &JsAssignmentExpression, | ||
| target: &AnyJsAssignment, | ||
| ) -> Option<Type> { | ||
| match target { | ||
| AnyJsAssignment::JsIdentifierAssignment(identifier) => { | ||
| let name = identifier.name_token().ok()?; | ||
| Some(ctx.type_of_named_value(assignment.range(), name.text_trimmed())) | ||
| } | ||
| AnyJsAssignment::JsParenthesizedAssignment(parenthesized) => { | ||
| type_of_assignment(ctx, assignment, &parenthesized.assignment().ok()?) | ||
| } | ||
| AnyJsAssignment::TsAsAssignment(ts_as) => { | ||
| type_of_assignment(ctx, assignment, &ts_as.assignment().ok()?) | ||
| } | ||
| AnyJsAssignment::TsNonNullAssertionAssignment(non_null) => { | ||
| type_of_assignment(ctx, assignment, &non_null.assignment().ok()?) | ||
| } | ||
| AnyJsAssignment::TsSatisfiesAssignment(satisfies) => { | ||
| type_of_assignment(ctx, assignment, &satisfies.assignment().ok()?) | ||
| } | ||
| AnyJsAssignment::TsTypeAssertionAssignment(assertion) => { | ||
| type_of_assignment(ctx, assignment, &assertion.assignment().ok()?) | ||
| } | ||
| AnyJsAssignment::JsBogusAssignment(_) | ||
| | AnyJsAssignment::JsComputedMemberAssignment(_) | ||
| | AnyJsAssignment::JsStaticMemberAssignment(_) => None, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full rule file structure
wc -l crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rsRepository: biomejs/biome
Length of output: 128
🏁 Script executed:
# Examine the rule documentation at the beginning of the file
head -70 crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rsRepository: biomejs/biome
Length of output: 2196
🏁 Script executed:
# Check if tests exist for this rule
find . -path "*/tests/specs/*no_unsafe_plus_operands*" -type fRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Look at the structure of test directories
find crates/biome_js_analyze/tests/specs -type d | grep -i unsafe | head -20Repository: biomejs/biome
Length of output: 396
🏁 Script executed:
# Check what test files exist for this rule
find crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands -type fRepository: biomejs/biome
Length of output: 366
🏁 Script executed:
# Let's look at the existing test files
cat crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/invalid.tsRepository: biomejs/biome
Length of output: 994
🏁 Script executed:
# Check if there's a valid.ts file
cat crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/valid.ts 2>/dev/null || echo "No valid.ts found"Repository: biomejs/biome
Length of output: 689
🏁 Script executed:
# Now let's look at the specific lines 198-240 more carefully and check the member assignment handling
sed -n '180,250p' crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rsRepository: biomejs/biome
Length of output: 2604
🏁 Script executed:
# Let's verify the exact names of member assignment variants to confirm they're being skipped
grep -n "JsComputedMemberAssignment\|JsStaticMemberAssignment" crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rsRepository: biomejs/biome
Length of output: 185
🏁 Script executed:
# Check if there are any other usages or handling of member assignments elsewhere in the file
grep -c "JsComputedMemberAssignment\|JsStaticMemberAssignment" crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rsRepository: biomejs/biome
Length of output: 58
🏁 Script executed:
# Verify the rule docs quote about "always checks compound"
grep -n "always checks" crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rsRepository: biomejs/biome
Length of output: 144
🏁 Script executed:
# Let's check if there are any snapshot test results that show member assignment cases
grep -i "member.*+=" crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/invalid.ts.snapRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Double-check what assignment patterns are tested in the invalid.ts snapshot
grep -A5 "total +=" crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/invalid.ts.snap | head -20Repository: biomejs/biome
Length of output: 268
🏁 Script executed:
# Let's verify that member assignments are valid assignment targets in the context
# by checking if there are any TypeScript/JS examples in comments or docs
grep -B5 -A5 "obj\." crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/invalid.tsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Check the rule's actual scope - look for how assignment expressions are matched
grep -n "JsAssignmentExpression\|run_assignment" crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rs | head -20Repository: biomejs/biome
Length of output: 654
🏁 Script executed:
# Let's look at the full run_assignment function to understand the context better
sed -n '171,195p' crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rsRepository: biomejs/biome
Length of output: 755
🏁 Script executed:
# Check if there are any TODO or FIXME comments related to member assignments
grep -n "TODO\|FIXME" crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Verify the AnyJsAssignment enum variants to understand what's available
rg "enum AnyJsAssignment" -A 15 crates/biome_js_syntax/src/Repository: biomejs/biome
Length of output: 2834
Member assignments like obj.total += 1n are currently skipped.
The type_of_assignment function returns None for both JsComputedMemberAssignment and JsStaticMemberAssignment, which means these never reach analyse_pair despite the rule documentation explicitly promising to always check compound += assignments. Test coverage for this case is missing from tests/specs/nursery/noUnsafePlusOperands/invalid.ts — please add member assignment cases once the implementation supports them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rs` around
lines 198 - 240, The rule skips member assignments because type_of_assignment
returns None for AnyJsAssignment::JsComputedMemberAssignment and
::JsStaticMemberAssignment; update type_of_assignment to resolve and return the
member's type so compound assignments like obj.total += 1n reach analyse_pair.
Specifically, add match arms for JsStaticMemberAssignment and
JsComputedMemberAssignment that extract the object expression and property/key
(for static use the property name token, for computed use the expression/key),
then call the appropriate type resolution on the context (similar to
ctx.type_of_named_value) to return the member's Type for the given
assignment.range(); keep existing recursive handling style used in
type_of_assignment for parenthesized/ts-* cases and ensure analyse_pair will
receive the resolved Type.
| fn analyze_binary_operands( | ||
| ctx: &RuleContext<NoUnsafePlusOperands>, | ||
| range: TextRange, | ||
| left: AnyJsExpression, | ||
| right: AnyJsExpression, | ||
| ) -> Option<NoUnsafePlusOperandsState> { | ||
| let mut first_number_range = None; | ||
| let mut first_bigint_range = None; | ||
|
|
||
| analyze_expression(ctx, left, &mut first_number_range, &mut first_bigint_range)?; | ||
| analyze_expression(ctx, right, &mut first_number_range, &mut first_bigint_range)?; | ||
|
|
||
| if let (Some(left_range), Some(right_range)) = (first_number_range, first_bigint_range) { | ||
| return Some(NoUnsafePlusOperandsState::MixedBigIntAndNumber { | ||
| range, | ||
| left_range, | ||
| right_range, | ||
| }); | ||
| } | ||
|
|
||
| None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full implementation of the analyze_binary_operands function
cat -n crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rs | sed -n '397,475p'Repository: biomejs/biome
Length of output: 2808
🏁 Script executed:
# Also, let's check the analyze_expression function to understand how it works
cat -n crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rs | sed -n '1,100p'Repository: biomejs/biome
Length of output: 3694
🏁 Script executed:
# Check the test structure for this rule
find tests/specs -path "*no_unsafe_plus_operands*" -type f | head -20Repository: biomejs/biome
Length of output: 103
🏁 Script executed:
# Search for test files related to no_unsafe_plus_operands
find . -name "*no_unsafe_plus_operands*" -type f 2>/dev/null | head -20Repository: biomejs/biome
Length of output: 185
🏁 Script executed:
# Also check common test directory structures
find . -path "*/tests/*" -name "*unsafe*" -type f 2>/dev/null | head -20Repository: biomejs/biome
Length of output: 1565
🏁 Script executed:
# Look for the test structure for JS analyzer rules
ls -la crates/biome_js_analyze/tests/ 2>/dev/null || echo "No tests dir"
find crates/biome_js_analyze -type d -name "tests" 2>/dev/nullRepository: biomejs/biome
Length of output: 576
🏁 Script executed:
# Check the test specs directory structure for nursery rules
ls -la crates/biome_js_analyze/tests/specs/ | grep -i "nursery\|unsafe"Repository: biomejs/biome
Length of output: 114
🏁 Script executed:
# Look for any test files for no_unsafe_plus_operands
find crates/biome_js_analyze/tests/specs -type d -name "*unsafe*"Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Get all directories in specs to see the structure
ls -la crates/biome_js_analyze/tests/specs/Repository: biomejs/biome
Length of output: 700
🏁 Script executed:
# List the nursery rules to find no_unsafe_plus_operands
ls -la crates/biome_js_analyze/tests/specs/nursery/ | grep -i "unsafe"Repository: biomejs/biome
Length of output: 127
🏁 Script executed:
# Get all nursery rule directories
ls crates/biome_js_analyze/tests/specs/nursery/ | head -20Repository: biomejs/biome
Length of output: 453
🏁 Script executed:
# Check if no_unsafe_plus_operands directory exists
ls -la crates/biome_js_analyze/tests/specs/nursery/no_unsafe_plus_operands/ 2>/dev/null || echo "Directory not found"Repository: biomejs/biome
Length of output: 76
🏁 Script executed:
# Check the noUnsafePlusOperands directory
ls -la crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/Repository: biomejs/biome
Length of output: 420
🏁 Script executed:
# Read the test files to understand current coverage
cat crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/valid.tsRepository: biomejs/biome
Length of output: 689
🏁 Script executed:
# Also check invalid.ts
cat crates/biome_js_analyze/tests/specs/nursery/noUnsafePlusOperands/invalid.tsRepository: biomejs/biome
Length of output: 994
Add regression tests for string-concat chains with mixed numeric types.
This function collects the first number and bigint from anywhere in the entire expression tree. Cases like "1" + 1n + 1 or 1 + "1" + 1n incorrectly trigger the mixed-type diagnostic even though they're valid string concatenations. The fix requires left-to-right evaluation instead of a tree-wide leaf collection.
Please add test cases to valid.ts covering these scenarios. Per the coding guidelines, lint rules require snapshot tests in tests/specs/{group}/{rule}/.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_js_analyze/src/lint/nursery/no_unsafe_plus_operands.rs` around
lines 397 - 417, analyze_binary_operands currently collects the first number and
bigint from the whole expression tree which misflags valid left-to-right
string-concat chains; change the logic in analyze_binary_operands to perform a
left-to-right evaluation of the binary + chain by walking the binary expression
nodes in evaluation order (using the existing analyze_expression helper to
inspect each operand sequentially and updating
first_number_range/first_bigint_range as you encounter operands) and only emit
MixedBigIntAndNumber when a number and bigint are actually combined in
evaluation order; also add regression tests for examples like "1" + 1n + 1 and 1
+ "1" + 1n to valid.ts and snapshot tests under tests/specs/{group}/{rule}/ to
ensure these no longer produce diagnostics (refer to analyze_binary_operands,
analyze_expression, and NoUnsafePlusOperandsState to locate the change).
1b01b5b to
3495957
Compare
Summary
This adds
noUnsafePlusOperands, which is a port of https://typescript-eslint.io/rules/restrict-plus-operands/generated by gpt 5.4, but I had to steer it significantly
Test Plan
snapshots
Docs