Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/check-for-each-option.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@biomejs/biome": minor
---

Added `checkForEach` option to the [`useIterableCallbackReturn`](https://biomejs.dev/linter/rules/use-iterable-callback-return/) rule. When set to `true`, the rule will also check that `forEach` callbacks do not return a value. This matches the behavior of ESLint's `array-callback-return` rule with the same option. The default is `false`, which means `forEach` callbacks are not checked by default.
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,6 @@ declare_lint_rule! {
/// });
/// ```
///
/// ```js,expect_diagnostic
/// [].forEach(() => {
/// return 1; // Should not return a value
/// });
/// ```
///
/// ### Valid
///
/// ```js
Expand All @@ -80,6 +74,31 @@ declare_lint_rule! {
/// ```js
/// [].forEach(() => void null); // Void return value, which doesn't trigger the rule
/// ```
///
/// ## Options
///
/// ### checkForEach
///
/// By default, this rule does not check `forEach` callbacks. Set `checkForEach` to `true`
/// to enforce that `forEach` callbacks do not return a value.
///
/// Default: `false`
///
/// ```json,options
/// {
/// "options": {
/// "checkForEach": true
/// }
/// }
/// ```
Comment on lines +85 to +93
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grep for json,options to see how we usually document options

///
/// When `checkForEach` is `true`, the following code is invalid:
///
/// ```js,expect_diagnostic,use_options
/// [].forEach(() => {
/// return 1; // Should not return a value
/// });
/// ```
pub UseIterableCallbackReturn {
version: "2.0.0",
name: "useIterableCallbackReturn",
Expand Down Expand Up @@ -130,6 +149,11 @@ impl Rule for UseIterableCallbackReturn {

let method_config = ITERABLE_METHOD_INFOS.get(member_name.text_trimmed())?;

// Skip forEach check if checkForEach option is false (default)
if method_config.method_name == "forEach" && !ctx.options().check_for_each() {
return None;
}

let arg_position = argument_list
.elements()
.position(|arg| arg.node.is_ok() && arg.node.unwrap().syntax().eq(&cfg.node))?;
Expand All @@ -149,15 +173,18 @@ impl Rule for UseIterableCallbackReturn {

let mut problems: Vec<RuleProblemKind> = Vec::new();
let member_range = member_expression.member().ok()?.range();
// Consider implicit return (arrow expression body) as a return with value
let has_return_with_value =
!returns_info.returns_with_value.is_empty() || returns_info.implicit_return.is_some();
if method_config.return_value_required {
if returns_info.has_paths_without_returns {
if returns_info.returns_with_value.is_empty() {
if !has_return_with_value {
problems.push(RuleProblemKind::MissingReturnWithValue);
} else {
problems.push(RuleProblemKind::NotAllPathsReturnValue);
}
} else if !returns_info.returns_without_value.is_empty() {
if !returns_info.returns_with_value.is_empty() {
if has_return_with_value {
for return_range in returns_info.returns_without_value {
problems.push(RuleProblemKind::UnexpectedEmptyReturn(return_range));
}
Expand All @@ -166,6 +193,10 @@ impl Rule for UseIterableCallbackReturn {
}
}
} else {
// Handle implicit return (arrow expression body) separately
if let Some(implicit_range) = returns_info.implicit_return {
problems.push(RuleProblemKind::UnexpectedImplicitReturn(implicit_range));
}
for return_range in returns_info.returns_with_value {
problems.push(RuleProblemKind::UnexpectedReturnWithValue(return_range));
}
Expand Down Expand Up @@ -225,6 +256,14 @@ impl Rule for UseIterableCallbackReturn {
},
);
}
RuleProblemKind::UnexpectedImplicitReturn(return_range) => {
diagnostic = diagnostic.detail(
*return_range,
markup! {
"Use a block body with no return, or wrap the expression with "<Emphasis>"void"</Emphasis>"."
},
);
}
}
}

Expand Down Expand Up @@ -252,6 +291,8 @@ enum RuleProblemKind {
UnexpectedEmptyReturn(TextRange),
/// An unexpected `return` statement with value.
UnexpectedReturnWithValue(TextRange),
/// An unexpected implicit return (arrow expression body).
UnexpectedImplicitReturn(TextRange),
}

/// This struct holds information about the return statements in a function.
Expand All @@ -264,6 +305,8 @@ struct FunctionReturnsInfo {
returns_with_value: Vec<TextRange>,
/// The ranges of return keywords that do not return a value.
returns_without_value: Vec<TextRange>,
/// The range of an implicit return (arrow expression body), if any.
implicit_return: Option<TextRange>,
}

/// This function analyzes the control flow graph of a function and collects information about
Expand All @@ -274,6 +317,7 @@ fn get_function_returns_info(cfg: &JsControlFlowGraph) -> FunctionReturnsInfo {
has_paths_without_returns: false,
returns_with_value: Vec::new(),
returns_without_value: Vec::new(),
implicit_return: None,
};

if let Some(arrow_expression) = JsArrowFunctionExpression::cast_ref(&cfg.node)
Expand All @@ -289,9 +333,8 @@ fn get_function_returns_info(cfg: &JsControlFlowGraph) -> FunctionReturnsInfo {
.returns_without_value
.push(expression.range())
} else {
function_returns_info
.returns_with_value
.push(expression.range())
// Track implicit return separately from explicit return statements
function_returns_info.implicit_return = Some(expression.range());
}

return function_returns_info;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,3 @@
[].forEach((a) => {
return a.fn();
});
[].forEach(function(a) {
return a.fn();
});
[].forEach((a) => {
if (a) {
return a.fn();
}
});
[].forEach((a) => {
if (a) {
return;
}
return a.fn();
});
[].forEach((a) => {
if (a) {
return;
}
return a.fn();
});
[].forEach((a) => {
if (a) {
throw new Error();
}
return a.fn();
});
Array.from([], () => {});
Array.from([], function() {});
Array.from([], () => {
Expand Down
Loading