feat(lint): add nursery rule noUselessReturn#9029
Conversation
🦋 Changeset detectedLatest commit: 3a444e4 The changes in this PR will be included in the next version bump. 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 |
WalkthroughAdds a new JavaScript lint rule 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)
No actionable comments were generated in the recent review. 🎉 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: 1
🤖 Fix all issues with AI agents
In `@crates/biome_js_analyze/src/lint/nursery/no_useless_return.rs`:
- Line 75: The rule registration currently marks the ESLint mapping as identical
using RuleSource::Eslint("no-useless-return").same(), but the implementation
diverges from ESLint (AST-only vs CFG behavior); change the mapping to
.inspired() so it reflects different behavior. Locate the use of
RuleSource::Eslint("no-useless-return") in the no_useless_return rule
registration and replace .same() with .inspired(), ensuring any documentation or
rule metadata that relies on sameness is updated accordingly.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_useless_return.rs (1)
1-7: Nit: inconsistent import style forbiome_js_syntaxtypes.Some types (
JsFinallyClause,JsFunctionBody, etc.) are imported at the top, while others (JsBlockStatement,JsIfStatement,JsModule,JsScript, etc.) are used fully-qualified in the function bodies (lines 102–103, 215–221). Either approach is fine, but mixing both is a bit untidy.Add missing imports and use short names throughout
use biome_js_syntax::{ - JsDoWhileStatement, JsFinallyClause, JsForInStatement, JsForOfStatement, JsForStatement, - JsFunctionBody, JsReturnStatement, JsStatementList, JsSwitchStatement, JsWhileStatement, + JsBlockStatement, JsCatchClause, JsDoWhileStatement, JsElseClause, JsFinallyClause, + JsForInStatement, JsForOfStatement, JsForStatement, JsFunctionBody, JsIfStatement, + JsLabeledStatement, JsModule, JsReturnStatement, JsScript, JsStatementList, + JsSwitchStatement, JsTryFinallyStatement, JsTryStatement, JsWhileStatement, };
dyc3
left a comment
There was a problem hiding this comment.
We have ControlFlowGraph for analyzing control flow. Would that make the implementation of this rule more simple?
Merging this PR will not alter performance
Comparing Footnotes
|
|
I initially tried a CFG-based implementation using ControlFlowGraph as the query type, but the code grew to ~350 lines with additional dependencies (RoaringBitmap, FxHashSet) and required traversing all CFG blocks with forward-flow analysis to determine if a return was useless. Since the core problem — "is this return; the last statement before the function ends?" — is fundamentally a positional question rather than a reachability one, the AST approach answers it more naturally by walking ancestors to check tail position (~170 lines, no extra dependencies). If you'd like to review the CFG-based approach, I can share that implementation for comparison. |
|
Makes sense, thanks for the explanation. |
Summary
Closes #8205.
Port ESLint's
no-useless-returnrule to Biome as a nursery rulenoUselessReturn.A bare
return;at the end of a function body is redundant — removing it does not change the function's behavior. This ruledetects such cases and provides a safe fix to remove them.
The implementation uses an AST-based approach: for each
JsReturnStatementwithout an argument, it walks ancestors to check whether the return is in tail position relative to its enclosing function. It bails out for returns inside loops, switchstatements, and finally blocks.
Test Plan
crates/biome_js_analyze/tests/specs/nursery/noUselessReturn/invalid.js: 12 cases including end-of-function, after statements, if/else branches, nested ifs, try/catch blocks, arrowfunctions, labeled statements, and try-finally.
valid.js: 14 cases including return with value, early returns, loops, switch, finally block, empty function, and nestedfunction scopes.
cargo test -p biome_js_analyze -- no_useless_return.Docs
Rule documentation is provided via inline rustdoc in the rule implementation, including invalid and valid examples with
expect_diagnosticannotations.