Skip to content

Commit

Permalink
feat(rome_js_analyze): Suggested fix for Conditional Assignment (rome…
Browse files Browse the repository at this point in the history 
…#3771)
  • Loading branch information
@95th
95th authored Nov 17, 2022
1 parent ce16cb8 commit 9f0ba14
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 25 deletions.
25 changes: 23 additions & 2 deletions crates/rome_js_analyze/src/analyzers/nursery/no_conditional_assignment.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
use rome_analyze::context::RuleContext;
use rome_analyze::{declare_rule, Ast, Rule, RuleDiagnostic};
use rome_analyze::{declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_factory::make;
use rome_js_syntax::*;
use rome_rowan::{declare_node_union, AstNode};
use rome_rowan::{declare_node_union, AstNode, BatchMutationExt};

use crate::JsRuleAction;

declare_rule! {
/// Disallow assignment operators in conditional expressions.
Expand Down Expand Up @@ -79,6 +83,23 @@ impl Rule for NoConditionalAssignment {
},
))
}

fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
let op = state.operator().ok()?;
if let JsAssignmentOperator::Assign = op {
let mut mutation = ctx.root().begin();
let token = state.operator_token().ok()?;
mutation.replace_token(token, make::token(JsSyntaxKind::EQ3));
Some(JsRuleAction {
mutation,
applicability: Applicability::MaybeIncorrect,
category: ActionCategory::QuickFix,
message: markup!("Did you mean '==='?").to_owned(),
})
} else {
None
}
}
}

fn into_js_assignment(expr: JsAnyExpression) -> Option<JsAssignmentExpression> {
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/tests/specs/nursery/noConditionalAssignment/invalid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ for(; (x = y); ) { }
var x; var b = (x = 0) ? 1 : 0;
var x; var b = x && (y = 0) ? 1 : 0;
(((3496.29)).bkufyydt = 2e308) ? foo : bar;

if (a += b) {}
while ((c *= d)) {}
117 changes: 101 additions & 16 deletions crates/rome_js_analyze/tests/specs/nursery/noConditionalAssignment/invalid.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ for(; (x = y); ) { }
var x; var b = (x = 0) ? 1 : 0;
var x; var b = x && (y = 0) ? 1 : 0;
(((3496.29)).bkufyydt = 2e308) ? foo : bar;

if (a += b) {}
while ((c *= d)) {}
```

# Diagnostics
```
invalid.js:1:12 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:1:12 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -38,11 +39,15 @@ invalid.js:1:12 lint/nursery/noConditionalAssignment ━━━━━━━━━
2 │ var x; while (x = 0) { var b = 1; }
3 │ var x = 0, y; do { y = x; } while (x = x + 1);
i Suggested fix: Did you mean '==='?
1 │ var·x;·if·(x·===·0)·{·var·b·=·1;·}
│ ++
```

```
invalid.js:2:15 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:2:15 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -52,11 +57,15 @@ invalid.js:2:15 lint/nursery/noConditionalAssignment ━━━━━━━━━
3 │ var x = 0, y; do { y = x; } while (x = x + 1);
4 │ var x; for(; x+=1 ;){};
i Suggested fix: Did you mean '==='?
2 │ var·x;·while·(x·===·0)·{·var·b·=·1;·}
│ ++
```

```
invalid.js:3:36 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:3:36 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -67,6 +76,10 @@ invalid.js:3:36 lint/nursery/noConditionalAssignment ━━━━━━━━━
4 │ var x; for(; x+=1 ;){};
5 │ var x; if ((x) = (0));
i Suggested fix: Did you mean '==='?
3 │ var·x·=·0,·y;·do·{·y·=·x;·}·while·(x·===·x·+·1);
│ ++
```

Expand All @@ -86,7 +99,7 @@ invalid.js:4:14 lint/nursery/noConditionalAssignment ━━━━━━━━━
```

```
invalid.js:5:12 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:5:12 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -97,11 +110,15 @@ invalid.js:5:12 lint/nursery/noConditionalAssignment ━━━━━━━━━
6 │ if (someNode || (someNode = parentNode)) { }
7 │ while (someNode || (someNode = parentNode)) { }
i Suggested fix: Did you mean '==='?
5 │ var·x;·if·((x)·===·(0));
│ ++
```

```
invalid.js:10:5 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:10:5 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -112,11 +129,15 @@ invalid.js:10:5 lint/nursery/noConditionalAssignment ━━━━━━━━━
11 │ while (x = 0) { }
12 │ do { } while (x = x + 1);
i Suggested fix: Did you mean '==='?
10 │ if·(x·===·0)·{·}
│ ++
```

```
invalid.js:11:8 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:11:8 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -127,11 +148,15 @@ invalid.js:11:8 lint/nursery/noConditionalAssignment ━━━━━━━━━
12 │ do { } while (x = x + 1);
13 │ for(; x = y; ) { }
i Suggested fix: Did you mean '==='?
11 │ while·(x·===·0)·{·}
│ ++
```

```
invalid.js:12:15 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:12:15 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -142,11 +167,15 @@ invalid.js:12:15 lint/nursery/noConditionalAssignment ━━━━━━━━
13 │ for(; x = y; ) { }
14 │ if ((x = 0)) { }
i Suggested fix: Did you mean '==='?
12 │ do·{·}·while·(x·===·x·+·1);
│ ++
```

```
invalid.js:13:7 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:13:7 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -157,11 +186,15 @@ invalid.js:13:7 lint/nursery/noConditionalAssignment ━━━━━━━━━
14 │ if ((x = 0)) { }
15 │ while ((x = 0)) { }
i Suggested fix: Did you mean '==='?
13 │ for(;·x·===·y;·)·{·}
│ ++
```

```
invalid.js:14:6 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:14:6 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -172,11 +205,15 @@ invalid.js:14:6 lint/nursery/noConditionalAssignment ━━━━━━━━━
15 │ while ((x = 0)) { }
16 │ do { } while ((x = x + 1));
i Suggested fix: Did you mean '==='?
14 │ if·((x·===·0))·{·}
│ ++
```

```
invalid.js:15:9 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:15:9 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -187,11 +224,15 @@ invalid.js:15:9 lint/nursery/noConditionalAssignment ━━━━━━━━━
16 │ do { } while ((x = x + 1));
17 │ for(; (x = y); ) { }
i Suggested fix: Did you mean '==='?
15 │ while·((x·===·0))·{·}
│ ++
```

```
invalid.js:16:16 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:16:16 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -202,11 +243,15 @@ invalid.js:16:16 lint/nursery/noConditionalAssignment ━━━━━━━━
17 │ for(; (x = y); ) { }
18 │ var x; var b = (x = 0) ? 1 : 0;
i Suggested fix: Did you mean '==='?
16 │ do·{·}·while·((x·===·x·+·1));
│ ++
```

```
invalid.js:17:8 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:17:8 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -217,11 +262,15 @@ invalid.js:17:8 lint/nursery/noConditionalAssignment ━━━━━━━━━
18 │ var x; var b = (x = 0) ? 1 : 0;
19 │ var x; var b = x && (y = 0) ? 1 : 0;
i Suggested fix: Did you mean '==='?
17 │ for(;·(x·===·y);·)·{·}
│ ++
```

```
invalid.js:18:17 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:18:17 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -232,19 +281,55 @@ invalid.js:18:17 lint/nursery/noConditionalAssignment ━━━━━━━━
19 │ var x; var b = x && (y = 0) ? 1 : 0;
20 │ (((3496.29)).bkufyydt = 2e308) ? foo : bar;
i Suggested fix: Did you mean '==='?
18 │ var·x;·var·b·=·(x·===·0)·?·1·:·0;
│ ++
```

```
invalid.js:20:2 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:20:2 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
18 │ var x; var b = (x = 0) ? 1 : 0;
19 │ var x; var b = x && (y = 0) ? 1 : 0;
> 20 │ (((3496.29)).bkufyydt = 2e308) ? foo : bar;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
21 │
21 │ if (a += b) {}
22 │ while ((c *= d)) {}
i Suggested fix: Did you mean '==='?
20 │ (((3496.29)).bkufyydt·===·2e308)·?·foo·:·bar;
│ ++
```

```
invalid.js:21:5 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
19 │ var x; var b = x && (y = 0) ? 1 : 0;
20 │ (((3496.29)).bkufyydt = 2e308) ? foo : bar;
> 21 │ if (a += b) {}
│ ^^^^^^
22 │ while ((c *= d)) {}
```

```
invalid.js:22:9 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
20 │ (((3496.29)).bkufyydt = 2e308) ? foo : bar;
21 │ if (a += b) {}
> 22 │ while ((c *= d)) {}
│ ^^^^^^
```
Expand Down
24 changes: 20 additions & 4 deletions crates/rome_js_analyze/tests/specs/nursery/noConditionalAssignment/valid.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ switch (foo) { case baz + (a = b): bar(); }

# Diagnostics
```
valid.js:6:6 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
valid.js:6:6 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -45,11 +45,15 @@ valid.js:6:6 lint/nursery/noConditionalAssignment ━━━━━━━━━━
7 │ while ((a = b));
8 │ do {} while ((a = b));
i Suggested fix: Did you mean '==='?
6 │ if·((a·===·b));
│ ++
```

```
valid.js:7:9 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
valid.js:7:9 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -60,11 +64,15 @@ valid.js:7:9 lint/nursery/noConditionalAssignment ━━━━━━━━━━
8 │ do {} while ((a = b));
9 │ for (;(a = b););
i Suggested fix: Did you mean '==='?
7 │ while·((a·===·b));
│ ++
```

```
valid.js:8:15 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
valid.js:8:15 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -75,11 +83,15 @@ valid.js:8:15 lint/nursery/noConditionalAssignment ━━━━━━━━━
9 │ for (;(a = b););
10 │ for (;;) {}
i Suggested fix: Did you mean '==='?
8 │ do·{}·while·((a·===·b));
│ ++
```

```
valid.js:9:8 lint/nursery/noConditionalAssignment ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
valid.js:9:8 lint/nursery/noConditionalAssignment FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Expected a conditional expression and instead saw an assignment.
Expand All @@ -90,6 +102,10 @@ valid.js:9:8 lint/nursery/noConditionalAssignment ━━━━━━━━━━
10 │ for (;;) {}
11 │ if (someNode || (someNode = parentNode)) { }
i Suggested fix: Did you mean '==='?
9 │ for·(;(a·===·b););
│ ++
```

Expand Down
Loading

0 comments on commit 9f0ba14

Please sign in to comment.