Skip to content

Commit

Permalink
fix(linter): false positive in no-return-assign (#6128)
Browse files Browse the repository at this point in the history
  • Loading branch information
@DonIsaac
DonIsaac committed Sep 27, 2024
1 parent ae539af commit e7e8ead
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 75 deletions.
63 changes: 35 additions & 28 deletions crates/oxc_linter/src/rules/eslint/no_return_assign.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ use serde_json::Value;

use crate::{context::LintContext, rule::Rule, AstNode};

fn no_return_assign_diagnostic(span: Span, message: &str) -> OxcDiagnostic {
OxcDiagnostic::warn(message.to_string()).with_label(span)
fn no_return_assign_diagnostic(span: Span, message: &'static str) -> OxcDiagnostic {
OxcDiagnostic::warn(message)
.with_label(span)
.with_help("Did you mean to use `==` instead of `=`?")
}

#[derive(Debug, Default, Clone)]
Expand Down Expand Up @@ -39,7 +41,7 @@ declare_oxc_lint!(
/// ```
NoReturnAssign,
style,
suggestion
pending // TODO: add a suggestion
);

fn is_sentinel_node(ast_kind: AstKind) -> bool {
Expand All @@ -60,35 +62,39 @@ impl Rule for NoReturnAssign {
}

fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::AssignmentExpression(_) = node.kind() {
if !self.always_disallow_assignment_in_return
&& ctx
.nodes()
.parent_node(node.id())
.is_some_and(|node| node.kind().as_parenthesized_expression().is_some())
{
return;
}
let mut parent_node = ctx.nodes().parent_node(node.id());
while parent_node.is_some_and(|parent| !is_sentinel_node(parent.kind())) {
parent_node = ctx.nodes().parent_node(parent_node.unwrap().id());
}
if let Some(parent) = parent_node {
match parent.kind() {
AstKind::ReturnStatement(_) => {
ctx.diagnostic(no_return_assign_diagnostic(
parent.span(),
"Return statement should not contain an assignment.",
));
}
AstKind::ArrowFunctionExpression(_) => {
let AstKind::AssignmentExpression(assign) = node.kind() else {
return;
};
if !self.always_disallow_assignment_in_return
&& ctx
.nodes()
.parent_node(node.id())
.is_some_and(|node| node.kind().as_parenthesized_expression().is_some())
{
return;
}

let mut parent_node = ctx.nodes().parent_node(node.id());
while parent_node.is_some_and(|parent| !is_sentinel_node(parent.kind())) {
parent_node = ctx.nodes().parent_node(parent_node.unwrap().id());
}
if let Some(parent) = parent_node {
match parent.kind() {
AstKind::ReturnStatement(_) => {
ctx.diagnostic(no_return_assign_diagnostic(
assign.span(),
"Return statements should not contain an assignment.",
));
}
AstKind::ArrowFunctionExpression(arrow) => {
if arrow.expression {
ctx.diagnostic(no_return_assign_diagnostic(
parent.span(),
"Arrow function should not return an assignment.",
assign.span(),
"Arrow functions should not return an assignment.",
));
}
_ => (),
}
_ => (),
}
}
}
Expand Down Expand Up @@ -116,6 +122,7 @@ fn test() {
"function x() { return function y() { result = a * b }; }",
Some(serde_json::json!(["always"])),
),
("() => { a = b; }", None),
("() => { return (result = a * b); }", Some(serde_json::json!(["except-parens"]))),
("() => (result = a * b)", Some(serde_json::json!(["except-parens"]))),
("const foo = (a,b,c) => ((a = b), c)", None),
Expand Down
110 changes: 63 additions & 47 deletions crates/oxc_linter/src/snapshots/no_return_assign.snap
View file
Original file line number Diff line number Diff line change
@@ -1,111 +1,127 @@
---
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:1:16]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:1:23]
1 │ function x() { return result = a * b; };
· ──────────────────────
· ──────────────
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:1:16]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:1:23]
1 │ function x() { return (result) = (a * b); };
· ──────────────────────────
· ──────────────────
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:1:16]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:1:23]
1 │ function x() { return result = a * b; };
· ──────────────────────
· ──────────────
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:1:16]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:1:23]
1 │ function x() { return (result) = (a * b); };
· ──────────────────────────
· ──────────────────
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:1:9]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:1:16]
1 │ () => { return result = a * b; }
· ──────────────────────
· ──────────────
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Arrow function should not return an assignment.
╭─[no_return_assign.tsx:1:1]
⚠ eslint(no-return-assign): Arrow functions should not return an assignment.
╭─[no_return_assign.tsx:1:7]
1 │ () => result = a * b
· ────────────────────
· ──────────────
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:1:16]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:1:23]
1 │ function x() { return result = a * b; };
· ──────────────────────
· ──────────────
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:1:16]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:1:24]
1 │ function x() { return (result = a * b); };
· ────────────────────────
· ──────────────
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:1:16]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:1:34]
1 │ function x() { return result || (result = a * b); };
· ──────────────────────────────────
· ──────────────
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:2:20]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:2:27]
1 │ function foo(){
2 │ return a = b
· ────────────
· ─────
3 │ }
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:2:20]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:2:27]
1 │ function doSomething() {
2 │ return foo = bar && foo > 0;
· ────────────────────────────
· ────────────────────
3 │ }
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:2:20]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:2:27]
1 │ function doSomething() {
2 │ ╭─▶ return foo = function(){
3 │ │ return (bar = bar1)
4 │ ╰─▶ }
5 │ }
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:2:20]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:2:27]
1 │ function doSomething() {
2 │ return foo = () => a
· ────────────────────
· ─────────────
3 │ }
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Arrow function should not return an assignment.
╭─[no_return_assign.tsx:2:27]
⚠ eslint(no-return-assign): Arrow functions should not return an assignment.
╭─[no_return_assign.tsx:2:33]
1 │ function doSomething() {
2 │ return () => a = () => b
· ─────────────────
· ───────────
3 │ }
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Return statement should not contain an assignment.
╭─[no_return_assign.tsx:3:24]
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
╭─[no_return_assign.tsx:3:31]
2 │ return function bar(b){
3 │ return a = b
· ────────────
· ─────
4 │ }
╰────
help: Did you mean to use `==` instead of `=`?

⚠ eslint(no-return-assign): Arrow function should not return an assignment.
╭─[no_return_assign.tsx:1:20]
⚠ eslint(no-return-assign): Arrow functions should not return an assignment.
╭─[no_return_assign.tsx:1:27]
1 │ const foo = (a) => (b) => a = b
· ────────────
· ─────
╰────
help: Did you mean to use `==` instead of `=`?

0 comments on commit e7e8ead

Please sign in to comment.