Skip to content

Commit d7230b0

Browse files
camc314camc314
committed
fix(linter/no-constant-condition): handle generator yields (#22046)
Updates no-constant-condition to match ESLint's generator-loop handling by suppressing default loop diagnostics when a same-generator yield occurs before the loop exits.
1 parent e8dbc56 commit d7230b0

2 files changed

Lines changed: 316 additions & 41 deletions

File tree

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

Lines changed: 140 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
1-
use oxc_ast::{AstKind, ast::Expression};
1+
use schemars::JsonSchema;
2+
use serde::{Deserialize, Serialize};
3+
4+
use oxc_ast::{
5+
AstKind,
6+
ast::{ArrowFunctionExpression, Expression, Function, YieldExpression},
7+
};
8+
use oxc_ast_visit::Visit;
29
use oxc_diagnostics::OxcDiagnostic;
310
use oxc_macros::declare_oxc_lint;
411
use oxc_span::{GetSpan, Span};
5-
use schemars::JsonSchema;
6-
use serde::{Deserialize, Serialize};
12+
use oxc_syntax::{node::NodeId, scope::ScopeFlags};
713

814
use crate::{
915
AstNode,
@@ -133,24 +139,49 @@ impl Rule for NoConstantCondition {
133139
match node.kind() {
134140
AstKind::IfStatement(if_stmt) => check(ctx, &if_stmt.test),
135141
AstKind::ConditionalExpression(condition_expr) => check(ctx, &condition_expr.test),
136-
AstKind::WhileStatement(while_stmt) => self.check_loop(ctx, &while_stmt.test, true),
142+
AstKind::WhileStatement(while_stmt) => {
143+
self.check_loop(ctx, &while_stmt.test, true, || {
144+
has_yield_before_loop_exit_in_same_generator(ctx, node.id(), None, |finder| {
145+
finder.visit_while_statement(while_stmt);
146+
})
147+
});
148+
}
137149
AstKind::DoWhileStatement(do_while_stmt) => {
138-
self.check_loop(ctx, &do_while_stmt.test, false);
150+
self.check_loop(ctx, &do_while_stmt.test, false, || {
151+
has_yield_before_loop_exit_in_same_generator(ctx, node.id(), None, |finder| {
152+
finder.visit_do_while_statement(do_while_stmt);
153+
})
154+
});
139155
}
140156
AstKind::ForStatement(for_stmt) => {
141157
let Some(test) = &for_stmt.test else {
142158
return;
143159
};
144160

145-
self.check_loop(ctx, test, false);
161+
self.check_loop(ctx, test, false, || {
162+
has_yield_before_loop_exit_in_same_generator(
163+
ctx,
164+
node.id(),
165+
Some(test.span()),
166+
|finder| {
167+
finder.visit_for_statement(for_stmt);
168+
},
169+
)
170+
});
146171
}
147172
_ => {}
148173
}
149174
}
150175
}
151176

152177
impl NoConstantCondition {
153-
fn check_loop<'a>(&self, ctx: &LintContext<'a>, test: &'a Expression<'_>, is_while: bool) {
178+
fn check_loop<'a>(
179+
&self,
180+
ctx: &LintContext<'a>,
181+
test: &'a Expression<'_>,
182+
is_while: bool,
183+
has_yield_before_loop_exit: impl FnOnce() -> bool,
184+
) {
154185
match self.check_loops {
155186
CheckLoops::None => return,
156187
CheckLoops::AllExceptWhileTrue if is_while => match test {
@@ -160,7 +191,15 @@ impl NoConstantCondition {
160191
_ => {}
161192
}
162193

163-
check(ctx, test);
194+
if !test.is_constant(true, ctx) {
195+
return;
196+
}
197+
198+
if self.check_loops == CheckLoops::AllExceptWhileTrue && has_yield_before_loop_exit() {
199+
return;
200+
}
201+
202+
ctx.diagnostic(no_constant_condition_diagnostic(test.span()));
164203
}
165204
}
166205

@@ -170,6 +209,46 @@ fn check<'a>(ctx: &LintContext<'a>, test: &'a Expression<'_>) {
170209
}
171210
}
172211

