Skip to content
Merged
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
31 changes: 21 additions & 10 deletions crates/oxc_linter/src/rules/eslint/array_callback_return/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@ fn expect_return(method_name: &str, span: Span) -> OxcDiagnostic {
.with_label(span)
}

fn expect_no_return(method_name: &str, span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("Unexpected return for array method {method_name:?}"))
fn expect_no_return(method_name: &str, call_span: Span, return_span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("Unexpected return value in callback for {method_name:?}"))
.with_help(format!(
"Array method {method_name:?} expects no useless return from the function"
"{method_name:?} ignores the callback's return value. Remove the returned value (use `return;` or no `return`), or use `map`/`flatMap` if you meant to produce a new array."
))
.with_label(span)
.with_labels([
call_span.label(format!("{method_name:?} is called here.")),
return_span.label("This returned value is ignored."),
])
}

#[derive(Debug, Default, Clone, JsonSchema, Deserialize)]
Expand Down Expand Up @@ -103,7 +106,7 @@ impl Rule for ArrayCallbackReturn {
};

// Filter on target methods on Arrays
if let Some(array_method) = get_array_method_name(node, ctx) {
if let Some((array_method_span, array_method)) = get_array_method_info(node, ctx) {
let return_status = if always_explicit_return {
StatementReturnStatus::AlwaysExplicit
} else {
Expand All @@ -114,9 +117,14 @@ impl Rule for ArrayCallbackReturn {
("forEach", false, _) => (),
("forEach", true, _) => {
if return_status.may_return_explicit() {
let return_span = return_checker::get_explicit_return_spans(function_body)
.into_iter()
.next()
.unwrap_or_else(|| function_body.span());
ctx.diagnostic(expect_no_return(
&full_array_method_name(array_method),
function_body.span,
array_method_span,
return_span,
));
}
}
Expand Down Expand Up @@ -144,7 +152,10 @@ impl Rule for ArrayCallbackReturn {
/// Code ported from [eslint](https://github.com/eslint/eslint/blob/v9.9.1/lib/rules/array-callback-return.js)
/// We're currently on a `Function` or `ArrowFunctionExpression`, findout if it is an argument
/// to the target array methods we're interested in.
pub fn get_array_method_name<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> Option<&'a str> {
pub fn get_array_method_info<'a>(
node: &AstNode<'a>,
ctx: &LintContext<'a>,
) -> Option<(Span, &'a str)> {
let mut current_node = node;
loop {
let parent = ctx.nodes().parent_node(current_node.id());
Expand Down Expand Up @@ -197,12 +208,12 @@ pub fn get_array_method_name<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> O
&& let Some(call_arg) = call.arguments[1].as_expression()
&& call_arg.span() == current_node.kind().span()
{
return Some("from");
return Some((callee.span(), "from"));
}
}

// "methods",
let array_method = callee.static_property_name()?;
let (array_method_span, array_method) = callee.static_property_info()?;

if TARGET_METHODS.contains(&array_method)
// Check that current node is parent's first argument
Expand All @@ -212,7 +223,7 @@ pub fn get_array_method_name<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> O
.as_expression()
.is_some_and(|arg| arg.span() == current_node.kind().span())
{
return Some(array_method);
return Some((array_method_span, array_method));
}

return None;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
use oxc_ast::ast::{BlockStatement, FunctionBody, Statement, SwitchCase};
use oxc_ast::ast::{
ArrowFunctionExpression, BlockStatement, Function, FunctionBody, ReturnStatement, Statement,
SwitchCase,
};
use oxc_ast_visit::Visit;
use oxc_ecmascript::{ToBoolean, WithoutGlobalReferenceInformation};
use oxc_semantic::ScopeFlags;
use oxc_span::{GetSpan, Span};

/// `StatementReturnStatus` describes whether the CFG corresponding to
/// the statement is termitated by return statement in all/some/nome of
Expand Down Expand Up @@ -116,6 +122,33 @@ pub fn check_function_body(function: &FunctionBody) -> StatementReturnStatus {
status
}

/// Collect spans of **explicit** return values (`return <expr>`) in the given function body.
///
/// This is used by `array-callback-return` when `checkForEach` is enabled to highlight the
/// returned value(s) which are ignored by `forEach`.
pub fn get_explicit_return_spans(function: &FunctionBody) -> Vec<Span> {
let mut finder = ReturnStatementFinder::default();
finder.visit_function_body(function);
finder.spans
}

#[derive(Default)]
struct ReturnStatementFinder {
spans: Vec<Span>,
}

impl Visit<'_> for ReturnStatementFinder {
fn visit_return_statement(&mut self, return_statement: &ReturnStatement) {
if let Some(argument) = &return_statement.argument {
self.spans.push(argument.span());
}
}

fn visit_function(&mut self, _func: &Function<'_>, _flags: ScopeFlags) {}

fn visit_arrow_function_expression(&mut self, _it: &ArrowFunctionExpression<'_>) {}
}

/// Return checkers runs a Control Flow-like Analysis on a statement to see if it
/// always returns on all paths of execution.
pub fn check_statement(statement: &Statement) -> StatementReturnStatus {
Expand Down
Loading
Loading