Skip to content

Conversation

kedevked
Copy link
Contributor

Summary

It adds the useConsistentArrowReturn rule #5050

Test Plan

Test cases have been added

Docs

Doc has been added to the code

Copy link

changeset-bot bot commented Aug 17, 2025

🦋 Changeset detected

Latest commit: 382bd84

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

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

@kedevked kedevked marked this pull request as draft August 17, 2025 10:29
Copy link
Contributor

coderabbitai bot commented Aug 17, 2025

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • crates/biome_js_analyze/tests/specs/nursery/useConsistentArrowReturn/invalid.js.snap is excluded by !**/*.snap and included by **

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Adds a new JavaScript lint rule UseConsistentArrowReturn that detects arrow functions whose block body consists of a single return of an expression (ignoring bodies with directives or comment descendants) and offers a safe auto-fix to convert to a concise body, adding parentheses when needed. Adds UseConsistentArrowReturnOptions, re-exports it, includes tests (valid/invalid), and an accompanying changeset.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Diagnostic Area: diagnostocis labels Aug 17, 2025
Comment on lines 39 to 58
pub UseConsistentArrowReturn {
version: "next",
name: "useConsistentArrowReturn",
language: "js",
recommended: false,
}
Copy link
Contributor

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

Copy link
Contributor

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

@kedevked kedevked changed the title feat(biome_js_analyze): UseConsistentArrowReturn feat(biome_js_analyze): add UseConsistentArrowReturn rule Aug 17, 2025
@kedevked kedevked force-pushed the feature/add-useConsistentArrowReturn branch 2 times, most recently from 7d33d47 to 11ab8e6 Compare August 17, 2025 23:02
@kedevked kedevked marked this pull request as ready for review August 17, 2025 23:05
@kedevked kedevked force-pushed the feature/add-useConsistentArrowReturn branch from 11ab8e6 to 4a8194f Compare August 17, 2025 23:11
@github-actions github-actions bot added the A-CLI Area: CLI label Aug 18, 2025
Copy link

codspeed-hq bot commented Aug 18, 2025

CodSpeed Performance Report

Merging #7245 will not alter performance

Comparing kedevked:feature/add-useConsistentArrowReturn (382bd84) with main (91a0fc7)1

Summary

✅ 133 untouched benchmarks

Footnotes

  1. No successful run was found on main (b1f8cbd) during the generation of this report, so 91a0fc7 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Copy link
Contributor

@arendjr arendjr left a 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?

@arendjr
Copy link
Contributor

arendjr commented Aug 18, 2025

A few more suggestions for improvement:

  • The rule should check if the function has directives, and not flag any functions that do.
  • The rule should not flag functions that have comments in their body, because in those cases the syntax is likely to be more intentional.
  • We may want to consider what we want to do with functions that contain nothing but an empty return statement that doesn't return anything. I believe ESLint simply doesn't flag those.
  • An auto-fix action can be added still. When we do so, make sure to have handle parentheses around expressions that need them (object literals and sequence expressions).

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between aba3d61 and d5bf991.

⛔ 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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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” check

Trailing 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 + comment

Empty 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.

📥 Commits

Reviewing files that changed from the base of the PR and between d5bf991 and 5b3a6a1.

⛔ 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 expressions

These concise-body cases are exactly what we want to remain unflagged.


12-20: Good coverage of multi-statement arrow bodies

Multiple 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.js

No further action needed.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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’s arrow-body-style via RuleSource::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! on AnyJsExpression 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 direct JsObjectExpression/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.

📥 Commits

Reviewing files that changed from the base of the PR and between a1b67e1 and 86b0a32.

📒 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.

Comment on lines 94 to 122
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,
))
}
Copy link
Contributor

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 an awaited 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.

Suggested change
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();
}
Suggested change
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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 variant

Using RuleSource::Eslint("arrow-body-style") is correct. Please verify whether .same() is intended/available on RuleSource 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 changeset

You’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 cases

The 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 applicability

Minor 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 sensible

Non‑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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5f5f64b and 5db6236.

⛔ 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: LGTM

Matches the review ask and avoids clobbering intentional formatting or ASI‑sensitive layouts.


66-73: Single statement + non‑empty return detection: LGTM

This aligns with ESLint’s “as-needed” spirit and avoids flagging empty returns.


1-14: Imports: anticipate TS wrappers if you adopt the paren helper

If you implement the helper, remember to import the TS expression nodes (e.g. TsAsExpression, TsSatisfiesExpression, TsTypeAssertionExpression) and JsParenthesizedExpression. Keep imports grouped with the existing biome_js_syntax block.


78-92: Diagnostic copy reads well

Concise and actionable. No changes needed.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5db6236 and e9c3801.

📒 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 across crates/biome_js_analyze (excluding tests) for any rule matching no-return-await or that warns on return await. The only hit was a documentation example in src/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.

@kedevked
Copy link
Contributor Author

Thanks!

Could you add a changeset too?

Done

@kedevked
Copy link
Contributor Author

A few more suggestions for improvement:

  • The rule should check if the function has directives, and not flag any functions that do.
  • The rule should not flag functions that have comments in their body, because in those cases the syntax is likely to be more intentional.
  • We may want to consider what we want to do with functions that contain nothing but an empty return statement that doesn't return anything. I believe ESLint simply doesn't flag those.
  • An auto-fix action can be added still. When we do so, make sure to have handle parentheses around expressions that need them (object literals and sequence expressions).

Done

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 — nice

Correct 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.

📥 Commits

Reviewing files that changed from the base of the PR and between e9c3801 and bddc8ed.

📒 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 autofix

The rule declares a safe fix and implements action(). Either keep the fix (preferred) and update PR description/changeset, or drop fix_kind and action() 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
The has_comments_descendants method is defined on SyntaxNode in crates/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.

Comment on lines 93 to 98
markup! {
"This arrow function doesn't need a return statement."
},
)
.note(markup! {
"Consider changing the function body into the returned expression."
Copy link
Member

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.

Copy link
Contributor Author

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

Comment on lines 5 to 6
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
Copy link
Member

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

Copy link
Contributor Author

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

Copy link
Contributor

@arendjr arendjr left a 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.

@kedevked kedevked force-pushed the feature/add-useConsistentArrowReturn branch from 2f670bb to db6d6b7 Compare September 3, 2025 13:32
@kedevked
Copy link
Contributor Author

kedevked commented Sep 3, 2025

I did a rebase and a force push to fix conflicts with the main branch

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 2f670bb and db6d6b7.

⛔ 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.

Comment on lines +105 to +109
let new_body = if needs_parens_in_concise_body(&return_argument) {
AnyJsExpression::from(make::parenthesized(return_argument))
} else {
return_argument
};
Copy link
Contributor

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.

@siketyan
Copy link
Member

siketyan commented Sep 4, 2025

@kedevked As the last step, can you merge the main branch or rebase this branch to main? You can ignore generated files when resolving conflicts, and re-run the codegen.

@kedevked
Copy link
Contributor Author

kedevked commented Sep 4, 2025

@kedevked As the last step, can you merge the main branch or rebase this branch to main? You can ignore generated files when resolving conflicts, and re-run the codegen.

Done

@siketyan
Copy link
Member

siketyan commented Sep 4, 2025

@kedevked Just one more thing, can you update the outdated snapshots?

@kedevked
Copy link
Contributor Author

kedevked commented Sep 4, 2025

@kedevked Just one more thing, can you update the outdated snapshots?

Done

@ematipico ematipico merged commit fad34b9 into biomejs:main Sep 4, 2025
30 checks passed
@github-actions github-actions bot mentioned this pull request Sep 4, 2025
dyc3 pushed a commit that referenced this pull request Oct 3, 2025
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CLI Area: CLI A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants