Skip to content

Commit 17e49c3

Browse files
ulrichstarkulrichstark
authored
feat(linter): implement configuration and checking loops for eslint/no_constant_condition (#10949)
Followup to #271 - implemented configuration correctly (supports boolean or choosing one of three enum variants) - implemented checking while, do-while and for loops - re-imported tests from eslint and re-ordered them to reduce git diff - updated rule docs to recommended format - added explanation of new configuration option to rule docs
1 parent 9a4b135 commit 17e49c3

File tree

2 files changed

+331
-75
lines changed

2 files changed

+331
-75
lines changed

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

Lines changed: 149 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
use oxc_ast::AstKind;
1+
use oxc_ast::{AstKind, ast::Expression};
22
use oxc_diagnostics::OxcDiagnostic;
33
use oxc_macros::declare_oxc_lint;
44
use oxc_span::{GetSpan, Span};
5+
use serde_json::Value;
56

67
use crate::{AstNode, ast_util::IsConstant, context::LintContext, rule::Rule};
78

@@ -11,9 +12,38 @@ fn no_constant_condition_diagnostic(span: Span) -> OxcDiagnostic {
1112
.with_label(span)
1213
}
1314

15+
#[derive(Debug, Default, Clone, PartialEq)]
16+
enum CheckLoops {
17+
All,
18+
#[default]
19+
AllExceptWhileTrue,
20+
None,
21+
}
22+
23+
impl CheckLoops {
24+
fn from(value: &Value) -> Option<Self> {
25+
match value {
26+
Value::String(str) => match str.as_str() {
27+
"all" => Some(Self::All),
28+
"allExceptWhileTrue" => Some(Self::AllExceptWhileTrue),
29+
"none" => Some(Self::None),
30+
_ => None,
31+
},
32+
Value::Bool(bool) => {
33+
if *bool {
34+
Some(Self::All)
35+
} else {
36+
Some(Self::None)
37+
}
38+
}
39+
_ => None,
40+
}
41+
}
42+
}
43+
1444
#[derive(Debug, Default, Clone)]
1545
pub struct NoConstantCondition {
16-
_check_loops: bool,
46+
check_loops: CheckLoops,
1747
}
1848

1949
declare_oxc_lint!(
@@ -31,73 +61,108 @@ declare_oxc_lint!(
3161
/// - `if`, `for`, `while`, or `do...while` statement
3262
/// - `?`: ternary expression
3363
///
34-
///
35-
/// ### Example
64+
/// ### Examples
3665
///
3766
/// Examples of **incorrect** code for this rule:
3867
/// ```js
3968
/// if (false) {
40-
/// doSomethingUnfinished();
69+
/// doSomethingUnfinished();
4170
/// }
4271
///
4372
/// if (new Boolean(x)) {
44-
/// doSomethingAlways();
73+
/// doSomethingAlways();
4574
/// }
4675
/// if (x ||= true) {
47-
/// doSomethingAlways();
76+
/// doSomethingAlways();
4877
/// }
4978
///
5079
/// do {
51-
/// doSomethingForever();
80+
/// doSomethingForever();
5281
/// } while (x = -1);
5382
/// ```
5483
///
5584
/// Examples of **correct** code for this rule:
5685
/// ```js
5786
/// if (x === 0) {
58-
/// doSomething();
87+
/// doSomething();
5988
/// }
6089
///
6190
/// while (typeof x === "undefined") {
62-
/// doSomething();
91+
/// doSomething();
6392
/// }
6493
/// ```
94+
///
95+
/// ### Options
96+
///
97+
/// #### checkLoops
98+
///
99+
/// `{ type: "all" | "allExceptWhileTrue" | "none" | boolean, default: "allExceptWhileTrue" }`
100+
///
101+
/// - `"all"` or `true` disallows constant expressions in loops
102+
/// - `"allExceptWhileTrue"` disallows constant expressions in loops except while loops with expression `true`
103+
/// - `"none"` or `false` allows constant expressions in loops
65104
NoConstantCondition,
66105
eslint,
67106
correctness
68107
);
69108

70109
impl Rule for NoConstantCondition {
71-
fn from_configuration(value: serde_json::Value) -> Self {
110+
fn from_configuration(value: Value) -> Self {
72111
let obj = value.get(0);
73112

74113
Self {
75-
_check_loops: obj
114+
check_loops: obj
76115
.and_then(|v| v.get("checkLoops"))
77-
.and_then(serde_json::Value::as_bool)
116+
.and_then(CheckLoops::from)
78117
.unwrap_or_default(),
79118
}
80119
}
81120

82121
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
83122
match node.kind() {
84-
AstKind::IfStatement(if_stmt) => {
85-
if if_stmt.test.is_constant(true, ctx) {
86-
ctx.diagnostic(no_constant_condition_diagnostic(if_stmt.test.span()));
87-
}
123+
AstKind::IfStatement(if_stmt) => check(ctx, &if_stmt.test),
124+
AstKind::ConditionalExpression(condition_expr) => check(ctx, &condition_expr.test),
125+
AstKind::WhileStatement(while_stmt) => self.check_loop(ctx, &while_stmt.test, true),
126+
AstKind::DoWhileStatement(do_while_stmt) => {
127+
self.check_loop(ctx, &do_while_stmt.test, false);
88128
}
89-
AstKind::ConditionalExpression(condition_expr) => {
90-
if condition_expr.test.is_constant(true, ctx) {
91-
ctx.diagnostic(no_constant_condition_diagnostic(condition_expr.test.span()));
92-
}
129+
AstKind::ForStatement(for_stmt) => {
130+
let Some(test) = &for_stmt.test else {
131+
return;
132+
};
133+
134+
self.check_loop(ctx, test, false);
93135
}
94136
_ => {}
95137
}
96138
}
97139
}
98140

141+
impl NoConstantCondition {
142+
fn check_loop<'a>(&self, ctx: &LintContext<'a>, test: &'a Expression<'_>, is_while: bool) {
143+
match self.check_loops {
144+
CheckLoops::None => return,
145+
CheckLoops::AllExceptWhileTrue if is_while => match test {
146+
Expression::BooleanLiteral(bool) if bool.value => return,
147+
_ => {}
148+
},
149+
_ => {}
150+
}
151+
152+
check(ctx, test);
153+
}
154+
}
155+
156+
fn check<'a>(ctx: &LintContext<'a>, test: &'a Expression<'_>) {
157+
if test.is_constant(true, ctx) {
158+
ctx.diagnostic(no_constant_condition_diagnostic(test.span()));
159+
}
160+
}
161+
99162
#[test]
100163
fn test() {
164+
use serde_json::json;
165+
101166
use crate::tester::Tester;
102167

103168
let pass = vec![
@@ -220,28 +285,36 @@ fn test() {
220285
// ("const undefined = 'lol'; if (undefined) {}", None),
221286
// ("function foo(Boolean) { if (Boolean(1)) {} }", None),
222287
// ("const Boolean = () => {}; if (Boolean(1)) {}", None),
223-
// "if (Boolean()) {}",
224-
// "if (undefined) {}",
288+
// ("if (Boolean()) {}", None),
289+
// ("if (undefined) {}", None),
225290
("q > 0 ? 1 : 2;", None),
226291
("`${a}` === a ? 1 : 2", None),
227292
("`foo${a}` === a ? 1 : 2", None),
228293
("tag`a` === a ? 1 : 2", None),
229294
("tag`${a}` === a ? 1 : 2", None),
230-
//TODO
231-
// ("while(~!a);", None),
232-
// ("while(a = b);", None),
233-
// ("while(`${a}`);", None),
234-
// ("for(;x < 10;);", None),
235-
// ("for(;;);", None),
236-
// ("for(;`${a}`;);", None),
237-
// ("do{ }while(x)", None),
238-
// ("while(x += 3) {}", None),
239-
// ("while(tag`a`) {}", None),
240-
// ("while(tag`${a}`) {}", None),
241-
// ("while(`\\\n${a}`) {}", None),
242-
// ("while(true);", Some(json!([{"checkLoops":false}]))),
243-
// ("for(;true;);", Some(json!([{"checkLoops":false}]))),
244-
// ("do{}while(true)", Some(json!([{"checkLoops":false}]))),
295+
("while(~!a);", None),
296+
("while(a = b);", None),
297+
("while(`${a}`);", None),
298+
("for(;x < 10;);", None),
299+
("for(;;);", None),
300+
("for(;`${a}`;);", None),
301+
("do{ }while(x)", None),
302+
("while(x += 3) {}", None),
303+
("while(tag`a`) {}", None),
304+
("while(tag`${a}`) {}", None),
305+
("while(`\\\n${a}`) {}", None),
306+
("while(true);", Some(json!([{ "checkLoops": false }]))),
307+
("for(;true;);", Some(json!([{ "checkLoops": false }]))),
308+
("do{}while(true)", Some(json!([{ "checkLoops": "none" }]))),
309+
("while(true);", Some(json!([{ "checkLoops": "none" }]))),
310+
("for(;true;);", Some(json!([{ "checkLoops": "none" }]))),
311+
("do{ }while(x);", Some(json!([{ "checkLoops": "all" }]))),
312+
("while(true);", Some(json!([{ "checkLoops": "allExceptWhileTrue" }]))),
313+
("while(true);", None),
314+
("while(a == b);", Some(json!([{ "checkLoops": "all" }]))),
315+
("for (let x = 0; x <= 10; x++) {};", Some(json!([{ "checkLoops": "all" }]))),
316+
("do{}while(true)", Some(json!([{ "checkLoops": false }]))),
317+
// TODO
245318
// ("function* foo(){while(true){yield 'foo';}}", None),
246319
// ("function* foo(){for(;true;){yield 'foo';}}", None),
247320
// ("function* foo(){do{yield 'foo';}while(true)}", None),
@@ -254,7 +327,6 @@ fn test() {
254327
];
255328

256329
let fail = vec![
257-
("if(-2);", None),
258330
("if(-2);", None),
259331
("if(true);", None),
260332
("if(1);", None),
@@ -306,11 +378,13 @@ fn test() {
306378
("if((a &&= null) && b);", None),
307379
("if(false || (a &&= false));", None),
308380
("if((a &&= false) || false);", None),
381+
("while(x = 1);", None),
309382
("if(typeof x){}", None),
310383
("if(typeof 'abc' === 'string'){}", None),
311384
("if(a = typeof b){}", None),
312385
("if(a, typeof b){}", None),
313386
("if(typeof 'a' == 'string' || typeof 'b' == 'string'){}", None),
387+
("while(typeof x){}", None),
314388
("if(1 || void x);", None),
315389
("if(void x);", None),
316390
("if(y = void x);", None),
@@ -370,39 +444,46 @@ fn test() {
370444
("`` ? 1 : 2;", None),
371445
("`foo` ? 1 : 2;", None),
372446
("`foo${bar}` ? 1 : 2;", None),
447+
("for(;true;);", None),
448+
("for(;``;);", None),
449+
("for(;`foo`;);", None),
450+
("for(;`foo${bar}`;);", None),
451+
("do{}while(true)", None),
452+
("do{}while('1')", None),
453+
("do{}while(0)", None),
454+
("do{}while(t = -2)", None),
455+
("do{}while(``)", None),
456+
("do{}while(`foo`)", None),
457+
("do{}while(`foo${bar}`)", None),
458+
("while([]);", None),
459+
("while(~!0);", None),
460+
("while(x = 1);", Some(json!([{ "checkLoops": "all" }]))),
461+
("while(function(){});", None),
462+
("while(true);", Some(json!([{ "checkLoops": "all" }]))),
463+
("while(1);", None),
464+
("while(() => {});", None),
465+
("while(`foo`);", None),
466+
("while(``);", None),
467+
("while(`${'foo'}`);", None),
468+
("while(`${'foo' + 'bar'}`);", None),
469+
("do{ }while(x = 1)", Some(json!([{ "checkLoops": "all" }]))),
470+
("for (;true;) {};", Some(json!([{ "checkLoops": "all" }]))),
373471
// TODO
374-
// ("for(;true;);", None),
375-
// ("for(;``;);", None),
376-
// ("for(;`foo`;);", None),
377-
// ("for(;`foo${bar}`;);", None),
378-
// ("do{}while(true)", None),
379-
// ("do{}while('1')", None),
380-
// ("do{}while(0)", None),
381-
// ("do{}while(t = -2)", None),
382-
// ("do{}while(``)", None),
383-
// ("do{}while(`foo`)", None),
384-
// ("do{}while(`foo${bar}`)", None),
385-
// ("while([]);", None),
386-
// ("while(~!0);", None),
387-
// ("while(x = 1);", None),
388-
// ("while(function(){});", None),
389-
// ("while(true);", None),
390-
// ("while(1);", None),
391-
// ("while(() => {});", None),
392-
// ("while(`foo`);", None),
393-
// ("while(``);", None),
394-
// ("while(`${'foo'}`);", None),
395-
// ("while(`${'foo' + 'bar'}`);", None),
396-
// ("function* foo(){while(true){} yield 'foo';}", None),
397-
// ("function* foo(){while(true){if (true) {yield 'foo';}}}", None),
398-
// ("function* foo(){while(true){yield 'foo';} while(true) {}}", None),
399-
// ("var a = function* foo(){while(true){} yield 'foo';}", None),
400-
// ("while (true) { function* foo() {yield;}}", None),
472+
// ("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": "all" }]))),
473+
// ("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": true }]))),
474+
// ("function* foo(){while(true){if (true) {yield 'foo';}}}",Some(json!([{ "checkLoops": "all" }])),),
475+
// ("function* foo(){while(true){if (true) {yield 'foo';}}}",Some(json!([{ "checkLoops": true }])),),
476+
// ("function* foo(){while(true){yield 'foo';} while(true) {}}", Some(json!([{ "checkLoops": "all" }])),),
477+
// ("function* foo(){while(true){yield 'foo';} while(true) {}}",Some(json!([{ "checkLoops": true }])),),
478+
// ("var a = function* foo(){while(true){} yield 'foo';}",Some(json!([{ "checkLoops": "all" }])),),
479+
// ("var a = function* foo(){while(true){} yield 'foo';}",Some(json!([{ "checkLoops": true }])),),
480+
// ("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": "all" }]))),
481+
// ("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": true }]))),
401482
// ("function* foo(){if (true) {yield 'foo';}}", None),
402483
// ("function* foo() {for (let foo = yield; true;) {}}", None),
403484
// ("function* foo() {for (foo = yield; true;) {}}", None),
404-
// ("function foo() {while (true) {function* bar() {while (true) {yield;}}}}", None),
405-
// ("function foo() {while (true) {const bar = function*() {while (true) {yield;}}}}", None),
485+
// ("function foo() {while (true) {function* bar() {while (true) {yield;}}}}",Some(json!([{ "checkLoops": "all" }])),),
486+
// ("function foo() {while (true) {const bar = function*() {while (true) {yield;}}}}",Some(json!([{ "checkLoops": "all" }])),),
406487
// ("function* foo() { for (let foo = 1 + 2 + 3 + (yield); true; baz) {}}", None),
407488
];
408489

0 commit comments

Comments
 (0)