Skip to content

Commit b0b6ac7

Browse files
committedOct 19, 2024·
fix(linter/no-cond-assign): false positive when assignment is in body statement (#6665)
- fixes #6656 rather than reporting any assignment inside of `if`, `while`, etc. when the `always` option is enabled, we only check if the assignment resides in specific areas that assignment should not be. for example, inside the test of an `if`, or the test of a `for` loop. incidentally, this also fixes an issue (seen in snapshot file) where the same assignment was reported twice.
1 parent 70c6f24 commit b0b6ac7

File tree

2 files changed

+58
-17
lines changed

2 files changed

+58
-17
lines changed
 

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

+58-10
Original file line numberDiff line numberDiff line change
@@ -78,21 +78,46 @@ impl Rule for NoCondAssign {
7878
self.check_expression(ctx, expr.test.get_inner_expression());
7979
}
8080
AstKind::AssignmentExpression(expr) if self.config == NoCondAssignConfig::Always => {
81-
for node_id in ctx.nodes().ancestors(node.id()).skip(1) {
82-
match ctx.nodes().kind(node_id) {
83-
AstKind::IfStatement(_)
84-
| AstKind::WhileStatement(_)
85-
| AstKind::DoWhileStatement(_)
86-
| AstKind::ForStatement(_)
87-
| AstKind::ConditionalExpression(_) => {
88-
Self::emit_diagnostic(ctx, expr);
81+
let mut spans = vec![];
82+
for ancestor in ctx.nodes().iter_parents(node.id()).skip(1) {
83+
match ancestor.kind() {
84+
AstKind::IfStatement(if_stmt) => {
85+
spans.push(if_stmt.test.span());
86+
}
87+
AstKind::WhileStatement(while_stmt) => {
88+
spans.push(while_stmt.test.span());
89+
}
90+
AstKind::DoWhileStatement(do_while_stmt) => {
91+
spans.push(do_while_stmt.test.span());
92+
}
93+
AstKind::ForStatement(for_stmt) => {
94+
if let Some(test) = &for_stmt.test {
95+
spans.push(test.span());
96+
}
97+
if let Some(update) = &for_stmt.update {
98+
spans.push(update.span());
99+
}
100+
if let Some(update) = &for_stmt.update {
101+
spans.push(update.span());
102+
}
103+
}
104+
AstKind::ConditionalExpression(cond_expr) => {
105+
spans.push(cond_expr.span());
89106
}
90107
AstKind::Function(_)
91108
| AstKind::ArrowFunctionExpression(_)
92109
| AstKind::Program(_)
93-
| AstKind::BlockStatement(_) => break,
110+
| AstKind::BlockStatement(_) => {
111+
break;
112+
}
94113
_ => {}
95-
}
114+
};
115+
}
116+
117+
// Only report the diagnostic if the assignment is in a span where it should not be.
118+
// For example, report `if (a = b) { ...}`, not `if (...) { a = b }`
119+
if spans.iter().any(|span| span.contains_inclusive(node.span())) {
120+
Self::emit_diagnostic(ctx, expr);
96121
}
97122
}
98123
_ => {}
@@ -177,6 +202,29 @@ fn test() {
177202
("for (;;) { (obj.key=false) }", Some(serde_json::json!(["always"]))),
178203
("while (obj.key) { (obj.key=false) }", Some(serde_json::json!(["always"]))),
179204
("do { (obj.key=false) } while (obj.key)", Some(serde_json::json!(["always"]))),
205+
// https://github.com/oxc-project/oxc/issues/6656
206+
(
207+
"
208+
if (['a', 'b', 'c', 'd'].includes(value)) newValue = value;
209+
else newValue = 'default';
210+
",
211+
Some(serde_json::json!(["always"])),
212+
),
213+
(
214+
"
215+
while(true) newValue = value;
216+
",
217+
Some(serde_json::json!(["always"])),
218+
),
219+
(
220+
"
221+
for(;;) newValue = value;
222+
",
223+
Some(serde_json::json!(["always"])),
224+
),
225+
("for (; (typeof l === 'undefined' ? (l = 0) : l); i++) { }", None),
226+
("for (x = 0;x<10;x++) { x = 0 }", None),
227+
("for (x = 0;x<10;(x = x + 1)) { x = 0 }", None),
180228
];
181229

182230
let fail = vec![

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

-7
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,6 @@ source: crates/oxc_linter/src/tester.rs
6464
╰────
6565
help: Consider wrapping the assignment in additional parentheses
6666

67-
eslint(no-cond-assign): Expected a conditional expression and instead saw an assignment
68-
╭─[no_cond_assign.tsx:1:39]
69-
1for (; (typeof l === 'undefined' ? (l = 0) : l); i++) { }
70-
· ─
71-
╰────
72-
help: Consider wrapping the assignment in additional parentheses
73-
7467
eslint(no-cond-assign): Expected a conditional expression and instead saw an assignment
7568
╭─[no_cond_assign.tsx:1:7]
7669
1if (x = 0) { }

0 commit comments

Comments
 (0)
Please sign in to comment.