feat(lint): implement useReduceTypeParameter nursery rule#9577
feat(lint): implement useReduceTypeParameter nursery rule#9577tt-a1i wants to merge 8 commits intobiomejs:mainfrom
Conversation
Add a new lint rule that enforces using a type parameter on Array#reduce and Array#reduceRight instead of type assertions on the initial value. Closes biomejs#8487
🦋 Changeset detectedLatest commit: 7c868d5 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
|
| 1 1 │ // basic reduce with as assertion | ||
| 2 2 │ const arr1: number[] = [1, 2, 3]; | ||
| 3 │ - arr1.reduce((sum,·num)·=>·sum.concat(num·*·2),·[]·as·number[]); | ||
| 3 │ + arr1.reduce<number[]>((sum,·num)·=>·sum.concat(num·*·2),·[]); |
There was a problem hiding this comment.
Are we sure we need type inferencing in order to implement this rule?
There was a problem hiding this comment.
good point — I added Typed<> mainly to avoid false positives on custom classes with a reduce method (e.g. some Redux-style store). the type check confirms the receiver is actually an array/tuple before reporting.
but realistically, .reduce(cb, init as T) on a non-array is pretty rare, and useFlatMap does fine without type info for a similar pattern. so if you think the overhead isn't worth it I'm happy to drop it and go syntax-only.
There was a problem hiding this comment.
Yeah, let's not have Typed<> if we can avoid it.
There was a problem hiding this comment.
done in 224b18e: switched the rule to syntax-only (Ast<JsCallExpression>), removed RuleDomain::Types + array/tuple type checks, and updated the valid fixture/snapshots accordingly.
crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs
Outdated
Show resolved
Hide resolved
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces a new nursery linter rule Suggested labels
Suggested reviewers
🚥 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)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.changeset/use-reduce-type-parameter-1774115911.md (1)
5-15: Release notes don’t needexpect_diagnostic.The before/after snippet is handy, but the fence hint and the receiver-type inference note read more like reviewer notes than release-note copy. I’d keep this to the user-visible behaviour and autofix outcome. As per coding guidelines, "Changeset descriptions must target end users (explain impact, not implementation) and be concise (1-3 sentences)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/use-reduce-type-parameter-1774115911.md around lines 5 - 15, Update the changeset text for the new nursery rule useReduceTypeParameter so it reads like a concise user-facing release note (1–3 sentences) describing the visible behavior and autofix outcome (e.g., "Enforces using a type parameter on Array#reduce and Array#reduceRight instead of a type assertion; will autofix to add the type parameter"). Remove the fenced "```ts,expect_diagnostic" hint and the implementation/receiver-type inference note about array/tuple receivers — keep only the end-user impact and autofix result.
🤖 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/use_reduce_type_parameter.rs`:
- Around line 127-142: The diagnostic message incorrectly hardcodes
"Array#reduce" — change it to use the matched member name from the query so it
reads "Array#<member>" (e.g., "Array#reduceRight" when appropriate); locate the
diagnostic function (fn diagnostic) and replace the hardcoded string in
RuleDiagnostic::new (currently using call.syntax().text_trimmed_range() and
markup! { "Use a type parameter on \"Array#reduce\" ..." }) to interpolate the
actual member identifier from the matched call expression, and make the same
replacement for the other message/action text at the later occurrence referenced
(around the block at lines 181-184) so both diagnostic and fix text reflect the
actual member name.
- Around line 91-94: The early return that skips checking when
call.type_arguments().is_some() causes a false negative (e.g.,
arr.reduce<Foo>(..., expr as Foo)) — update the logic around the call site that
currently does "if call.type_arguments().is_some() { return None; }" so that you
preserve/keep the type arguments but still unwrap and inspect the second
argument (the initial value) for an as-cast assertion; specifically, in the
function handling reduce invocations (the code that checks call.type_arguments()
and then inspects call.arguments()/call.args().nth(1) or similar), remove the
unconditional return and instead continue to evaluate the second parameter for
an as-expression even when type_arguments() is present; apply the same change to
the analogous block in the 150-177 region so reduce<T>(..., expr as T) is caught
and add a regression test fixture for that shape.
---
Nitpick comments:
In @.changeset/use-reduce-type-parameter-1774115911.md:
- Around line 5-15: Update the changeset text for the new nursery rule
useReduceTypeParameter so it reads like a concise user-facing release note (1–3
sentences) describing the visible behavior and autofix outcome (e.g., "Enforces
using a type parameter on Array#reduce and Array#reduceRight instead of a type
assertion; will autofix to add the type parameter"). Remove the fenced
"```ts,expect_diagnostic" hint and the implementation/receiver-type inference
note about array/tuple receivers — keep only the end-user impact and autofix
result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a8d797a8-7545-432d-a5c2-508a3caa0a3c
⛔ Files ignored due to path filters (11)
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/useReduceTypeParameter/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/invalid.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/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 (8)
.changeset/use-reduce-type-parameter-1774115911.mdcrates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rscrates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/invalid.jscrates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/invalid.tscrates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/valid.jscrates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/valid.tscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_reduce_type_parameter.rs
| // already has type arguments like .reduce<Foo>(...) | ||
| if call.type_arguments().is_some() { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
Don’t give reduce<T> a free pass.
This early return misses arr.reduce<Foo>(..., expr as Foo), even though the assertion still suppresses checking of the initial value. The fix can preserve the existing type arguments and just unwrap the second argument, so this is a real false negative for the core rule; I’d add a regression fixture for that shape too.
Also applies to: 150-177
🤖 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/use_reduce_type_parameter.rs` around
lines 91 - 94, The early return that skips checking when
call.type_arguments().is_some() causes a false negative (e.g.,
arr.reduce<Foo>(..., expr as Foo)) — update the logic around the call site that
currently does "if call.type_arguments().is_some() { return None; }" so that you
preserve/keep the type arguments but still unwrap and inspect the second
argument (the initial value) for an as-cast assertion; specifically, in the
function handling reduce invocations (the code that checks call.type_arguments()
and then inspects call.arguments()/call.args().nth(1) or similar), remove the
unconditional return and instead continue to evaluate the second parameter for
an as-expression even when type_arguments() is present; apply the same change to
the analogous block in the 150-177 region so reduce<T>(..., expr as T) is caught
and add a regression test fixture for that shape.
crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs
Outdated
Show resolved
Hide resolved
- Use value_token().token_text_trimmed() instead of to_trimmed_text() to avoid string allocation when matching method name - Fix diagnostic message to follow rule pillars: describe the problem (WHAT) instead of the solution (HOW) - Distinguish reduce vs reduceRight in diagnostic message - Remove redundant second .note() that duplicated the action message
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs (1)
84-87:⚠️ Potential issue | 🟠 MajorStill skips the
reduce<T>(..., seed as T)case.Line 85 bails out before the seed is inspected, so
arr.reduce<Foo>(cb, init as Foo)is still a false negative. Please keep the existing type arguments and only unwrap the asserted initial value here.🤖 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/use_reduce_type_parameter.rs` around lines 84 - 87, The current guard returns early when call.type_arguments().is_some(), which skips cases like arr.reduce<Foo>(cb, init as Foo); instead, keep existing type arguments and only inspect/unpack the asserted initial value expression: remove the early return and, where you handle the seed (the second argument on call), detect a TypeAssertion or AsExpr and extract its inner expression while preserving call.type_arguments(); update the logic that reads the seed to use the unwrapped inner expression but do not remove or ignore call.type_arguments() or other type arguments.
🤖 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/use_reduce_type_parameter.rs`:
- Around line 89-102: The current check only compares the member name
(method_name) so any .reduce/.reduceRight on non-array receivers will match;
restrict the receiver by inspecting member_expr.object() before reporting: only
proceed if member_expr.object() is an array literal (use
object.as_js_array_expression()), or a new Array expression
(object.as_js_new_expression() whose callee token_text_trimmed() == "Array"), or
an Array.from call (object.as_js_call_expression() whose callee resolves to
"Array.from"); if none of those hold, return None. Keep the existing
method_name/is_reduce_right logic but gate it behind this tighter receiver check
to avoid suggesting <T> on non-generic APIs.
---
Duplicate comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs`:
- Around line 84-87: The current guard returns early when
call.type_arguments().is_some(), which skips cases like arr.reduce<Foo>(cb, init
as Foo); instead, keep existing type arguments and only inspect/unpack the
asserted initial value expression: remove the early return and, where you handle
the seed (the second argument on call), detect a TypeAssertion or AsExpr and
extract its inner expression while preserving call.type_arguments(); update the
logic that reads the seed to use the unwrapped inner expression but do not
remove or ignore call.type_arguments() or other type arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c65ea120-70c9-4a08-a4a4-0a15618ff7ba
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/invalid.ts.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/valid.ts.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rscrates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/valid.ts
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/valid.ts
| let callee = call.callee().ok()?.omit_parentheses(); | ||
| let member_expr = callee.as_js_static_member_expression()?; | ||
|
|
||
| let method_name = member_expr | ||
| .member() | ||
| .ok()? | ||
| .as_js_name()? | ||
| .value_token() | ||
| .ok()? | ||
| .token_text_trimmed(); | ||
| if method_name != "reduce" && method_name != "reduceRight" { | ||
| return None; | ||
| } | ||
| let is_reduce_right = method_name == "reduceRight"; |
There was a problem hiding this comment.
This matches any .reduce/.reduceRight, not just arrays.
The guard is only the member name, so custom reducer methods will get the same diagnostic and fix. That can add <T> to a non-generic API and produce a wrong rewrite. If this rule must stay syntax-only, it still needs a tighter receiver gate than the method name alone.
🤖 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/use_reduce_type_parameter.rs` around
lines 89 - 102, The current check only compares the member name (method_name) so
any .reduce/.reduceRight on non-array receivers will match; restrict the
receiver by inspecting member_expr.object() before reporting: only proceed if
member_expr.object() is an array literal (use object.as_js_array_expression()),
or a new Array expression (object.as_js_new_expression() whose callee
token_text_trimmed() == "Array"), or an Array.from call
(object.as_js_call_expression() whose callee resolves to "Array.from"); if none
of those hold, return None. Keep the existing method_name/is_reduce_right logic
but gate it behind this tighter receiver check to avoid suggesting <T> on
non-generic APIs.
| arr.reduce<number[]>((acc, x) => acc.concat(x), []); | ||
| ``` | ||
|
|
||
| The rule uses type information to only trigger on actual array or tuple receivers, avoiding false positives on custom objects with a `reduce` method. |
There was a problem hiding this comment.
The implementation doesn't look using type information
There was a problem hiding this comment.
you're right, that was stale after switching to syntax-only. updated the changeset in the latest push.
There was a problem hiding this comment.
good catch — updated in eebeffa. I rewrote the changeset to user-facing behavior/autofix wording and removed the implementation-specific wording.
Remove outdated claim about type information usage and remove expect_diagnostic from code block.
crates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/invalid.js
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/tests/specs/nursery/useReduceTypeParameter/valid.js
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs
Outdated
Show resolved
Hide resolved
- Remove useless .js placeholder files and their snapshots - Remove file= attributes from doc examples - Merge valid examples into a single code block
There was a problem hiding this comment.
♻️ Duplicate comments (2)
crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs (2)
78-81:⚠️ Potential issue | 🟠 MajorDon’t skip asserted initial values just because
<T>is present.Line 79 returns early, so
arr.reduce<Foo>(..., expr as Foo)is still missed. That’s a core false negative for this rule. Keep checking arg#2for assertions even when type arguments already exist, and in that path only unwrap the assertion (don’t re-insert type args).#!/bin/bash set -euo pipefail # 1) Confirm the early-return guard is still present rg -n -C2 'type_arguments\(\)\.is_some\(\)' crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs # 2) Check whether regression fixtures include reduce<T>(..., <assertion>) fd 'invalid\.ts$' crates -x rg -n -C2 'reduce<[^>]+>\s*\(.*(as\s+[A-Za-z_]|<[^>]+>)'🤖 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/use_reduce_type_parameter.rs` around lines 78 - 81, The early return on call.type_arguments().is_some() in the use_reduce_type_parameter lint causes false negatives for calls like arr.reduce<Foo>(..., expr as Foo); change the logic in use_reduce_type_parameter so that you do not return early when type arguments exist: always inspect the second argument (arg `#2`) for an assertion/cast, and if you find an as-cast only suggest removing the cast (unwrap the assertion) when type args are already present; keep the existing branch that inserts a type argument only for calls that lack type arguments, but in the “has type args” path avoid re-inserting type args and only produce the fix that removes the assertion on the initializer.
70-95:⚠️ Potential issue | 🟠 MajorReceiver-type gate is still missing; this can hit custom
.reduce()APIs.The query is
Ast<JsCallExpression>and the match is effectively method-name based (Lines 86-95). That still allows false positives on non-array methods namedreduce/reduceRight, with potentially wrong fixes. Please gate via type info so only actual arrays/tuples are targeted.#!/bin/bash set -euo pipefail # Inspect current rule query and method-name gate rg -n -C2 'type Query = .*JsCallExpression' crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs rg -n -C2 'as_js_static_member_expression|method_name != "reduce"|method_name != "reduceRight"' \ crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs # Compare with other lint rules using typed/type-domain patterns rg -n 'Typed<JsCallExpression>|RuleDomain::Types' crates/biome_js_analyze/src/lint -g '*.rs'🤖 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/use_reduce_type_parameter.rs` around lines 70 - 95, The rule currently matches any call named "reduce"/"reduceRight" because Query = Ast<JsCallExpression> and only checks method_name; change the query to carry type information and gate on the receiver's type: set type Query = Typed<JsCallExpression> (or otherwise fetch type info via the rule domain/types APIs), then in run(...) inspect the member_expr.object() type (e.g., via the typed node or ctx.type_of / type_domain helper) and only proceed when that type is an Array or Tuple (or other concrete array-like types you intend to support); keep the existing method_name check and early returns but add this receiver-type check before producing a signal so custom non-array .reduce implementations are ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs`:
- Around line 78-81: The early return on call.type_arguments().is_some() in the
use_reduce_type_parameter lint causes false negatives for calls like
arr.reduce<Foo>(..., expr as Foo); change the logic in use_reduce_type_parameter
so that you do not return early when type arguments exist: always inspect the
second argument (arg `#2`) for an assertion/cast, and if you find an as-cast only
suggest removing the cast (unwrap the assertion) when type args are already
present; keep the existing branch that inserts a type argument only for calls
that lack type arguments, but in the “has type args” path avoid re-inserting
type args and only produce the fix that removes the assertion on the
initializer.
- Around line 70-95: The rule currently matches any call named
"reduce"/"reduceRight" because Query = Ast<JsCallExpression> and only checks
method_name; change the query to carry type information and gate on the
receiver's type: set type Query = Typed<JsCallExpression> (or otherwise fetch
type info via the rule domain/types APIs), then in run(...) inspect the
member_expr.object() type (e.g., via the typed node or ctx.type_of / type_domain
helper) and only proceed when that type is an Array or Tuple (or other concrete
array-like types you intend to support); keep the existing method_name check and
early returns but add this receiver-type check before producing a signal so
custom non-array .reduce implementations are ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e4bab03e-ae88-4fd8-8066-391625f67e50
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/use_reduce_type_parameter.rs
Summary
Implements the nursery lint rule
useReduceTypeParameteras requested in #8487. This rule enforces using a type parameter onArray#reduceandArray#reduceRightinstead of type assertions (asor angle bracket<T>) on the initial value.Before:
After (with autofix):
Inspired by typescript-eslint's
prefer-reduce-type-parameter. Marked as.inspired()rather than.same()because Biome uses its own type inference (TypedService) rather than the TypeScript compiler API.Key design decisions:
Typed<JsCallExpression>withRuleDomain::Typesto verify the receiver is actually anArrayor tuple, avoiding false positives on custom classes with areducemethod.as Typeand<Type>exprassertion syntax, including parenthesized forms like(expr as Type).FixKind::Unsafebecause without full type assignability checks, the fix may not always be semantically equivalent.Test Plan
cargo test -p biome_js_analyze --test spec_tests -- use_reduce_type_parameter(4/4 passing)just lint-rulespassingjust fpassingInvalid cases tested: basic reduce, object accumulator, reduceRight, tuple/union/intersection types, angle bracket syntax, parenthesized assertions, tuple receivers.
Valid cases tested: no initial value, no assertion, already has type parameter, satisfies expression, custom class with reduce method, non-reduce methods.
Docs
Rule documentation is inline rustdoc with Invalid/Valid examples.
AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.