dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-19 04:08:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(linter/no-cond-assign): false positive when assignment is in body statement (#6665)

Browse source 
- fixes https://github.com/oxc-project/oxc/issues/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.
This commit is contained in:
camchenry 2024-10-19 03:17:07 +00:00
parent 70c6f24a08
commit b0b6ac71e7
2 changed files with 58 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

68
crates/oxc_linter/src/rules/eslint/no_cond_assign.rs
View file

@ -78,21 +78,46 @@ impl Rule for NoCondAssign {
self.check_expression(ctx, expr.test.get_inner_expression());
}
AstKind::AssignmentExpression(expr) if self.config == NoCondAssignConfig::Always => {
for node_id in ctx.nodes().ancestors(node.id()).skip(1) {
match ctx.nodes().kind(node_id) {
AstKind::IfStatement(_)
| AstKind::WhileStatement(_)
| AstKind::DoWhileStatement(_)
| AstKind::ForStatement(_)
| AstKind::ConditionalExpression(_) => {
Self::emit_diagnostic(ctx, expr);
let mut spans = vec![];
for ancestor in ctx.nodes().iter_parents(node.id()).skip(1) {
match ancestor.kind() {
AstKind::IfStatement(if_stmt) => {
spans.push(if_stmt.test.span());
}
AstKind::WhileStatement(while_stmt) => {
spans.push(while_stmt.test.span());
}
AstKind::DoWhileStatement(do_while_stmt) => {
spans.push(do_while_stmt.test.span());
}
AstKind::ForStatement(for_stmt) => {
if let Some(test) = &for_stmt.test {
spans.push(test.span());
}
if let Some(update) = &for_stmt.update {
spans.push(update.span());
}
if let Some(update) = &for_stmt.update {
spans.push(update.span());
}
}
AstKind::ConditionalExpression(cond_expr) => {
spans.push(cond_expr.span());
}
AstKind::Function(_)
| AstKind::ArrowFunctionExpression(_)
| AstKind::Program(_)
| AstKind::BlockStatement(_) => break,
| AstKind::BlockStatement(_) => {
break;
}
_ => {}
}
};
}
// Only report the diagnostic if the assignment is in a span where it should not be.
// For example, report `if (a = b) { ...}`, not `if (...) { a = b }`
if spans.iter().any(|span| span.contains_inclusive(node.span())) {
Self::emit_diagnostic(ctx, expr);
}
}
_ => {}
@ -177,6 +202,29 @@ fn test() {
("for (;;) { (obj.key=false) }", Some(serde_json::json!(["always"]))),
("while (obj.key) { (obj.key=false) }", Some(serde_json::json!(["always"]))),
("do { (obj.key=false) } while (obj.key)", Some(serde_json::json!(["always"]))),
// https://github.com/oxc-project/oxc/issues/6656
(
"
if (['a', 'b', 'c', 'd'].includes(value)) newValue = value;
else newValue = 'default';
",
Some(serde_json::json!(["always"])),
),
(
"
while(true) newValue = value;
",
Some(serde_json::json!(["always"])),
),
(
"
for(;;) newValue = value;
",
Some(serde_json::json!(["always"])),
),
("for (; (typeof l === 'undefined' ? (l = 0) : l); i++) { }", None),
("for (x = 0;x<10;x++) { x = 0 }", None),
("for (x = 0;x<10;(x = x + 1)) { x = 0 }", None),
];
let fail = vec![

7
crates/oxc_linter/src/snapshots/no_cond_assign.snap
View file

@ -64,13 +64,6 @@ source: crates/oxc_linter/src/tester.rs
╰────
help: Consider wrapping the assignment in additional parentheses
⚠ eslint(no-cond-assign): Expected a conditional expression and instead saw an assignment
╭─[no_cond_assign.tsx:1:39]
1 │ for (; (typeof l === 'undefined' ? (l = 0) : l); i++) { }
· ─
╰────
help: Consider wrapping the assignment in additional parentheses
⚠ eslint(no-cond-assign): Expected a conditional expression and instead saw an assignment
╭─[no_cond_assign.tsx:1:7]
1 │ if (x = 0) { }