-
-
Notifications
You must be signed in to change notification settings - Fork 725
feat(biome_js_analyze): add UseConsistentArrowReturn rule #7245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(biome_js_analyze): add UseConsistentArrowReturn rule #7245
Conversation
🦋 Changeset detectedLatest commit: 382bd84 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 |
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds a new JavaScript lint rule UseConsistentArrowReturn that detects arrow functions whose block body consists of a single ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
pub UseConsistentArrowReturn { | ||
version: "next", | ||
name: "useConsistentArrowReturn", | ||
language: "js", | ||
recommended: false, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should add a rule source that points to the eslint rule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: revert changes to quick test
7d33d47
to
11ab8e6
Compare
11ab8e6
to
4a8194f
Compare
CodSpeed Performance ReportMerging #7245 will not alter performanceComparing Summary
Footnotes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Could you add a changeset too?
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js.snap
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
Outdated
Show resolved
Hide resolved
A few more suggestions for improvement:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js (2)
1-3
: Broaden invalid coverage to exercise more return-expression shapes (arrays, calls, sequences, async).This single case is good. Add a few more invalid samples so the rule is tested against common expressions and tricky parenthesisation (notably sequence expressions).
Apply this diff to extend the file:
const returnsObjectArrow = () => { return { a: 1, b: 2 }; } + +const returnsNumberArrow = () => { + return 42; +}; + +const returnsArrayArrow = () => { + return [1, 2]; +}; + +const returnsCallArrow = (x) => { + return x(); +}; + +// Ensures we handle required parentheses in fixes for sequence expressions +const returnsSequenceArrow = () => { + return (a(), b()); +}; + +// Async case: still a single return; fix should keep `await` in the concise body +const returnsAwaitArrow = async () => { + return await foo(); +};
3-3
: Tiny nit: terminate the const declaration with a semicolon.ASI will bail you out, but our default style prefers explicit semicolons.
-} +};
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js.snap
is excluded by!**/*.snap
and included by**
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js.snap
is excluded by!**/*.snap
and included by**
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
- crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
**/tests/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js (3)
40-44
: Add comment variants inside the body to harden the “contains comments” checkTrailing and block comments can sneak in via trivia; adding a few variants reduces false positives.
Apply this diff to extend coverage near this block:
const withComment = () => { // intentional comment return 1; }; +// Inline/trailing and block comment variants +const withInlineComment = () => { return 1; /* inline */ }; +const withBlockCommentBefore = () => { /* leading */ return 1; }; +const withCommentBetweenReturnAndExpr = () => { return /* comment */ 1; };
46-49
: Consider empty return + commentEmpty return is already covered; adding a commented variant ensures the “contains comments” bypass is consistent here too.
Apply this diff after the emptyReturn sample:
const emptyReturn = () => { return; }; +const emptyReturnWithComment = () => { + return; // explicitly empty +};
50-53
: Exercise directive edge-cases (only-directive body and pseudo-directive)
- Only-directive body should never be flagged.
- A “pseudo-directive” after a non-directive statement is not a directive prologue; include it to avoid over-skipping.
- Include an async variant with an internal comment to ensure async functions respect the comment skip-path too.
Apply this diff to add the extra cases:
const withDirective = () => { "use strict"; return 1; }; +const onlyDirective = () => { + "use strict"; +}; + +const pseudoDirectiveAfterStatement = () => { + let x = 1; + "use strict"; // not a directive here + return x; +}; + +const asyncWithComment = async () => { + /* intentional comment */ + return 1; +};
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js.snap
is excluded by!**/*.snap
and included by**
📒 Files selected for processing (1)
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
**/tests/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
🔇 Additional comments (3)
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js (3)
2-5
: Solid baseline for no-diagnostic arrow expressionsThese concise-body cases are exactly what we want to remain unflagged.
12-20
: Good coverage of multi-statement arrow bodiesMultiple statements (side effects + return) should not trigger this rule; these samples exercise that correctly.
1-54
: Smoke-check passed: invalid suite exists for useConsistentArrowReturn
- Found invalid test at
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.jsNo further action needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (1)
45-46
: Good: ESLint source wired up.Linking to
eslint
’sarrow-body-style
viaRuleSource::Eslint(...).same()
addresses the earlier review ask. Nicely done.
🧹 Nitpick comments (4)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (4)
16-39
: Docs: Call out the “skip” conditions explicitly.Since the rule intentionally skips bodies with directives, comments, and empty
return
, consider adding those as “Valid” examples to the doc block. It helps users understand why some arrow bodies aren’t flagged.
78-92
: Tighter diagnostic span: highlight the body, not the whole arrow.Point the diagnostic at the
JsFunctionBody
range for a more precise underline in editors.Apply this diff:
- fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> { - let node = ctx.query(); + fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> { Some( RuleDiagnostic::new( rule_category!(), - node.range(), + state.range(), markup! { "This arrow function doesn't need a return statement." }, ) .note(markup! { "Consider changing the function body into the returned expression." }), ) }
94-120
: Autofix looks correct; consider a more idiomatic type check.Using
matches!
onAnyJsExpression
avoids poking at raw syntax kinds and reads clearer.Apply this diff:
- let new_body = if JsObjectExpression::can_cast(return_argument.syntax().kind()) - || JsSequenceExpression::can_cast(return_argument.syntax().kind()) - { - AnyJsExpression::from(make::parenthesized(return_argument)) - } else { - return_argument - }; + let needs_parens = matches!( + return_argument, + AnyJsExpression::JsObjectExpression(_) + | AnyJsExpression::JsSequenceExpression(_) + ); + let new_body: AnyJsExpression = if needs_parens { + make::parenthesized(return_argument).into() + } else { + return_argument + };Also, since an autofix is already implemented (and
fix_kind: Safe
is set), you might want to update the PR description that previously said the fix would land later.Would you like me to open a follow-up to add more fix snapshots?
1-14
: Minor: import hygiene (optional).If you adopt the
matches!
refactor above, you can drop the directJsObjectExpression
/JsSequenceExpression
imports.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f
/just format
).
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options in the `biome_rule_options` crate under `lib/`, with serde- and schemars-compatible derives and `#[serde(rename_all = "camelCase", deny_unknown_fields, default)]`
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (1)
crates/biome_js_factory/src/make.rs (1)
parenthesized
(110-116)
🔇 Additional comments (2)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (2)
58-76
: Signal logic looks solid.Casts only block bodies, skips directives or any descendant comments, and requires a single
return
with an argument. This matches the reviewer’s requested behaviour and avoids the “empty return” pitfall.
41-49
: Rule metadata is consistent.Non‑recommended, warning severity, safe fix kind — all sane defaults for a new nursery rule.
fn action(ctx: &RuleContext<Self>, body: &Self::State) -> Option<JsRuleAction> { | ||
let mut mutation = ctx.root().begin(); | ||
|
||
let return_statement = body.statements().iter().next()?; | ||
let return_statement = JsReturnStatement::cast(return_statement.into_syntax())?; | ||
let return_argument = return_statement.argument()?; | ||
|
||
let new_body = if JsObjectExpression::can_cast(return_argument.syntax().kind()) | ||
|| JsSequenceExpression::can_cast(return_argument.syntax().kind()) | ||
{ | ||
AnyJsExpression::from(make::parenthesized(return_argument)) | ||
} else { | ||
return_argument | ||
}; | ||
|
||
mutation.replace_node( | ||
AnyJsFunctionBody::from(body.clone()), | ||
AnyJsFunctionBody::AnyJsExpression(new_body), | ||
); | ||
|
||
Some(JsRuleAction::new( | ||
ctx.metadata().action_category(ctx.category(), ctx.group()), | ||
ctx.metadata().applicability(), | ||
markup! { "Remove the return statement" }.to_owned(), | ||
mutation, | ||
)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Add a couple of edge-case specs for the fix.
- Sequence expression:
const f = () => { return (a, b) }
→() => (a, b)
. - Await:
const f = async () => { return await x }
→async () => await x
. - Already-parenthesised object: ensure no double parens.
Here’s a quick script to check the current specs and add gaps to your list:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the rule's spec directory
spec_dir="$(fd useConsistentArrowReturn -t d crates | head -n1)"
echo "Spec dir: ${spec_dir}"
echo -e "\nCurrent invalid cases mentioning objects/sequences:"
rg -n --no-heading -C1 'return\s*{|\(.*,\s*.*\)' "${spec_dir}/invalid.js" || true
echo -e "\nCurrent valid cases for directives/comments/empty return:"
rg -n --no-heading -C1 '"use strict"|//|/\*|return\s*;|return\s*\n' "${spec_dir}/valid.js" || true
Length of output: 1177
Let's inspect the existing spec files to see what we’ve got:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
dir="crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn"
invalid="$dir/invalid.js"
valid="$dir/valid.js"
echo "=== Invalid cases ==="
nl -ba "$invalid"
echo
echo "=== Valid cases ==="
nl -ba "$valid"
Length of output: 505
Here’s a script to print out both spec files with line numbers so we can spot which edge cases are missing:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
dir="crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn"
invalid="$dir/invalid.js"
valid="$dir/valid.js"
echo "=== invalid.js ==="
cat -n "$invalid"
echo
echo "=== valid.js ==="
cat -n "$valid"
Length of output: 2038
Add missing edge‐case specs for sequence and async/await returns
We need to extend the useConsistentArrowReturn
tests to cover:
- Sequence expressions in single‐return arrow bodies
async
functions returning anawait
ed value
Please add the following to your spec files in
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn
:
• invalid.js
// existing object case…
+// sequence expression
+const returnsSequenceArrow = () => {
+ return (a, b);
+}
+
+// async-await
+const returnsAwaitArrow = async () => {
+ return await fetchData();
+}
• valid.js
// existing “no-fix” cases…
+// sequence expression fixed
+const returnsSequenceArrow = () => (a, b);
+
+// async-await fixed
+const returnsAwaitArrow = async () => await fetchData();
This ensures we won’t regress on parentheses for sequences or drop the await
.
📝 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.
fn action(ctx: &RuleContext<Self>, body: &Self::State) -> Option<JsRuleAction> { | |
let mut mutation = ctx.root().begin(); | |
let return_statement = body.statements().iter().next()?; | |
let return_statement = JsReturnStatement::cast(return_statement.into_syntax())?; | |
let return_argument = return_statement.argument()?; | |
let new_body = if JsObjectExpression::can_cast(return_argument.syntax().kind()) | |
|| JsSequenceExpression::can_cast(return_argument.syntax().kind()) | |
{ | |
AnyJsExpression::from(make::parenthesized(return_argument)) | |
} else { | |
return_argument | |
}; | |
mutation.replace_node( | |
AnyJsFunctionBody::from(body.clone()), | |
AnyJsFunctionBody::AnyJsExpression(new_body), | |
); | |
Some(JsRuleAction::new( | |
ctx.metadata().action_category(ctx.category(), ctx.group()), | |
ctx.metadata().applicability(), | |
markup! { "Remove the return statement" }.to_owned(), | |
mutation, | |
)) | |
} | |
// File: crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js | |
// existing object case… | |
// sequence expression | |
const returnsSequenceArrow = () => { | |
return (a, b); | |
} | |
// async-await | |
const returnsAwaitArrow = async () => { | |
return await fetchData(); | |
} |
fn action(ctx: &RuleContext<Self>, body: &Self::State) -> Option<JsRuleAction> { | |
let mut mutation = ctx.root().begin(); | |
let return_statement = body.statements().iter().next()?; | |
let return_statement = JsReturnStatement::cast(return_statement.into_syntax())?; | |
let return_argument = return_statement.argument()?; | |
let new_body = if JsObjectExpression::can_cast(return_argument.syntax().kind()) | |
|| JsSequenceExpression::can_cast(return_argument.syntax().kind()) | |
{ | |
AnyJsExpression::from(make::parenthesized(return_argument)) | |
} else { | |
return_argument | |
}; | |
mutation.replace_node( | |
AnyJsFunctionBody::from(body.clone()), | |
AnyJsFunctionBody::AnyJsExpression(new_body), | |
); | |
Some(JsRuleAction::new( | |
ctx.metadata().action_category(ctx.category(), ctx.group()), | |
ctx.metadata().applicability(), | |
markup! { "Remove the return statement" }.to_owned(), | |
mutation, | |
)) | |
} | |
// File: crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js | |
// existing “no-fix” cases… | |
// sequence expression fixed | |
const returnsSequenceArrow = () => (a, b); | |
// async-await fixed | |
const returnsAwaitArrow = async () => await fetchData(); |
🤖 Prompt for AI Agents
In crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
around lines 94 to 120 the tests do not cover two edge cases (sequence
expressions returned from single-expression arrow bodies and async functions
returning an awaited value), so add spec files under
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn: create
invalid.js containing a single-return arrow body with a sequence expression that
requires parentheses to avoid breaking semantics, and create valid.js containing
an async arrow function that returns an awaited value (ensure the await is
preserved) plus any other existing valid/invalid cases; run the test suite and
adjust expectations if the rule output needs to preserve parentheses or keep
await in transformed code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (1)
45-45
: ESLint rule source looks good; double‑check the API variantUsing
RuleSource::Eslint("arrow-body-style")
is correct. Please verify whether.same()
is intended/available onRuleSource
in this crate version; if not, drop it or replace with the supported variant that links to the ESLint docs.
🧹 Nitpick comments (4)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (4)
41-49
: Autofix is implemented; please update PR description and add a changesetYou’ve shipped an action and declared
fix_kind: FixKind::Safe
, but the PR text says “autofix will be added later.” Please update the description and include a changeset to reflect the autofix.Happy to draft the changeset text and tweak the PR body if you want.
16-40
: Document the intentional skip conditions and noteworthy casesThe docs are clear but omit two key behaviours you’ve implemented:
- Bodies with directives or any descendant comments are ignored.
- Empty
return;
is ignored.
Also worth calling out: parentheses are added for objects and sequence expressions in the autofix.Consider expanding the examples to include these so users aren’t surprised.
@@ /// Enforce consistent arrow function bodies. /// /// This rule enforces the use of arrow functions with no body block when the function body consists of a single return statement. +/// +/// This rule does not report when: +/// - the function body contains directives (e.g. `"use strict"`), or +/// - the body (or its descendants) contain comments, or +/// - the single `return` has no argument (`return;`). +/// +/// The fix wraps expressions in parentheses when required for correctness (e.g. object literals and sequence expressions). @@ /// ### Valid /// /// ```js /// const foo = () => 0; +/// const bar = () => { "use strict"; return 1 } +/// const baz = () => { /* intentional */ return x } +/// const qux = () => ({ a: 1 }) // already concise with parens /// ```
94-120
: Message polish and applicabilityMinor nit: “Remove the return statement” could be clearer as “Use a concise arrow body” which is what the fix does. Also, given the broadened paren handling above,
FixKind::Safe
remains appropriate.- markup! { "Remove the return statement" }.to_owned(), + markup! { "Use a concise arrow body" }.to_owned(),
41-49
: Severity/recommendation combo is sensibleNon‑recommended + Warning is a good starting stance for a stylistic rule with autofix. Consider promoting after bake‑time if the signal/noise ratio looks healthy.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
crates/biome_configuration/src/analyzer/linter/rules.rs
is excluded by!**/rules.rs
and included by**
packages/@biomejs/backend-jsonrpc/src/workspace.ts
is excluded by!**/backend-jsonrpc/src/workspace.ts
and included by**
packages/@biomejs/biome/configuration_schema.json
is excluded by!**/configuration_schema.json
and included by**
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f
/just format
).
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options in the `biome_rule_options` crate under `lib/`, with serde- and schemars-compatible derives and `#[serde(rename_all = "camelCase", deny_unknown_fields, default)]`
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : In `declare_lint_rule!` declarations, set `version: "next"`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
🔇 Additional comments (4)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (4)
58-65
: Skipping bodies with directives or comments: LGTMMatches the review ask and avoids clobbering intentional formatting or ASI‑sensitive layouts.
66-73
: Single statement + non‑empty return detection: LGTMThis aligns with ESLint’s “as-needed” spirit and avoids flagging empty returns.
1-14
: Imports: anticipate TS wrappers if you adopt the paren helperIf you implement the helper, remember to import the TS expression nodes (e.g.
TsAsExpression
,TsSatisfiesExpression
,TsTypeAssertionExpression
) andJsParenthesizedExpression
. Keep imports grouped with the existingbiome_js_syntax
block.
78-92
: Diagnostic copy reads wellConcise and actionable. No changes needed.
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js (1)
1-3
: Add matching valid test for concise object literal (paren requirement).Great invalid case. Please also add the concise-body counterpart to valid.js to ensure we don’t regress and to exercise the mandatory parentheses for object literals once an autofix lands.
Proposed addition to valid.js:
+// valid: concise object literal +const returnsObjectArrowConcise = () => ({ a: 1, b: 2 })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
- crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options in the `biome_rule_options` crate under `lib/`, with serde- and schemars-compatible derives and `#[serde(rename_all = "camelCase", deny_unknown_fields, default)]`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Use `domains` in `declare_lint_rule!` when applicable; recommended rules with domains enable only when the domain is active
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
🧬 Code graph analysis (1)
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js (1)
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js (4)
returnsSequenceArrow
(63-63)a
(2-2)b
(3-3)returnsAwaitArrow
(64-64)
🔇 Additional comments (2)
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js (2)
5-7
: Sequence expression case is spot on.This is a good invalid sample; concise form requires parentheses and you’ve covered that in valid.js.
10-12
: No overlapping “return await” rule detected
I ran a search acrosscrates/biome_js_analyze
(excluding tests) for any rule matchingno-return-await
or that warns onreturn await
. The only hit was a documentation example insrc/lint/complexity/use_arrow_function.rs
—not an active lint. There’s no separate “no-return-await” rule, so your snapshots won’t pick up extra diagnostics here.
Done |
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (1)
54-55
: Rule source wired to ESLint’s arrow-body-style — niceCorrect source mapping with
.same()
.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f
/just format
).
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options in the `biome_rule_options` crate under `lib/`, with serde- and schemars-compatible derives and `#[serde(rename_all = "camelCase", deny_unknown_fields, default)]`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Use `domains` in `declare_lint_rule!` when applicable; recommended rules with domains enable only when the domain is active
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (2)
crates/biome_analyze/src/rule.rs (1)
same
(233-238)crates/biome_js_factory/src/make.rs (1)
parenthesized
(110-116)
🔇 Additional comments (2)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (2)
57-58
: PR text says “no autofix”, code ships an autofixThe rule declares a safe fix and implements
action()
. Either keep the fix (preferred) and update PR description/changeset, or dropfix_kind
andaction()
for now to align.If you decide to drop the fix for this PR, apply:
- fix_kind: FixKind::Safe, + // fix_kind: FixKind::Safe,and temporarily gate the action:
- fn action(ctx: &RuleContext<Self>, body: &Self::State) -> Option<JsRuleAction> { + /*fn action(ctx: &RuleContext<Self>, body: &Self::State) -> Option<JsRuleAction> { ... - } + }*/Otherwise, update the PR description to “includes safe autofix”.
Also applies to: 103-129
71-73
: Verified API:has_comments_descendants
is correct
Thehas_comments_descendants
method is defined onSyntaxNode
incrates/biome_rowan/src/syntax/node.rs
(around line 711) and iterates over all descendant tokens, checking for either leading or trailing comments—exactly matching the intended semantics. No replacement or swap is needed.
markup! { | ||
"This arrow function doesn't need a return statement." | ||
}, | ||
) | ||
.note(markup! { | ||
"Consider changing the function body into the returned expression." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure the diagnostic follows the rule pillars https://biomejs.dev/linter/#rule-pillars
This diagnostic doesn't explain why this is an error.
Also the message of the note is basically the message of action. We shouldn't repeat. Choose only one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure the diagnostic follows the rule pillars https://biomejs.dev/linter/#rule-pillars
This diagnostic doesn't explain why this is an error.
Also the message of the note is basically the message of action. We shouldn't repeat. Choose only one.
Done
.changeset/early-books-jam.md
Outdated
Added a new lint `useConsistentArrowReturn` rule. | ||
This rule is a port of ESLint's [arrow-body-style](https://eslint.org/docs/latest/rules/arrow-body-style) rule |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can keep the link at the bottom, but we should still:
- explain the rule (a summary)
- have a code snippet that triggers the rule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can keep the link at the bottom, but we should still:
- explain the rule (a summary)
- have a code snippet that triggers the rule
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one is pretty much ready, just left some nits.
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: Arend van Beelen jr. <[email protected]>
Co-authored-by: Arend van Beelen jr. <[email protected]>
Co-authored-by: Arend van Beelen jr. <[email protected]>
2f670bb
to
db6d6b7
Compare
I did a rebase and a force push to fix conflicts with the main branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (1)
50-58
: Good call on rule metadata and source mapping.Rule declaration looks correct; pointing to ESLint’s arrow-body-style via
RuleSource::Eslint(...).same()
is exactly what we want.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (1)
87-96
: Point the diagnostic at the body, not the whole arrow.Sharper range improves UX and aligns with “pin the problem” guidance. Use the provided state to highlight only the block body.
- fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> { - let node = ctx.query(); + fn diagnostic(ctx: &RuleContext<Self>, body: &Self::State) -> Option<RuleDiagnostic> { Some(RuleDiagnostic::new( rule_category!(), - node.range(), + body.range(), markup! { "The body of this arrow function contains a single "<Emphasis>"return"</Emphasis> " statement." }, )) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (8)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs
is excluded by!**/migrate/eslint_any_rule_to_biome.rs
and included by**
crates/biome_configuration/src/analyzer/linter/rules.rs
is excluded by!**/rules.rs
and included by**
crates/biome_diagnostics_categories/src/categories.rs
is excluded by!**/categories.rs
and included by**
crates/biome_js_analyze/src/lint/nursery.rs
is excluded by!**/nursery.rs
and included by**
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js.snap
is excluded by!**/*.snap
and included by**
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js.snap
is excluded by!**/*.snap
and included by**
packages/@biomejs/backend-jsonrpc/src/workspace.ts
is excluded by!**/backend-jsonrpc/src/workspace.ts
and included by**
packages/@biomejs/biome/configuration_schema.json
is excluded by!**/configuration_schema.json
and included by**
📒 Files selected for processing (6)
.changeset/early-books-jam.md
(1 hunks)crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
(1 hunks)crates/biome_rule_options/src/lib.rs
(1 hunks)crates/biome_rule_options/src/use_consistent_arrow_return.rs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .changeset/early-books-jam.md
- crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js
- crates/biome_rule_options/src/lib.rs
- crates/biome_rule_options/src/use_consistent_arrow_return.rs
- crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/valid.js
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f
/just format
).
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Use `domains` in `declare_lint_rule!` when applicable; recommended rules with domains enable only when the domain is active
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Document rules with a one-line first paragraph, `## Examples` with `### Invalid` (first) and `### Valid`, and document options under `## Options` with properly marked code blocks
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options in the `biome_rule_options` crate under `lib/`, with serde- and schemars-compatible derives and `#[serde(rename_all = "camelCase", deny_unknown_fields, default)]`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : When porting from other linters, declare `sources: &[RuleSource::<...>]` in `declare_lint_rule!` using `.same()` or `.inspired()` as appropriate
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Avoid `unwrap`/`expect`; use `?`, `ok()?`, and combinators (`map`, `and_then`, `filter`) to handle `Result`/`Option` when navigating the CST
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
📚 Learning: 2025-08-11T11:46:05.836Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Diagnostics should explain why something went wrong, using log advices and links to relevant documentation
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (1)
crates/biome_js_factory/src/make.rs (1)
parenthesized
(110-116)
🔇 Additional comments (3)
crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs (3)
67-74
: Solid early exits for directives/comments.Skipping bodies with directives or any comment descendants matches the review guidance and avoids risky fixes. Nicely cautious.
101-114
: Mutation flow looks correct.Extract/replace is clean and uses
AnyJsFunctionBody::AnyJsExpression
properly. Assuming paren logic is refined (see below), this will produce stable diffs.
116-121
: Action wording is crisp.Message and applicability plumbing are spot on; no duplication with the diagnostic. Carry on.
let new_body = if needs_parens_in_concise_body(&return_argument) { | ||
AnyJsExpression::from(make::parenthesized(return_argument)) | ||
} else { | ||
return_argument | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Avoid redundant/double parentheses for TS wrappers.
needs_parens_in_concise_body
currently adds parens for any TsAs/Satisfies/TypeAssertion
regardless of the inner expression. This can yield churn like (() => ((x as T)))
or double-parens for ({a:1}) as T
. We can stay non-recursive and still be precise: only require parens when the wrapper’s inner is an object or sequence, and skip if that inner is already parenthesised.
-fn needs_parens_in_concise_body(expr: &AnyJsExpression) -> bool {
- use AnyJsExpression::*;
- matches!(
- expr,
- JsObjectExpression(_)
- | JsSequenceExpression(_)
- | TsAsExpression(_)
- | TsSatisfiesExpression(_)
- | TsTypeAssertionExpression(_)
- )
-}
+fn needs_parens_in_concise_body(expr: &AnyJsExpression) -> bool {
+ use AnyJsExpression::*;
+ match expr {
+ // These always require parens in a concise arrow body
+ JsObjectExpression(_) | JsSequenceExpression(_) => true,
+
+ // For TS wrappers, only require parens if the wrapped expression itself would need them,
+ // and it's not already parenthesised.
+ TsAsExpression(e) => e.expression().map_or(false, |inner| needs_parens_inner(&inner)),
+ TsSatisfiesExpression(e) => e.expression().map_or(false, |inner| needs_parens_inner(&inner)),
+ TsTypeAssertionExpression(e) => e.expression().map_or(false, |inner| needs_parens_inner(&inner)),
+
+ // Already parenthesised → no extra parens, please
+ JsParenthesizedExpression(_) => false,
+
+ _ => false,
+ }
+}
+
+fn needs_parens_inner(inner: AnyJsExpression) -> bool {
+ use AnyJsExpression::*;
+ match inner {
+ JsParenthesizedExpression(_) => false,
+ JsObjectExpression(_) | JsSequenceExpression(_) => true,
+ _ => false,
+ }
+}
Verification suggestion:
- Ensure test specs cover:
return {} as T
→() => ({} as T)
return ({a:1}) as T
→() => ({a:1}) as T
(no extra parens)return (a, b) as T
→() => (a, b) as T
Also applies to: 125-135
🏁 Script executed:
#!/bin/bash
# Grep for TS-wrapped object/sequence cases in specs; add if missing.
fd useConsistentArrowReturn crates/biome_js_analyze/tests/specs -t d | while read -r d; do
rg -n --no-heading -e 'return\s*\{\}\s*as\s' -e 'return\s*\(\s*\{.*\}\s*\)\s*as\s' -e 'return\s*\(\s*\w+\s*,\s*\w+\s*\)\s*as\s' "$d" || true
done
Length of output: 301
🏁 Script executed:
#!/bin/bash
# Find spec directories for both snake_case and camelCase
dirs=$(fd -t d -i use_consistent_arrow_return crates/biome_js_analyze/tests/specs || true; fd -t d -i useConsistentArrowReturn crates/biome_js_analyze/tests/specs || true)
if [ -z "$dirs" ]; then
echo "No spec directories found for use_consistent_arrow_return or useConsistentArrowReturn"
exit 0
fi
for dir in $dirs; do
echo "Inspecting $dir"
rg -n --no-heading \
-e 'return\s*\{\}\s*as\s' \
-e 'return\s*\(\s*\{.*\}\)\s*as\s' \
-e 'return\s*\(\s*\w+\s*,\s*\w+\s*\)\s*as\s' \
"$dir" || echo "No matches in $dir"
done
Length of output: 876
Add missing spec tests for TS wrapper parentheses
The following scenarios aren’t covered under crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/:
return {} as T
→ expect() => ({} as T)
return ({a:1}) as T
→ expect() => ({a:1}) as T
return (a, b) as T
→ expect() => (a, b) as T
Please add specs for each case.
🤖 Prompt for AI Agents
In crates/biome_js_analyze/src/lint/nursery/use_consistent_arrow_return.rs
around lines 105 to 109, the lexer/transform logic is fine but the test suite is
missing TypeScript wrapper-parentheses cases; add spec tests under
crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/ that
cover: 1) input with `return {} as T` expecting transform `() => ({} as T)`, 2)
input with `return ({a:1}) as T` expecting `() => ({a:1}) as T`, and 3) input
with `return (a, b) as T` expecting `() => (a, b) as T`; create one spec file
(or three small files) containing the source snippet and the expected output
snapshot for each case so the rule verifies parentheses are preserved around TS
`as` wrapper expressions.
Co-authored-by: Naoki Ikeguchi <[email protected]>
@kedevked As the last step, can you merge the |
…istentArrowReturn
Done |
@kedevked Just one more thing, can you update the outdated snapshots? |
Done |
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Arend van Beelen jr. <[email protected]> Co-authored-by: Naoki Ikeguchi <[email protected]>
Summary
It adds the useConsistentArrowReturn rule #5050
Test Plan
Test cases have been added
Docs
Doc has been added to the code