From 47af88715585f156495f91f198709289eb0feefa Mon Sep 17 00:00:00 2001 From: Ulrich Stark Date: Sun, 11 May 2025 21:47:06 +0200 Subject: [PATCH 1/4] check loops and implement configuration correctly --- .../src/rules/eslint/no_constant_condition.rs | 235 ++++++++++++------ .../eslint_no_constant_condition.snap | 189 +++++++++++++- 2 files changed, 339 insertions(+), 85 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs b/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs index f0dbf46af2d42..68d6ccb9d2709 100644 --- a/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs +++ b/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs @@ -1,7 +1,8 @@ -use oxc_ast::AstKind; +use oxc_ast::{AstKind, ast::Expression}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_span::{GetSpan, Span}; +use serde_json::Value; use crate::{AstNode, ast_util::IsConstant, context::LintContext, rule::Rule}; @@ -11,9 +12,38 @@ fn no_constant_condition_diagnostic(span: Span) -> OxcDiagnostic { .with_label(span) } +#[derive(Debug, Default, Clone, PartialEq)] +enum CheckLoops { + All, + #[default] + AllExceptWhileTrue, + None, +} + +impl CheckLoops { + fn from(value: &Value) -> Option { + match value { + Value::String(str) => match str.as_str() { + "all" => Some(Self::All), + "allExceptWhileTrue" => Some(Self::AllExceptWhileTrue), + "none" => Some(Self::None), + _ => None, + }, + Value::Bool(bool) => { + if *bool { + Some(Self::All) + } else { + Some(Self::None) + } + } + _ => None, + } + } +} + #[derive(Debug, Default, Clone)] pub struct NoConstantCondition { - _check_loops: bool, + check_loops: CheckLoops, } declare_oxc_lint!( @@ -68,36 +98,72 @@ declare_oxc_lint!( ); impl Rule for NoConstantCondition { - fn from_configuration(value: serde_json::Value) -> Self { + fn from_configuration(value: Value) -> Self { let obj = value.get(0); Self { - _check_loops: obj + check_loops: obj .and_then(|v| v.get("checkLoops")) - .and_then(serde_json::Value::as_bool) + .and_then(CheckLoops::from) .unwrap_or_default(), } } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { match node.kind() { - AstKind::IfStatement(if_stmt) => { - if if_stmt.test.is_constant(true, ctx) { - ctx.diagnostic(no_constant_condition_diagnostic(if_stmt.test.span())); - } + AstKind::IfStatement(if_stmt) => check(ctx, &if_stmt.test), + AstKind::ConditionalExpression(condition_expr) => check(ctx, &condition_expr.test), + AstKind::WhileStatement(while_stmt) => self.check_loop(ctx, &while_stmt.test, true), + AstKind::DoWhileStatement(do_while_stmt) => { + self.check_loop(ctx, &do_while_stmt.test, false); } - AstKind::ConditionalExpression(condition_expr) => { - if condition_expr.test.is_constant(true, ctx) { - ctx.diagnostic(no_constant_condition_diagnostic(condition_expr.test.span())); - } + AstKind::ForStatement(for_stmt) => { + let Some(test) = &for_stmt.test else { + return; + }; + + self.check_loop(ctx, test, false); } _ => {} } } } +impl NoConstantCondition { + fn check_loop<'a>(&self, ctx: &LintContext<'a>, test: &'a Expression<'_>, is_while: bool) { + let return_early = match self.check_loops { + CheckLoops::None => true, + CheckLoops::AllExceptWhileTrue => { + if is_while { + match test { + Expression::BooleanLiteral(bool) => bool.value, + _ => false, + } + } else { + false + } + } + CheckLoops::All => false, + }; + + if return_early { + return; + } + + check(ctx, test); + } +} + +fn check<'a>(ctx: &LintContext<'a>, test: &'a Expression<'_>) { + if test.is_constant(true, ctx) { + ctx.diagnostic(no_constant_condition_diagnostic(test.span())); + } +} + #[test] fn test() { + use serde_json::json; + use crate::tester::Tester; let pass = vec![ @@ -216,45 +282,50 @@ fn test() { ("if (Boolean(a)) {}", None), ("if (Boolean(...args)) {}", None), ("if (foo.Boolean(1)) {}", None), - // TODO - // ("const undefined = 'lol'; if (undefined) {}", None), - // ("function foo(Boolean) { if (Boolean(1)) {} }", None), - // ("const Boolean = () => {}; if (Boolean(1)) {}", None), - // "if (Boolean()) {}", - // "if (undefined) {}", + ("const undefined = 'lol'; if (undefined) {}", None), + ("function foo(Boolean) { if (Boolean(1)) {} }", None), + ("const Boolean = () => {}; if (Boolean(1)) {}", None), + // TODO: ("if (Boolean()) {}", None), + // TODO: ("if (undefined) {}", None), ("q > 0 ? 1 : 2;", None), ("`${a}` === a ? 1 : 2", None), ("`foo${a}` === a ? 1 : 2", None), ("tag`a` === a ? 1 : 2", None), ("tag`${a}` === a ? 1 : 2", None), - //TODO - // ("while(~!a);", None), - // ("while(a = b);", None), - // ("while(`${a}`);", None), - // ("for(;x < 10;);", None), - // ("for(;;);", None), - // ("for(;`${a}`;);", None), - // ("do{ }while(x)", None), - // ("while(x += 3) {}", None), - // ("while(tag`a`) {}", None), - // ("while(tag`${a}`) {}", None), - // ("while(`\\\n${a}`) {}", None), - // ("while(true);", Some(json!([{"checkLoops":false}]))), - // ("for(;true;);", Some(json!([{"checkLoops":false}]))), - // ("do{}while(true)", Some(json!([{"checkLoops":false}]))), - // ("function* foo(){while(true){yield 'foo';}}", None), - // ("function* foo(){for(;true;){yield 'foo';}}", None), - // ("function* foo(){do{yield 'foo';}while(true)}", None), - // ("function* foo(){while (true) { while(true) {yield;}}}", None), - // ("function* foo() {for (; yield; ) {}}", None), - // ("function* foo() {for (; ; yield) {}}", None), - // ("function* foo() {while (true) {function* foo() {yield;}yield;}}", None), - // ("function* foo() { for (let x = yield; x < 10; x++) {yield;}yield;}", None), - // ("function* foo() { for (let x = yield; ; x++) { yield; }}", None), + ("while(~!a);", None), + ("while(a = b);", None), + ("while(`${a}`);", None), + ("for(;x < 10;);", None), + ("for(;;);", None), + ("for(;`${a}`;);", None), + ("do{ }while(x)", None), + ("while(x += 3) {}", None), + ("while(tag`a`) {}", None), + ("while(tag`${a}`) {}", None), + ("while(`\\\n${a}`) {}", None), + ("while(true);", Some(json!([{ "checkLoops": false }]))), + ("for(;true;);", Some(json!([{ "checkLoops": false }]))), + ("do{}while(true)", Some(json!([{ "checkLoops": "none" }]))), + ("while(true);", Some(json!([{ "checkLoops": "none" }]))), + ("for(;true;);", Some(json!([{ "checkLoops": "none" }]))), + ("do{ }while(x);", Some(json!([{ "checkLoops": "all" }]))), + ("while(true);", Some(json!([{ "checkLoops": "allExceptWhileTrue" }]))), + ("while(true);", None), + ("while(a == b);", Some(json!([{ "checkLoops": "all" }]))), + ("for (let x = 0; x <= 10; x++) {};", Some(json!([{ "checkLoops": "all" }]))), + ("do{}while(true)", Some(json!([{ "checkLoops": false }]))), + // TODO: ("function* foo(){while(true){yield 'foo';}}", None), + // TODO: ("function* foo(){for(;true;){yield 'foo';}}", None), + // TODO: ("function* foo(){do{yield 'foo';}while(true)}", None), + // TODO: ("function* foo(){while (true) { while(true) {yield;}}}", None), + // TODO: ("function* foo() {for (; yield; ) {}}", None), + // TODO: ("function* foo() {for (; ; yield) {}}", None), + // TODO: ("function* foo() {while (true) {function* foo() {yield;}yield;}}", None), + // TODO: ("function* foo() { for (let x = yield; x < 10; x++) {yield;}yield;}", None), + // TODO: ("function* foo() { for (let x = yield; ; x++) { yield; }}", None), ]; let fail = vec![ - ("if(-2);", None), ("if(-2);", None), ("if(true);", None), ("if(1);", None), @@ -306,11 +377,13 @@ fn test() { ("if((a &&= null) && b);", None), ("if(false || (a &&= false));", None), ("if((a &&= false) || false);", None), + ("while(x = 1);", None), ("if(typeof x){}", None), ("if(typeof 'abc' === 'string'){}", None), ("if(a = typeof b){}", None), ("if(a, typeof b){}", None), ("if(typeof 'a' == 'string' || typeof 'b' == 'string'){}", None), + ("while(typeof x){}", None), ("if(1 || void x);", None), ("if(void x);", None), ("if(y = void x);", None), @@ -370,40 +443,46 @@ fn test() { ("`` ? 1 : 2;", None), ("`foo` ? 1 : 2;", None), ("`foo${bar}` ? 1 : 2;", None), - // TODO - // ("for(;true;);", None), - // ("for(;``;);", None), - // ("for(;`foo`;);", None), - // ("for(;`foo${bar}`;);", None), - // ("do{}while(true)", None), - // ("do{}while('1')", None), - // ("do{}while(0)", None), - // ("do{}while(t = -2)", None), - // ("do{}while(``)", None), - // ("do{}while(`foo`)", None), - // ("do{}while(`foo${bar}`)", None), - // ("while([]);", None), - // ("while(~!0);", None), - // ("while(x = 1);", None), - // ("while(function(){});", None), - // ("while(true);", None), - // ("while(1);", None), - // ("while(() => {});", None), - // ("while(`foo`);", None), - // ("while(``);", None), - // ("while(`${'foo'}`);", None), - // ("while(`${'foo' + 'bar'}`);", None), - // ("function* foo(){while(true){} yield 'foo';}", None), - // ("function* foo(){while(true){if (true) {yield 'foo';}}}", None), - // ("function* foo(){while(true){yield 'foo';} while(true) {}}", None), - // ("var a = function* foo(){while(true){} yield 'foo';}", None), - // ("while (true) { function* foo() {yield;}}", None), - // ("function* foo(){if (true) {yield 'foo';}}", None), - // ("function* foo() {for (let foo = yield; true;) {}}", None), - // ("function* foo() {for (foo = yield; true;) {}}", None), - // ("function foo() {while (true) {function* bar() {while (true) {yield;}}}}", None), - // ("function foo() {while (true) {const bar = function*() {while (true) {yield;}}}}", None), - // ("function* foo() { for (let foo = 1 + 2 + 3 + (yield); true; baz) {}}", None), + ("for(;true;);", None), + ("for(;``;);", None), + ("for(;`foo`;);", None), + ("for(;`foo${bar}`;);", None), + ("do{}while(true)", None), + ("do{}while('1')", None), + ("do{}while(0)", None), + ("do{}while(t = -2)", None), + ("do{}while(``)", None), + ("do{}while(`foo`)", None), + ("do{}while(`foo${bar}`)", None), + ("while([]);", None), + ("while(~!0);", None), + ("while(x = 1);", Some(json!([{ "checkLoops": "all" }]))), + ("while(function(){});", None), + ("while(true);", Some(json!([{ "checkLoops": "all" }]))), + ("while(1);", None), + ("while(() => {});", None), + ("while(`foo`);", None), + ("while(``);", None), + ("while(`${'foo'}`);", None), + ("while(`${'foo' + 'bar'}`);", None), + ("do{ }while(x = 1)", Some(json!([{ "checkLoops": "all" }]))), + ("for (;true;) {};", Some(json!([{ "checkLoops": "all" }]))), + // TODO: ("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": "all" }]))), + // TODO: ("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": true }]))), + // TODO: ("function* foo(){while(true){if (true) {yield 'foo';}}}",Some(json!([{ "checkLoops": "all" }])),), + // TODO: ("function* foo(){while(true){if (true) {yield 'foo';}}}",Some(json!([{ "checkLoops": true }])),), + // TODO: ("function* foo(){while(true){yield 'foo';} while(true) {}}", Some(json!([{ "checkLoops": "all" }])),), + // TODO: ("function* foo(){while(true){yield 'foo';} while(true) {}}",Some(json!([{ "checkLoops": true }])),), + // TODO: ("var a = function* foo(){while(true){} yield 'foo';}",Some(json!([{ "checkLoops": "all" }])),), + // TODO: ("var a = function* foo(){while(true){} yield 'foo';}",Some(json!([{ "checkLoops": true }])),), + // TODO: ("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": "all" }]))), + // TODO: ("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": true }]))), + // TODO: ("function* foo(){if (true) {yield 'foo';}}", None), + // TODO: ("function* foo() {for (let foo = yield; true;) {}}", None), + // TODO: ("function* foo() {for (foo = yield; true;) {}}", None), + // TODO: ("function foo() {while (true) {function* bar() {while (true) {yield;}}}}",Some(json!([{ "checkLoops": "all" }])),), + // TODO: ("function foo() {while (true) {const bar = function*() {while (true) {yield;}}}}",Some(json!([{ "checkLoops": "all" }])),), + // TODO: ("function* foo() { for (let foo = 1 + 2 + 3 + (yield); true; baz) {}}", None), ]; Tester::new(NoConstantCondition::NAME, NoConstantCondition::PLUGIN, pass, fail) diff --git a/crates/oxc_linter/src/snapshots/eslint_no_constant_condition.snap b/crates/oxc_linter/src/snapshots/eslint_no_constant_condition.snap index 0678248bdf0dd..9743c3248f7a1 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_constant_condition.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_constant_condition.snap @@ -8,13 +8,6 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Constant expression as a test condition is not allowed - ⚠ eslint(no-constant-condition): Unexpected constant condition - ╭─[no_constant_condition.tsx:1:4] - 1 │ if(-2); - · ── - ╰──── - help: Constant expression as a test condition is not allowed - ⚠ eslint(no-constant-condition): Unexpected constant condition ╭─[no_constant_condition.tsx:1:4] 1 │ if(true); @@ -365,6 +358,13 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Constant expression as a test condition is not allowed + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(x = 1); + · ───── + ╰──── + help: Constant expression as a test condition is not allowed + ⚠ eslint(no-constant-condition): Unexpected constant condition ╭─[no_constant_condition.tsx:1:4] 1 │ if(typeof x){} @@ -400,6 +400,13 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Constant expression as a test condition is not allowed + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(typeof x){} + · ──────── + ╰──── + help: Constant expression as a test condition is not allowed + ⚠ eslint(no-constant-condition): Unexpected constant condition ╭─[no_constant_condition.tsx:1:4] 1 │ if(1 || void x); @@ -770,3 +777,171 @@ source: crates/oxc_linter/src/tester.rs · ─────────── ╰──── help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:6] + 1 │ for(;true;); + · ──── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:6] + 1 │ for(;``;); + · ── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:6] + 1 │ for(;`foo`;); + · ───── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:6] + 1 │ for(;`foo${bar}`;); + · ─────────── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:11] + 1 │ do{}while(true) + · ──── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:11] + 1 │ do{}while('1') + · ─── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:11] + 1 │ do{}while(0) + · ─ + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:11] + 1 │ do{}while(t = -2) + · ────── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:11] + 1 │ do{}while(``) + · ── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:11] + 1 │ do{}while(`foo`) + · ───── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:11] + 1 │ do{}while(`foo${bar}`) + · ─────────── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while([]); + · ── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(~!0); + · ─── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(x = 1); + · ───── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(function(){}); + · ──────────── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(true); + · ──── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(1); + · ─ + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(() => {}); + · ──────── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(`foo`); + · ───── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(``); + · ── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(`${'foo'}`); + · ────────── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ while(`${'foo' + 'bar'}`); + · ────────────────── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:12] + 1 │ do{ }while(x = 1) + · ───── + ╰──── + help: Constant expression as a test condition is not allowed + + ⚠ eslint(no-constant-condition): Unexpected constant condition + ╭─[no_constant_condition.tsx:1:7] + 1 │ for (;true;) {}; + · ──── + ╰──── + help: Constant expression as a test condition is not allowed From 081a3f02129f44ed3ca1e3446d7ee884b59adcd9 Mon Sep 17 00:00:00 2001 From: Ulrich Stark Date: Mon, 12 May 2025 06:39:47 +0200 Subject: [PATCH 2/4] reduce diff --- .../src/rules/eslint/no_constant_condition.rs | 63 ++++++++++--------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs b/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs index 68d6ccb9d2709..c78f837c224da 100644 --- a/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs +++ b/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs @@ -282,11 +282,12 @@ fn test() { ("if (Boolean(a)) {}", None), ("if (Boolean(...args)) {}", None), ("if (foo.Boolean(1)) {}", None), - ("const undefined = 'lol'; if (undefined) {}", None), - ("function foo(Boolean) { if (Boolean(1)) {} }", None), - ("const Boolean = () => {}; if (Boolean(1)) {}", None), - // TODO: ("if (Boolean()) {}", None), - // TODO: ("if (undefined) {}", None), + // TODO + // ("const undefined = 'lol'; if (undefined) {}", None), + // ("function foo(Boolean) { if (Boolean(1)) {} }", None), + // ("const Boolean = () => {}; if (Boolean(1)) {}", None), + // ("if (Boolean()) {}", None), + // ("if (undefined) {}", None), ("q > 0 ? 1 : 2;", None), ("`${a}` === a ? 1 : 2", None), ("`foo${a}` === a ? 1 : 2", None), @@ -314,15 +315,16 @@ fn test() { ("while(a == b);", Some(json!([{ "checkLoops": "all" }]))), ("for (let x = 0; x <= 10; x++) {};", Some(json!([{ "checkLoops": "all" }]))), ("do{}while(true)", Some(json!([{ "checkLoops": false }]))), - // TODO: ("function* foo(){while(true){yield 'foo';}}", None), - // TODO: ("function* foo(){for(;true;){yield 'foo';}}", None), - // TODO: ("function* foo(){do{yield 'foo';}while(true)}", None), - // TODO: ("function* foo(){while (true) { while(true) {yield;}}}", None), - // TODO: ("function* foo() {for (; yield; ) {}}", None), - // TODO: ("function* foo() {for (; ; yield) {}}", None), - // TODO: ("function* foo() {while (true) {function* foo() {yield;}yield;}}", None), - // TODO: ("function* foo() { for (let x = yield; x < 10; x++) {yield;}yield;}", None), - // TODO: ("function* foo() { for (let x = yield; ; x++) { yield; }}", None), + // TODO + // ("function* foo(){while(true){yield 'foo';}}", None), + // ("function* foo(){for(;true;){yield 'foo';}}", None), + // ("function* foo(){do{yield 'foo';}while(true)}", None), + // ("function* foo(){while (true) { while(true) {yield;}}}", None), + // ("function* foo() {for (; yield; ) {}}", None), + // ("function* foo() {for (; ; yield) {}}", None), + // ("function* foo() {while (true) {function* foo() {yield;}yield;}}", None), + // ("function* foo() { for (let x = yield; x < 10; x++) {yield;}yield;}", None), + // ("function* foo() { for (let x = yield; ; x++) { yield; }}", None), ]; let fail = vec![ @@ -467,22 +469,23 @@ fn test() { ("while(`${'foo' + 'bar'}`);", None), ("do{ }while(x = 1)", Some(json!([{ "checkLoops": "all" }]))), ("for (;true;) {};", Some(json!([{ "checkLoops": "all" }]))), - // TODO: ("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": "all" }]))), - // TODO: ("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": true }]))), - // TODO: ("function* foo(){while(true){if (true) {yield 'foo';}}}",Some(json!([{ "checkLoops": "all" }])),), - // TODO: ("function* foo(){while(true){if (true) {yield 'foo';}}}",Some(json!([{ "checkLoops": true }])),), - // TODO: ("function* foo(){while(true){yield 'foo';} while(true) {}}", Some(json!([{ "checkLoops": "all" }])),), - // TODO: ("function* foo(){while(true){yield 'foo';} while(true) {}}",Some(json!([{ "checkLoops": true }])),), - // TODO: ("var a = function* foo(){while(true){} yield 'foo';}",Some(json!([{ "checkLoops": "all" }])),), - // TODO: ("var a = function* foo(){while(true){} yield 'foo';}",Some(json!([{ "checkLoops": true }])),), - // TODO: ("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": "all" }]))), - // TODO: ("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": true }]))), - // TODO: ("function* foo(){if (true) {yield 'foo';}}", None), - // TODO: ("function* foo() {for (let foo = yield; true;) {}}", None), - // TODO: ("function* foo() {for (foo = yield; true;) {}}", None), - // TODO: ("function foo() {while (true) {function* bar() {while (true) {yield;}}}}",Some(json!([{ "checkLoops": "all" }])),), - // TODO: ("function foo() {while (true) {const bar = function*() {while (true) {yield;}}}}",Some(json!([{ "checkLoops": "all" }])),), - // TODO: ("function* foo() { for (let foo = 1 + 2 + 3 + (yield); true; baz) {}}", None), + // TODO + // ("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": "all" }]))), + // ("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": true }]))), + // ("function* foo(){while(true){if (true) {yield 'foo';}}}",Some(json!([{ "checkLoops": "all" }])),), + // ("function* foo(){while(true){if (true) {yield 'foo';}}}",Some(json!([{ "checkLoops": true }])),), + // ("function* foo(){while(true){yield 'foo';} while(true) {}}", Some(json!([{ "checkLoops": "all" }])),), + // ("function* foo(){while(true){yield 'foo';} while(true) {}}",Some(json!([{ "checkLoops": true }])),), + // ("var a = function* foo(){while(true){} yield 'foo';}",Some(json!([{ "checkLoops": "all" }])),), + // ("var a = function* foo(){while(true){} yield 'foo';}",Some(json!([{ "checkLoops": true }])),), + // ("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": "all" }]))), + // ("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": true }]))), + // ("function* foo(){if (true) {yield 'foo';}}", None), + // ("function* foo() {for (let foo = yield; true;) {}}", None), + // ("function* foo() {for (foo = yield; true;) {}}", None), + // ("function foo() {while (true) {function* bar() {while (true) {yield;}}}}",Some(json!([{ "checkLoops": "all" }])),), + // ("function foo() {while (true) {const bar = function*() {while (true) {yield;}}}}",Some(json!([{ "checkLoops": "all" }])),), + // ("function* foo() { for (let foo = 1 + 2 + 3 + (yield); true; baz) {}}", None), ]; Tester::new(NoConstantCondition::NAME, NoConstantCondition::PLUGIN, pass, fail) From be56e36326473e6ae07596100b42b3a0e8a5de53 Mon Sep 17 00:00:00 2001 From: Ulrich Stark Date: Mon, 12 May 2025 17:54:42 +0200 Subject: [PATCH 3/4] simplify check --- .../src/rules/eslint/no_constant_condition.rs | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs b/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs index c78f837c224da..6af61467a1686 100644 --- a/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs +++ b/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs @@ -131,23 +131,13 @@ impl Rule for NoConstantCondition { impl NoConstantCondition { fn check_loop<'a>(&self, ctx: &LintContext<'a>, test: &'a Expression<'_>, is_while: bool) { - let return_early = match self.check_loops { - CheckLoops::None => true, - CheckLoops::AllExceptWhileTrue => { - if is_while { - match test { - Expression::BooleanLiteral(bool) => bool.value, - _ => false, - } - } else { - false - } - } - CheckLoops::All => false, - }; - - if return_early { - return; + match self.check_loops { + CheckLoops::None => return, + CheckLoops::AllExceptWhileTrue if is_while => match test { + Expression::BooleanLiteral(bool) if bool.value => return, + _ => {} + }, + _ => {} } check(ctx, test); From 9232262fb40b7e0fce210675154ce04b00a52705 Mon Sep 17 00:00:00 2001 From: Ulrich Stark Date: Mon, 12 May 2025 18:18:35 +0200 Subject: [PATCH 4/4] update rule docs --- .../src/rules/eslint/no_constant_condition.rs | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs b/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs index 6af61467a1686..0a668695a7ae2 100644 --- a/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs +++ b/crates/oxc_linter/src/rules/eslint/no_constant_condition.rs @@ -61,37 +61,46 @@ declare_oxc_lint!( /// - `if`, `for`, `while`, or `do...while` statement /// - `?`: ternary expression /// - /// - /// ### Example + /// ### Examples /// /// Examples of **incorrect** code for this rule: /// ```js /// if (false) { - /// doSomethingUnfinished(); + /// doSomethingUnfinished(); /// } /// /// if (new Boolean(x)) { - /// doSomethingAlways(); + /// doSomethingAlways(); /// } /// if (x ||= true) { - /// doSomethingAlways(); + /// doSomethingAlways(); /// } /// /// do { - /// doSomethingForever(); + /// doSomethingForever(); /// } while (x = -1); /// ``` /// /// Examples of **correct** code for this rule: /// ```js /// if (x === 0) { - /// doSomething(); + /// doSomething(); /// } /// /// while (typeof x === "undefined") { - /// doSomething(); + /// doSomething(); /// } /// ``` + /// + /// ### Options + /// + /// #### checkLoops + /// + /// `{ type: "all" | "allExceptWhileTrue" | "none" | boolean, default: "allExceptWhileTrue" }` + /// + /// - `"all"` or `true` disallows constant expressions in loops + /// - `"allExceptWhileTrue"` disallows constant expressions in loops except while loops with expression `true` + /// - `"none"` or `false` allows constant expressions in loops NoConstantCondition, eslint, correctness