feat(lint): add noUselessCall rule#10237
Conversation
🦋 Changeset detectedLatest commit: a3fdc82 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 |
github-actions
Bot
added
A-Project
WalkthroughAdds the Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/add-no-useless-call.md:
- Line 5: Update the changeset sentence to include the docs link for the rule by
replacing the plain rule name with the required markdown link format;
specifically change the mention of noUselessCall to
[`noUselessCall`](https://biomejs.dev/linter/rules/no-useless-call/) in
.changeset/add-no-useless-call.md so it follows the "Include links to rule
documentation" guideline.
In `@crates/biome_js_analyze/src/lint/nursery/no_useless_call.rs`:
- Around line 159-173: The function are_same_simple_reference currently only
matches identifier expressions; add a new match arm to treat this expressions as
equal by matching (AnyJsExpression::JsThisExpression(_),
AnyJsExpression::JsThisExpression(_)) => true before the existing
AnyJsExpression::JsIdentifierExpression arm so calls like this.foo.call(this,
...) and this.foo.apply(this, [...]) are recognized; also add test cases to
invalid.js for this.foo.call(this, 1) and this.foo.apply(this, [1]) to cover the
regressions.
🪄 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: cecc3576-19be-4abc-b612-7318e97c03c0
⛔ Files ignored due to path filters (6)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand 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/noUselessCall/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noUselessCall/valid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (6)
.changeset/add-no-useless-call.mdcrates/biome_js_analyze/src/lint/nursery/no_useless_call.rscrates/biome_js_analyze/tests/specs/nursery/noUselessCall/invalid.jscrates/biome_js_analyze/tests/specs/nursery/noUselessCall/valid.jscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_useless_call.rs
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Fixed [#10187](https://github.com/biomejs/biome/issues/10187): Added the `noUselessCall` lint rule to report unnecessary uses of `Function.prototype.call()` and `Function.prototype.apply()`. |
There was a problem hiding this comment.
Add the rule docs link in the changeset sentence.
Line 5 references the rule name but misses the required website link format. Tiny docs nit, big changelog win.
Suggested edit
-Fixed [`#10187`](https://github.com/biomejs/biome/issues/10187): Added the `noUselessCall` lint rule to report unnecessary uses of `Function.prototype.call()` and `Function.prototype.apply()`.
+Fixed [`#10187`](https://github.com/biomejs/biome/issues/10187): Added the [`noUselessCall`](https://biomejs.dev/linter/rules/no-useless-call/) lint rule to report unnecessary uses of `Function.prototype.call()` and `Function.prototype.apply()`.As per coding guidelines: "Include links to rule documentation on the website when referencing rules in changesets, formatted as: ruleName".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Fixed [#10187](https://github.com/biomejs/biome/issues/10187): Added the `noUselessCall` lint rule to report unnecessary uses of `Function.prototype.call()` and `Function.prototype.apply()`. | |
| Fixed [`#10187`](https://github.com/biomejs/biome/issues/10187): Added the [`noUselessCall`](https://biomejs.dev/linter/rules/no-useless-call/) lint rule to report unnecessary uses of `Function.prototype.call()` and `Function.prototype.apply()`. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/add-no-useless-call.md at line 5, Update the changeset sentence
to include the docs link for the rule by replacing the plain rule name with the
required markdown link format; specifically change the mention of noUselessCall
to [`noUselessCall`](https://biomejs.dev/linter/rules/no-useless-call/) in
.changeset/add-no-useless-call.md so it follows the "Include links to rule
documentation" guideline.
| fn are_same_simple_reference(left: &AnyJsExpression, right: &AnyJsExpression) -> bool { | ||
| match (left.clone().omit_parentheses(), right.clone().omit_parentheses()) { | ||
| (AnyJsExpression::JsIdentifierExpression(left), AnyJsExpression::JsIdentifierExpression(right)) => { | ||
| left.name() | ||
| .ok() | ||
| .zip(right.name().ok()) | ||
| .is_some_and(|(left, right)| { | ||
| left.value_token() | ||
| .ok() | ||
| .zip(right.value_token().ok()) | ||
| .is_some_and(|(left, right)| left.text_trimmed() == right.text_trimmed()) | ||
| }) | ||
| } | ||
| _ => false, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does ESLint's no-useless-callreportthis.foo.call(this, 1)andthis.foo.apply(this, [1]) as violations?
💡 Result:
Yes. ESLint’s no-useless-call rule reports both this.foo.call(this, 1) and this.foo.apply(this, [1]) as violations, because they match the rule’s “same as obj.foo(...);” pattern where the this value passed to .call/.apply is the same receiver (thisArg equals the method’s owning object). ESLint’s rule documentation shows these as incorrect examples: - obj.foo.call(obj, 1, 2, 3); - obj.foo.apply(obj, [1, 2, 3]); Substituting obj -> this and the arguments accordingly yields the reported violations for this.foo.call(this, 1) and this.foo.apply(this, [1]).
Citations:
🏁 Script executed:
# First, let's look at the current implementation and context
cd crates/biome_js_analyze && cat -n src/lint/nursery/no_useless_call.rs | sed -n '150,180p'Repository: biomejs/biome
Length of output: 1168
🏁 Script executed:
# Check if there are test files for this rule
find . -path "*/tests/*no_useless_call*" -type f | head -20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Look for the invalid.js fixture file
find . -path "*/no_useless_call/*" -name "invalid.js" -o -path "*/no_useless_call/*" -name "*.js" | head -10Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Search for test files related to no_useless_call
find . -type f -name "*.js" | xargs grep -l "no_useless_call" 2>/dev/null | head -20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Look for the specs directory and this rule
find . -type d -name "specs" | xargs find 2>/dev/null | grep -i useless_call | head -20Repository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Try a broader search for invalid.js files in test directories
find . -path "*/tests/*" -name "invalid.js" 2>/dev/null | xargs grep -l "call\|apply" 2>/dev/null | head -10Repository: biomejs/biome
Length of output: 788
🏁 Script executed:
# Read the invalid.js test file for noUselessCall
cat -n crates/biome_js_analyze/tests/specs/nursery/noUselessCall/invalid.jsRepository: biomejs/biome
Length of output: 328
🏁 Script executed:
# Look at how are_same_simple_reference is used in the file
cat -n crates/biome_js_analyze/src/lint/nursery/no_useless_call.rs | grep -B5 -A5 "are_same_simple_reference"Repository: biomejs/biome
Length of output: 1023
🏁 Script executed:
# Check the valid.js file as well
cat crates/biome_js_analyze/tests/specs/nursery/noUselessCall/valid.jsRepository: biomejs/biome
Length of output: 504
Add JsThisExpression to are_same_simple_reference to catch this.foo.call(this, …) and this.foo.apply(this, […]) patterns.
The function only handles identifier comparisons, so this expressions always fail to match. ESLint's no-useless-call rule reports these as violations. Add a match arm for (AnyJsExpression::JsThisExpression(_), AnyJsExpression::JsThisExpression(_)) => true before the identifier case, and add test cases to invalid.js for this.foo.call(this, 1) and this.foo.apply(this, [1]).
🤖 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_useless_call.rs` around lines 159
- 173, The function are_same_simple_reference currently only matches identifier
expressions; add a new match arm to treat this expressions as equal by matching
(AnyJsExpression::JsThisExpression(_), AnyJsExpression::JsThisExpression(_)) =>
true before the existing AnyJsExpression::JsIdentifierExpression arm so calls
like this.foo.call(this, ...) and this.foo.apply(this, [...]) are recognized;
also add test cases to invalid.js for this.foo.call(this, 1) and
this.foo.apply(this, [1]) to cover the regressions.
Merging this PR will not alter performance
Comparing 
Footnotes
|
| rule_category!(), | ||
| call.range(), | ||
| markup! { | ||
| "This "<Emphasis>"."{method}"()"</Emphasis>" is unnecessary." |
There was a problem hiding this comment.
Insufficient diagnostics, please follow the 3 pillars. The Why and What the user should do are missing
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/no_useless_call.rs (1)
188-202:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
are_same_simple_referencesilently dropsthisexpressions, sothis.foo.call(this, …)is never flagged.The wildcard arm (
_ => false) swallows every non-identifier pair, including twoJsThisExpressionnodes that are trivially equal. Adding a single match arm restores parity with ESLint'sno-useless-call.🐛 Proposed fix
fn are_same_simple_reference(left: &AnyJsExpression, right: &AnyJsExpression) -> bool { match (left.clone().omit_parentheses(), right.clone().omit_parentheses()) { + (AnyJsExpression::JsThisExpression(_), AnyJsExpression::JsThisExpression(_)) => true, (AnyJsExpression::JsIdentifierExpression(left), AnyJsExpression::JsIdentifierExpression(right)) => { left.name() .ok() .zip(right.name().ok()) .is_some_and(|(left, right)| { left.value_token() .ok() .zip(right.value_token().ok()) .is_some_and(|(left, right)| left.text_trimmed() == right.text_trimmed()) }) } _ => false, } }Don't forget to add
this.foo.call(this, 1)andthis.foo.apply(this, [1])toinvalid.js.🤖 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_useless_call.rs` around lines 188 - 202, The helper are_same_simple_reference currently returns false for any non-identifier pairs because the wildcard arm swallows cases like JsThisExpression, so calls like this.foo.call(this, …) are not detected; add a match arm to treat two JsThisExpression operands as equal (i.e., return true when both left and right are AnyJsExpression::JsThisExpression after omit_parentheses) and keep the existing identifier comparison intact in are_same_simple_reference, then add test cases this.foo.call(this, 1) and this.foo.apply(this, [1]) to invalid.js to cover these paths.
🤖 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_useless_call.rs`:
- Around line 86-88: The rule currently early-returns for CallMethod::Apply
whenever has_array_argument(call)? yields Some(false), which incorrectly skips
cases like foo.apply(null) (one argument) that should be treated like
foo.call(null); change the logic in no_useless_call.rs so that when method ==
CallMethod::Apply and has_array_argument(call)? == Some(false) you only return
None for calls that are NOT the one-argument form (i.e., check
call.arguments.len() and allow len == 1 to continue); reference
CallMethod::Apply, has_array_argument(call), and the call.arguments length check
when implementing this conditional.
---
Duplicate comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_useless_call.rs`:
- Around line 188-202: The helper are_same_simple_reference currently returns
false for any non-identifier pairs because the wildcard arm swallows cases like
JsThisExpression, so calls like this.foo.call(this, …) are not detected; add a
match arm to treat two JsThisExpression operands as equal (i.e., return true
when both left and right are AnyJsExpression::JsThisExpression after
omit_parentheses) and keep the existing identifier comparison intact in
are_same_simple_reference, then add test cases this.foo.call(this, 1) and
this.foo.apply(this, [1]) to invalid.js to cover these paths.
🪄 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: d0525cb6-8079-4fb9-b9bf-13fdbdda6797
⛔ Files ignored due to path filters (3)
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_js_analyze/tests/specs/nursery/noUselessCall/invalid.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_useless_call.rs
| if method == CallMethod::Apply && !has_array_argument(call)? { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
foo.apply(null) / foo.apply(undefined) (no second argument) is silently skipped, unlike the equivalent .call() forms.
has_array_argument returns Some(false) when the argument count is not exactly 2, causing the early return None. However, foo.apply(null) with no spread argument is semantically identical to foo(), just like foo.call(null) — and the .call path does flag this. foo.apply(undefined, args) is listed as valid (dynamic args), but ESLint's rule is otherwise aimed at statically-decidable useless calls. Allowing the 1-arg .apply() path through closes the inconsistency:
🐛 Proposed fix
- if method == CallMethod::Apply && !has_array_argument(call)? {
- return None;
- }
+ if method == CallMethod::Apply {
+ let apply_arg_count = call.arguments().ok()?.args().len();
+ // 1-arg .apply(thisArg) is equivalent to .call(thisArg) — allow it through.
+ // 2-arg .apply() is only flaggable when the second arg is a static array.
+ // Any other arity is too dynamic to analyse safely.
+ if apply_arg_count == 2 && !has_array_argument(call)? {
+ return None;
+ } else if apply_arg_count > 2 {
+ return None;
+ }
+ }🤖 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_useless_call.rs` around lines 86
- 88, The rule currently early-returns for CallMethod::Apply whenever
has_array_argument(call)? yields Some(false), which incorrectly skips cases like
foo.apply(null) (one argument) that should be treated like foo.call(null);
change the logic in no_useless_call.rs so that when method == CallMethod::Apply
and has_array_argument(call)? == Some(false) you only return None for calls that
are NOT the one-argument form (i.e., check call.arguments.len() and allow len ==
1 to continue); reference CallMethod::Apply, has_array_argument(call), and the
call.arguments length check when implementing this conditional.
2 participants
Summary
Implements the new nursery lint rule
noUselessCall, which ports ESLint'sno-useless-callrule to Biome.This rule reports unnecessary uses of
.call()and.apply()when they can be replaced with a normal function call without changing thethisbinding or argument semantics.Examples reported by this rule include:
The implementation is conservative and avoids reporting cases where replacing .call() or .apply() could change behavior, such as calls with a different receiver or .apply() calls whose second argument is not a static array expression.
Closes #10187.
Test Plan
Generated snapshots for the new rule.
Commands run:
cargo test -p biome_js_analyze no_useless_call