Skip to content

Commit e7e8ead

Browse files
DonIsaacDonIsaac
committedSep 27, 2024·
fix(linter): false positive in no-return-assign (#6128)
·
oxlint_v0.16.7crates_v0.30.4
1 parent ae539af commit e7e8ead

File tree

2 files changed

+98
-75
lines changed

2 files changed

+98
-75
lines changed
 

‎crates/oxc_linter/src/rules/eslint/no_return_assign.rs

+35-28
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ use serde_json::Value;
66

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

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

1315
#[derive(Debug, Default, Clone)]
@@ -39,7 +41,7 @@ declare_oxc_lint!(
3941
/// ```
4042
NoReturnAssign,
4143
style,
42-
suggestion
44+
pending // TODO: add a suggestion
4345
);
4446

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

6264
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
63-
if let AstKind::AssignmentExpression(_) = node.kind() {
64-
if !self.always_disallow_assignment_in_return
65-
&& ctx
66-
.nodes()
67-
.parent_node(node.id())
68-
.is_some_and(|node| node.kind().as_parenthesized_expression().is_some())
69-
{
70-
return;
71-
}
72-
let mut parent_node = ctx.nodes().parent_node(node.id());
73-
while parent_node.is_some_and(|parent| !is_sentinel_node(parent.kind())) {
74-
parent_node = ctx.nodes().parent_node(parent_node.unwrap().id());
75-
}
76-
if let Some(parent) = parent_node {
77-
match parent.kind() {
78-
AstKind::ReturnStatement(_) => {
79-
ctx.diagnostic(no_return_assign_diagnostic(
80-
parent.span(),
81-
"Return statement should not contain an assignment.",
82-
));
83-
}
84-
AstKind::ArrowFunctionExpression(_) => {
65+
let AstKind::AssignmentExpression(assign) = node.kind() else {
66+
return;
67+
};
68+
if !self.always_disallow_assignment_in_return
69+
&& ctx
70+
.nodes()
71+
.parent_node(node.id())
72+
.is_some_and(|node| node.kind().as_parenthesized_expression().is_some())
73+
{
74+
return;
75+
}
76+
77+
let mut parent_node = ctx.nodes().parent_node(node.id());
78+
while parent_node.is_some_and(|parent| !is_sentinel_node(parent.kind())) {
79+
parent_node = ctx.nodes().parent_node(parent_node.unwrap().id());
80+
}
81+
if let Some(parent) = parent_node {
82+
match parent.kind() {
83+
AstKind::ReturnStatement(_) => {
84+
ctx.diagnostic(no_return_assign_diagnostic(
85+
assign.span(),
86+
"Return statements should not contain an assignment.",
87+
));
88+
}
89+
AstKind::ArrowFunctionExpression(arrow) => {
90+
if arrow.expression {
8591
ctx.diagnostic(no_return_assign_diagnostic(
86-
parent.span(),
87-
"Arrow function should not return an assignment.",
92+
assign.span(),
93+
"Arrow functions should not return an assignment.",
8894
));
8995
}
90-
_ => (),
9196
}
97+
_ => (),
9298
}
9399
}
94100
}
@@ -116,6 +122,7 @@ fn test() {
116122
"function x() { return function y() { result = a * b }; }",
117123
Some(serde_json::json!(["always"])),
118124
),
125+
("() => { a = b; }", None),
119126
("() => { return (result = a * b); }", Some(serde_json::json!(["except-parens"]))),
120127
("() => (result = a * b)", Some(serde_json::json!(["except-parens"]))),
121128
("const foo = (a,b,c) => ((a = b), c)", None),

‎crates/oxc_linter/src/snapshots/no_return_assign.snap

+63-47
Original file line numberDiff line numberDiff line change
@@ -1,111 +1,127 @@
11
---
22
source: crates/oxc_linter/src/tester.rs
33
---
4-
⚠ eslint(no-return-assign): Return statement should not contain an assignment.
5-
╭─[no_return_assign.tsx:1:16]
4+
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
5+
╭─[no_return_assign.tsx:1:23]
66
1 │ function x() { return result = a * b; };
7-
· ──────────────────────
7+
· ──────────────
88
╰────
9+
help: Did you mean to use `==` instead of `=`?
910

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

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

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

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

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

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

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

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

58-
⚠ eslint(no-return-assign): Return statement should not contain an assignment.
59-
╭─[no_return_assign.tsx:2:20]
67+
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
68+
╭─[no_return_assign.tsx:2:27]
6069
1 │ function foo(){
6170
2 │ return a = b
62-
· ────────────
71+
· ─────
6372
3 │ }
6473
╰────
74+
help: Did you mean to use `==` instead of `=`?
6575

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

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

83-
⚠ eslint(no-return-assign): Return statement should not contain an assignment.
84-
╭─[no_return_assign.tsx:2:20]
95+
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
96+
╭─[no_return_assign.tsx:2:27]
8597
1 │ function doSomething() {
8698
2 │ return foo = () => a
87-
· ────────────────────
99+
· ─────────────
88100
3 │ }
89101
╰────
102+
help: Did you mean to use `==` instead of `=`?
90103

91-
⚠ eslint(no-return-assign): Arrow function should not return an assignment.
92-
╭─[no_return_assign.tsx:2:27]
104+
⚠ eslint(no-return-assign): Arrow functions should not return an assignment.
105+
╭─[no_return_assign.tsx:2:33]
93106
1 │ function doSomething() {
94107
2 │ return () => a = () => b
95-
· ─────────────────
108+
· ───────────
96109
3 │ }
97110
╰────
111+
help: Did you mean to use `==` instead of `=`?
98112

99-
⚠ eslint(no-return-assign): Return statement should not contain an assignment.
100-
╭─[no_return_assign.tsx:3:24]
113+
⚠ eslint(no-return-assign): Return statements should not contain an assignment.
114+
╭─[no_return_assign.tsx:3:31]
101115
2 │ return function bar(b){
102116
3 │ return a = b
103-
· ────────────
117+
· ─────
104118
4 │ }
105119
╰────
120+
help: Did you mean to use `==` instead of `=`?
106121

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

0 commit comments

Comments
 (0)
Please sign in to comment.