212+
fn has_yield_before_loop_exit_in_same_generator(
213+
ctx: &LintContext<'_>,
214+
loop_node_id: NodeId,
215+
after_span: Option<Span>,
216+
visit_loop: impl FnOnce(&mut YieldBeforeLoopExitFinder),
217+
) -> bool {
218+
if !ctx
219+
.nodes()
220+
.ancestors_enumerated(loop_node_id)
221+
.find_map(|(_id, node)| match node.kind() {
222+
AstKind::Function(function) => Some(function.generator),
223+
_ => None,
224+
})
225+
.unwrap_or(false)
226+
{
227+
return false;
228+
}
229+
230+
let mut finder = YieldBeforeLoopExitFinder { found: false, after_span };
231+
visit_loop(&mut finder);
232+
finder.found
233+
}
234+
235+
struct YieldBeforeLoopExitFinder {
236+
found: bool,
237+
after_span: Option<Span>,
238+
}
239+
240+
impl<'a> Visit<'a> for YieldBeforeLoopExitFinder {
241+
fn visit_yield_expression(&mut self, expr: &YieldExpression<'a>) {
242+
if self.after_span.is_none_or(|after_span| expr.span.start > after_span.start) {
243+
self.found = true;
244+
}
245+
}
246+
247+
fn visit_function(&mut self, _function: &Function<'a>, _flags: ScopeFlags) {}
248+
249+
fn visit_arrow_function_expression(&mut self, _expr: &ArrowFunctionExpression<'a>) {}
250+
}
251+
173252
#[test]
174253
fn test() {
175254
use serde_json::json;
@@ -292,12 +371,9 @@ fn test() {
292371
("if (Boolean(a)) {}", None),
293372
("if (Boolean(...args)) {}", None),
294373
("if (foo.Boolean(1)) {}", None),
295-
// TODO
296-
// ("const undefined = 'lol'; if (undefined) {}", None),
297-
// ("function foo(Boolean) { if (Boolean(1)) {} }", None),
298-
// ("const Boolean = () => {}; if (Boolean(1)) {}", None),
299-
// ("if (Boolean()) {}", None),
300-
// ("if (undefined) {}", None),
374+
("const undefined = 'lol'; if (undefined) {}", None),
375+
("function foo(Boolean) { if (Boolean(1)) {} }", None),
376+
("const Boolean = () => {}; if (Boolean(1)) {}", None),
301377
("q > 0 ? 1 : 2;", None),
302378
("`${a}` === a ? 1 : 2", None),
303379
("`foo${a}` === a ? 1 : 2", None),
@@ -325,16 +401,16 @@ fn test() {
325401
("while(a == b);", Some(json!([{ "checkLoops": "all" }]))),
326402
("for (let x = 0; x <= 10; x++) {};", Some(json!([{ "checkLoops": "all" }]))),
327403
("do{}while(true)", Some(json!([{ "checkLoops": false }]))),
328-
// TODO
329-
// ("function* foo(){while(true){yield 'foo';}}", None),
330-
// ("function* foo(){for(;true;){yield 'foo';}}", None),
331-
// ("function* foo(){do{yield 'foo';}while(true)}", None),
332-
// ("function* foo(){while (true) { while(true) {yield;}}}", None),
333-
// ("function* foo() {for (; yield; ) {}}", None),
334-
// ("function* foo() {for (; ; yield) {}}", None),
335-
// ("function* foo() {while (true) {function* foo() {yield;}yield;}}", None),
336-
// ("function* foo() { for (let x = yield; x < 10; x++) {yield;}yield;}", None),
337-
// ("function* foo() { for (let x = yield; ; x++) { yield; }}", None),
404+
("function* foo(){while(true){yield 'foo';}}", None),
405+
("function* foo(){for(;true;){yield 'foo';}}", None),
406+
("function* foo(){do{yield 'foo';}while(true)}", None),
407+
("function* foo(){while (true) { while(true) {yield;}}}", None),
408+
("function* foo() {for (; yield; ) {}}", None),
409+
("function* foo() {for (; ; yield) {}}", None),
410+
("function* foo() {for (; true; yield) {}}", None),
411+
("function* foo() {while (true) {function* foo() {yield;}yield;}}", None),
412+
("function* foo() { for (let x = yield; x < 10; x++) {yield;}yield;}", None),
413+
("function* foo() { for (let x = yield; ; x++) { yield; }}", None),
338414
];
339415

340416
let fail = vec![
@@ -479,23 +555,46 @@ fn test() {
479555
("while(`${'foo' + 'bar'}`);", None),
480556
("do{ }while(x = 1)", Some(json!([{ "checkLoops": "all" }]))),
481557
("for (;true;) {};", Some(json!([{ "checkLoops": "all" }]))),
482-
// TODO
483-
// ("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": "all" }]))),
484-
// ("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": true }]))),
485-
// ("function* foo(){while(true){if (true) {yield 'foo';}}}",Some(json!([{ "checkLoops": "all" }])),),
486-
// ("function* foo(){while(true){if (true) {yield 'foo';}}}",Some(json!([{ "checkLoops": true }])),),
487-
// ("function* foo(){while(true){yield 'foo';} while(true) {}}", Some(json!([{ "checkLoops": "all" }])),),
488-
// ("function* foo(){while(true){yield 'foo';} while(true) {}}",Some(json!([{ "checkLoops": true }])),),
489-
// ("var a = function* foo(){while(true){} yield 'foo';}",Some(json!([{ "checkLoops": "all" }])),),
490-
// ("var a = function* foo(){while(true){} yield 'foo';}",Some(json!([{ "checkLoops": true }])),),
491-
// ("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": "all" }]))),
492-
// ("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": true }]))),
493-
// ("function* foo(){if (true) {yield 'foo';}}", None),
494-
// ("function* foo() {for (let foo = yield; true;) {}}", None),
495-
// ("function* foo() {for (foo = yield; true;) {}}", None),
496-
// ("function foo() {while (true) {function* bar() {while (true) {yield;}}}}",Some(json!([{ "checkLoops": "all" }])),),
497-
// ("function foo() {while (true) {const bar = function*() {while (true) {yield;}}}}",Some(json!([{ "checkLoops": "all" }])),),
498-
// ("function* foo() { for (let foo = 1 + 2 + 3 + (yield); true; baz) {}}", None),
558+
("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": "all" }]))),
559+
("function* foo(){while(true){} yield 'foo';}", Some(json!([{ "checkLoops": true }]))),
560+
(
561+
"function* foo(){while(true){if (true) {yield 'foo';}}}",
562+
Some(json!([{ "checkLoops": "all" }])),
563+
),
564+
(
565+
"function* foo(){while(true){if (true) {yield 'foo';}}}",
566+
Some(json!([{ "checkLoops": true }])),
567+
),
568+
(
569+
"function* foo(){while(true){yield 'foo';} while(true) {}}",
570+
Some(json!([{ "checkLoops": "all" }])),
571+
),
572+
(
573+
"function* foo(){while(true){yield 'foo';} while(true) {}}",
574+
Some(json!([{ "checkLoops": true }])),
575+
),
576+
(
577+
"var a = function* foo(){while(true){} yield 'foo';}",
578+
Some(json!([{ "checkLoops": "all" }])),
579+
),
580+
(
581+
"var a = function* foo(){while(true){} yield 'foo';}",
582+
Some(json!([{ "checkLoops": true }])),
583+
),
584+
("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": "all" }]))),
585+
("while (true) { function* foo() {yield;}}", Some(json!([{ "checkLoops": true }]))),
586+
("function* foo(){if (true) {yield 'foo';}}", None),
587+
("function* foo() {for (let foo = yield; true;) {}}", None),
588+
("function* foo() {for (foo = yield; true;) {}}", None),
589+
(
590+
"function foo() {while (true) {function* bar() {while (true) {yield;}}}}",
591+
Some(json!([{ "checkLoops": "all" }])),
592+
),
593+
(
594+
"function foo() {while (true) {const bar = function*() {while (true) {yield;}}}}",
595+
Some(json!([{ "checkLoops": "all" }])),
596+
),
597+
("function* foo() { for (let foo = 1 + 2 + 3 + (yield); true; baz) {}}", None),
499598
];
500599

501600
Tester::new(NoConstantCondition::NAME, NoConstantCondition::PLUGIN, pass, fail)

0 commit comments

Comments
 (0)