From b3f3d34a81eb5bf364ce128d08430504228e831a Mon Sep 17 00:00:00 2001 From: Limerio <44976231+Limerio@users.noreply.github.com> Date: Thu, 12 Feb 2026 22:25:49 +0100 Subject: [PATCH 1/4] feat(linter): add no-unmodified-loop-condition rule Introduced a new ESLint rule to disallow references in loop conditions that are never modified within the loop, preventing potential infinite loops or logic bugs. This enhancement improves code quality by enforcing better practices in loop constructs. I made a mistake before with my rebase sorry. ^^' --- .../src/generated/rule_runner_impls.rs | 9 + crates/oxc_linter/src/generated/rules_enum.rs | 32 +- crates/oxc_linter/src/rules.rs | 1 + .../eslint/no_unmodified_loop_condition.rs | 392 ++++++++++++++++++ .../eslint_no_unmodified_loop_condition.snap | 81 ++++ 5 files changed, 514 insertions(+), 1 deletion(-) create mode 100644 crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs create mode 100644 crates/oxc_linter/src/snapshots/eslint_no_unmodified_loop_condition.snap diff --git a/crates/oxc_linter/src/generated/rule_runner_impls.rs b/crates/oxc_linter/src/generated/rule_runner_impls.rs index 73cf952d00100..c8b1d36ffb26b 100644 --- a/crates/oxc_linter/src/generated/rule_runner_impls.rs +++ b/crates/oxc_linter/src/generated/rule_runner_impls.rs @@ -1005,6 +1005,15 @@ impl RuleRunner for crate::rules::eslint::no_unexpected_multiline::NoUnexpectedM const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; } +impl RuleRunner for crate::rules::eslint::no_unmodified_loop_condition::NoUnmodifiedLoopCondition { + const NODE_TYPES: Option<&AstTypesBitset> = Some(&AstTypesBitset::from_types(&[ + AstType::DoWhileStatement, + AstType::ForStatement, + AstType::WhileStatement, + ])); + const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::Run; +} + impl RuleRunner for crate::rules::eslint::no_unneeded_ternary::NoUnneededTernary { const NODE_TYPES: Option<&AstTypesBitset> = Some(&AstTypesBitset::from_types(&[AstType::ConditionalExpression])); diff --git a/crates/oxc_linter/src/generated/rules_enum.rs b/crates/oxc_linter/src/generated/rules_enum.rs index ae6f0592a9317..782fd24a48046 100644 --- a/crates/oxc_linter/src/generated/rules_enum.rs +++ b/crates/oxc_linter/src/generated/rules_enum.rs @@ -129,6 +129,7 @@ pub use crate::rules::eslint::no_unassigned_vars::NoUnassignedVars as EslintNoUn pub use crate::rules::eslint::no_undef::NoUndef as EslintNoUndef; pub use crate::rules::eslint::no_undefined::NoUndefined as EslintNoUndefined; pub use crate::rules::eslint::no_unexpected_multiline::NoUnexpectedMultiline as EslintNoUnexpectedMultiline; +pub use crate::rules::eslint::no_unmodified_loop_condition::NoUnmodifiedLoopCondition as EslintNoUnmodifiedLoopCondition; pub use crate::rules::eslint::no_unneeded_ternary::NoUnneededTernary as EslintNoUnneededTernary; pub use crate::rules::eslint::no_unreachable::NoUnreachable as EslintNoUnreachable; pub use crate::rules::eslint::no_unsafe_finally::NoUnsafeFinally as EslintNoUnsafeFinally; @@ -842,6 +843,7 @@ pub enum RuleEnum { EslintNoUndef(EslintNoUndef), EslintNoUndefined(EslintNoUndefined), EslintNoUnexpectedMultiline(EslintNoUnexpectedMultiline), + EslintNoUnmodifiedLoopCondition(EslintNoUnmodifiedLoopCondition), EslintNoUnneededTernary(EslintNoUnneededTernary), EslintNoUnreachable(EslintNoUnreachable), EslintNoUnsafeFinally(EslintNoUnsafeFinally), @@ -1519,7 +1521,8 @@ const ESLINT_NO_UNASSIGNED_VARS_ID: usize = ESLINT_NO_THROW_LITERAL_ID + 1usize; const ESLINT_NO_UNDEF_ID: usize = ESLINT_NO_UNASSIGNED_VARS_ID + 1usize; const ESLINT_NO_UNDEFINED_ID: usize = ESLINT_NO_UNDEF_ID + 1usize; const ESLINT_NO_UNEXPECTED_MULTILINE_ID: usize = ESLINT_NO_UNDEFINED_ID + 1usize; -const ESLINT_NO_UNNEEDED_TERNARY_ID: usize = ESLINT_NO_UNEXPECTED_MULTILINE_ID + 1usize; +const ESLINT_NO_UNMODIFIED_LOOP_CONDITION_ID: usize = ESLINT_NO_UNEXPECTED_MULTILINE_ID + 1usize; +const ESLINT_NO_UNNEEDED_TERNARY_ID: usize = ESLINT_NO_UNMODIFIED_LOOP_CONDITION_ID + 1usize; const ESLINT_NO_UNREACHABLE_ID: usize = ESLINT_NO_UNNEEDED_TERNARY_ID + 1usize; const ESLINT_NO_UNSAFE_FINALLY_ID: usize = ESLINT_NO_UNREACHABLE_ID + 1usize; const ESLINT_NO_UNSAFE_NEGATION_ID: usize = ESLINT_NO_UNSAFE_FINALLY_ID + 1usize; @@ -2276,6 +2279,7 @@ impl RuleEnum { Self::EslintNoUndef(_) => ESLINT_NO_UNDEF_ID, Self::EslintNoUndefined(_) => ESLINT_NO_UNDEFINED_ID, Self::EslintNoUnexpectedMultiline(_) => ESLINT_NO_UNEXPECTED_MULTILINE_ID, + Self::EslintNoUnmodifiedLoopCondition(_) => ESLINT_NO_UNMODIFIED_LOOP_CONDITION_ID, Self::EslintNoUnneededTernary(_) => ESLINT_NO_UNNEEDED_TERNARY_ID, Self::EslintNoUnreachable(_) => ESLINT_NO_UNREACHABLE_ID, Self::EslintNoUnsafeFinally(_) => ESLINT_NO_UNSAFE_FINALLY_ID, @@ -3048,6 +3052,7 @@ impl RuleEnum { Self::EslintNoUndef(_) => EslintNoUndef::NAME, Self::EslintNoUndefined(_) => EslintNoUndefined::NAME, Self::EslintNoUnexpectedMultiline(_) => EslintNoUnexpectedMultiline::NAME, + Self::EslintNoUnmodifiedLoopCondition(_) => EslintNoUnmodifiedLoopCondition::NAME, Self::EslintNoUnneededTernary(_) => EslintNoUnneededTernary::NAME, Self::EslintNoUnreachable(_) => EslintNoUnreachable::NAME, Self::EslintNoUnsafeFinally(_) => EslintNoUnsafeFinally::NAME, @@ -3812,6 +3817,7 @@ impl RuleEnum { Self::EslintNoUndef(_) => EslintNoUndef::CATEGORY, Self::EslintNoUndefined(_) => EslintNoUndefined::CATEGORY, Self::EslintNoUnexpectedMultiline(_) => EslintNoUnexpectedMultiline::CATEGORY, + Self::EslintNoUnmodifiedLoopCondition(_) => EslintNoUnmodifiedLoopCondition::CATEGORY, Self::EslintNoUnneededTernary(_) => EslintNoUnneededTernary::CATEGORY, Self::EslintNoUnreachable(_) => EslintNoUnreachable::CATEGORY, Self::EslintNoUnsafeFinally(_) => EslintNoUnsafeFinally::CATEGORY, @@ -4619,6 +4625,7 @@ impl RuleEnum { Self::EslintNoUndef(_) => EslintNoUndef::FIX, Self::EslintNoUndefined(_) => EslintNoUndefined::FIX, Self::EslintNoUnexpectedMultiline(_) => EslintNoUnexpectedMultiline::FIX, + Self::EslintNoUnmodifiedLoopCondition(_) => EslintNoUnmodifiedLoopCondition::FIX, Self::EslintNoUnneededTernary(_) => EslintNoUnneededTernary::FIX, Self::EslintNoUnreachable(_) => EslintNoUnreachable::FIX, Self::EslintNoUnsafeFinally(_) => EslintNoUnsafeFinally::FIX, @@ -5402,6 +5409,9 @@ impl RuleEnum { Self::EslintNoUndef(_) => EslintNoUndef::documentation(), Self::EslintNoUndefined(_) => EslintNoUndefined::documentation(), Self::EslintNoUnexpectedMultiline(_) => EslintNoUnexpectedMultiline::documentation(), + Self::EslintNoUnmodifiedLoopCondition(_) => { + EslintNoUnmodifiedLoopCondition::documentation() + } Self::EslintNoUnneededTernary(_) => EslintNoUnneededTernary::documentation(), Self::EslintNoUnreachable(_) => EslintNoUnreachable::documentation(), Self::EslintNoUnsafeFinally(_) => EslintNoUnsafeFinally::documentation(), @@ -6560,6 +6570,10 @@ impl RuleEnum { EslintNoUnexpectedMultiline::config_schema(generator) .or_else(|| EslintNoUnexpectedMultiline::schema(generator)) } + Self::EslintNoUnmodifiedLoopCondition(_) => { + EslintNoUnmodifiedLoopCondition::config_schema(generator) + .or_else(|| EslintNoUnmodifiedLoopCondition::schema(generator)) + } Self::EslintNoUnneededTernary(_) => EslintNoUnneededTernary::config_schema(generator) .or_else(|| EslintNoUnneededTernary::schema(generator)), Self::EslintNoUnreachable(_) => EslintNoUnreachable::config_schema(generator) @@ -8233,6 +8247,7 @@ impl RuleEnum { Self::EslintNoUndef(_) => "eslint", Self::EslintNoUndefined(_) => "eslint", Self::EslintNoUnexpectedMultiline(_) => "eslint", + Self::EslintNoUnmodifiedLoopCondition(_) => "eslint", Self::EslintNoUnneededTernary(_) => "eslint", Self::EslintNoUnreachable(_) => "eslint", Self::EslintNoUnsafeFinally(_) => "eslint", @@ -9214,6 +9229,9 @@ impl RuleEnum { Self::EslintNoUnexpectedMultiline(_) => Ok(Self::EslintNoUnexpectedMultiline( EslintNoUnexpectedMultiline::from_configuration(value)?, )), + Self::EslintNoUnmodifiedLoopCondition(_) => Ok(Self::EslintNoUnmodifiedLoopCondition( + EslintNoUnmodifiedLoopCondition::from_configuration(value)?, + )), Self::EslintNoUnneededTernary(_) => Ok(Self::EslintNoUnneededTernary( EslintNoUnneededTernary::from_configuration(value)?, )), @@ -11064,6 +11082,7 @@ impl RuleEnum { Self::EslintNoUndef(rule) => rule.to_configuration(), Self::EslintNoUndefined(rule) => rule.to_configuration(), Self::EslintNoUnexpectedMultiline(rule) => rule.to_configuration(), + Self::EslintNoUnmodifiedLoopCondition(rule) => rule.to_configuration(), Self::EslintNoUnneededTernary(rule) => rule.to_configuration(), Self::EslintNoUnreachable(rule) => rule.to_configuration(), Self::EslintNoUnsafeFinally(rule) => rule.to_configuration(), @@ -11744,6 +11763,7 @@ impl RuleEnum { Self::EslintNoUndef(rule) => rule.run(node, ctx), Self::EslintNoUndefined(rule) => rule.run(node, ctx), Self::EslintNoUnexpectedMultiline(rule) => rule.run(node, ctx), + Self::EslintNoUnmodifiedLoopCondition(rule) => rule.run(node, ctx), Self::EslintNoUnneededTernary(rule) => rule.run(node, ctx), Self::EslintNoUnreachable(rule) => rule.run(node, ctx), Self::EslintNoUnsafeFinally(rule) => rule.run(node, ctx), @@ -12420,6 +12440,7 @@ impl RuleEnum { Self::EslintNoUndef(rule) => rule.run_once(ctx), Self::EslintNoUndefined(rule) => rule.run_once(ctx), Self::EslintNoUnexpectedMultiline(rule) => rule.run_once(ctx), + Self::EslintNoUnmodifiedLoopCondition(rule) => rule.run_once(ctx), Self::EslintNoUnneededTernary(rule) => rule.run_once(ctx), Self::EslintNoUnreachable(rule) => rule.run_once(ctx), Self::EslintNoUnsafeFinally(rule) => rule.run_once(ctx), @@ -13100,6 +13121,7 @@ impl RuleEnum { Self::EslintNoUndef(rule) => rule.run_on_jest_node(jest_node, ctx), Self::EslintNoUndefined(rule) => rule.run_on_jest_node(jest_node, ctx), Self::EslintNoUnexpectedMultiline(rule) => rule.run_on_jest_node(jest_node, ctx), + Self::EslintNoUnmodifiedLoopCondition(rule) => rule.run_on_jest_node(jest_node, ctx), Self::EslintNoUnneededTernary(rule) => rule.run_on_jest_node(jest_node, ctx), Self::EslintNoUnreachable(rule) => rule.run_on_jest_node(jest_node, ctx), Self::EslintNoUnsafeFinally(rule) => rule.run_on_jest_node(jest_node, ctx), @@ -13862,6 +13884,7 @@ impl RuleEnum { Self::EslintNoUndef(rule) => rule.should_run(ctx), Self::EslintNoUndefined(rule) => rule.should_run(ctx), Self::EslintNoUnexpectedMultiline(rule) => rule.should_run(ctx), + Self::EslintNoUnmodifiedLoopCondition(rule) => rule.should_run(ctx), Self::EslintNoUnneededTernary(rule) => rule.should_run(ctx), Self::EslintNoUnreachable(rule) => rule.should_run(ctx), Self::EslintNoUnsafeFinally(rule) => rule.should_run(ctx), @@ -14558,6 +14581,9 @@ impl RuleEnum { Self::EslintNoUndef(_) => EslintNoUndef::IS_TSGOLINT_RULE, Self::EslintNoUndefined(_) => EslintNoUndefined::IS_TSGOLINT_RULE, Self::EslintNoUnexpectedMultiline(_) => EslintNoUnexpectedMultiline::IS_TSGOLINT_RULE, + Self::EslintNoUnmodifiedLoopCondition(_) => { + EslintNoUnmodifiedLoopCondition::IS_TSGOLINT_RULE + } Self::EslintNoUnneededTernary(_) => EslintNoUnneededTernary::IS_TSGOLINT_RULE, Self::EslintNoUnreachable(_) => EslintNoUnreachable::IS_TSGOLINT_RULE, Self::EslintNoUnsafeFinally(_) => EslintNoUnsafeFinally::IS_TSGOLINT_RULE, @@ -15493,6 +15519,7 @@ impl RuleEnum { Self::EslintNoUndef(_) => EslintNoUndef::HAS_CONFIG, Self::EslintNoUndefined(_) => EslintNoUndefined::HAS_CONFIG, Self::EslintNoUnexpectedMultiline(_) => EslintNoUnexpectedMultiline::HAS_CONFIG, + Self::EslintNoUnmodifiedLoopCondition(_) => EslintNoUnmodifiedLoopCondition::HAS_CONFIG, Self::EslintNoUnneededTernary(_) => EslintNoUnneededTernary::HAS_CONFIG, Self::EslintNoUnreachable(_) => EslintNoUnreachable::HAS_CONFIG, Self::EslintNoUnsafeFinally(_) => EslintNoUnsafeFinally::HAS_CONFIG, @@ -16319,6 +16346,7 @@ impl RuleEnum { Self::EslintNoUndef(rule) => rule.types_info(), Self::EslintNoUndefined(rule) => rule.types_info(), Self::EslintNoUnexpectedMultiline(rule) => rule.types_info(), + Self::EslintNoUnmodifiedLoopCondition(rule) => rule.types_info(), Self::EslintNoUnneededTernary(rule) => rule.types_info(), Self::EslintNoUnreachable(rule) => rule.types_info(), Self::EslintNoUnsafeFinally(rule) => rule.types_info(), @@ -16995,6 +17023,7 @@ impl RuleEnum { Self::EslintNoUndef(rule) => rule.run_info(), Self::EslintNoUndefined(rule) => rule.run_info(), Self::EslintNoUnexpectedMultiline(rule) => rule.run_info(), + Self::EslintNoUnmodifiedLoopCondition(rule) => rule.run_info(), Self::EslintNoUnneededTernary(rule) => rule.run_info(), Self::EslintNoUnreachable(rule) => rule.run_info(), Self::EslintNoUnsafeFinally(rule) => rule.run_info(), @@ -17693,6 +17722,7 @@ pub static RULES: std::sync::LazyLock> = std::sync::LazyLock::new( RuleEnum::EslintNoUndef(EslintNoUndef::default()), RuleEnum::EslintNoUndefined(EslintNoUndefined::default()), RuleEnum::EslintNoUnexpectedMultiline(EslintNoUnexpectedMultiline::default()), + RuleEnum::EslintNoUnmodifiedLoopCondition(EslintNoUnmodifiedLoopCondition::default()), RuleEnum::EslintNoUnneededTernary(EslintNoUnneededTernary::default()), RuleEnum::EslintNoUnreachable(EslintNoUnreachable::default()), RuleEnum::EslintNoUnsafeFinally(EslintNoUnsafeFinally::default()), diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 786c8d7a8fe84..76239f361676c 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -161,6 +161,7 @@ pub(crate) mod eslint { pub mod no_undef; pub mod no_undefined; pub mod no_unexpected_multiline; + pub mod no_unmodified_loop_condition; pub mod no_unneeded_ternary; pub mod no_unreachable; pub mod no_unsafe_finally; diff --git a/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs b/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs new file mode 100644 index 0000000000000..b5f2f1dc1b901 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs @@ -0,0 +1,392 @@ +use rustc_hash::{FxHashMap, FxHashSet}; + +use oxc_ast::{ + AstKind, + ast::{Expression, IdentifierReference}, +}; +use oxc_ast_visit::{Visit, walk}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::{NodeId, Reference, SymbolId}; +use oxc_span::{GetSpan, Span}; + +use crate::{AstNode, context::LintContext, rule::Rule}; + +fn no_unmodified_loop_condition_diagnostic(name: &str, span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn(format!("'{name}' is not modified in this loop.")).with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoUnmodifiedLoopCondition; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallow references in loop conditions that are never modified within the loop. + /// + /// ### Why is this bad? + /// + /// A loop condition that depends on values that never change within the loop body + /// can cause infinite loops or logic bugs. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// let done = false; + /// while (!done) { + /// work(); + /// } + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// let done = false; + /// while (!done) { + /// done = checkDone(); + /// } + /// ``` + NoUnmodifiedLoopCondition, + eslint, + suspicious +); + +impl Rule for NoUnmodifiedLoopCondition { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match node.kind() { + AstKind::WhileStatement(statement) => { + let loop_info = LoopInfo { loop_span: statement.span, loop_kind: LoopKind::While }; + Self::check_loop_condition(&statement.test, &loop_info, ctx); + } + AstKind::DoWhileStatement(statement) => { + let loop_info = + LoopInfo { loop_span: statement.span, loop_kind: LoopKind::DoWhile }; + Self::check_loop_condition(&statement.test, &loop_info, ctx); + } + AstKind::ForStatement(statement) => { + let Some(test) = &statement.test else { + return; + }; + let loop_info = LoopInfo { + loop_span: statement.span, + loop_kind: LoopKind::For { + init_span: statement.init.as_ref().map(GetSpan::span), + }, + }; + Self::check_loop_condition(test, &loop_info, ctx); + } + _ => {} + } + } +} + +#[derive(Debug, Clone, Copy)] +struct LoopInfo { + loop_span: Span, + loop_kind: LoopKind, +} + +#[derive(Debug, Clone, Copy)] +enum LoopKind { + While, + DoWhile, + For { init_span: Option }, +} + +impl LoopInfo { + fn is_in_loop(&self, reference: &Reference, ctx: &LintContext<'_>) -> bool { + let reference_span = ctx.semantic().reference_span(reference); + if !self.loop_span.contains_inclusive(reference_span) { + return false; + } + match self.loop_kind { + LoopKind::For { init_span: Some(init_span) } => { + !init_span.contains_inclusive(reference_span) + } + _ => true, + } + } +} + +impl NoUnmodifiedLoopCondition { + fn check_loop_condition<'a>( + condition: &Expression<'a>, + loop_info: &LoopInfo, + ctx: &LintContext<'a>, + ) { + let mut collector = ConditionSymbolsCollector::new(ctx); + collector.visit_expression(condition); + + let mut standalone_symbols: Vec<(SymbolId, NodeId)> = vec![]; + let mut standalone_seen: FxHashSet = FxHashSet::default(); + let mut grouped_symbols: FxHashMap> = FxHashMap::default(); + let mut grouped_seen: FxHashMap> = FxHashMap::default(); + let mut group_order: Vec = vec![]; + for symbol in collector.symbols { + if let Some(group_span) = symbol.group_span { + if !grouped_symbols.contains_key(&group_span) { + grouped_symbols.insert(group_span, vec![]); + group_order.push(group_span); + } + let seen = grouped_seen.entry(group_span).or_default(); + if seen.insert(symbol.symbol_id) { + grouped_symbols + .entry(group_span) + .or_default() + .push((symbol.symbol_id, symbol.reference_node_id)); + } + } else if standalone_seen.insert(symbol.symbol_id) { + standalone_symbols.push((symbol.symbol_id, symbol.reference_node_id)); + } + } + + let mut modified_cache: FxHashMap = FxHashMap::default(); + let mut diagnostics: Vec = vec![]; + let mut reported_symbols: FxHashSet = FxHashSet::default(); + + for (symbol_id, reference_node_id) in standalone_symbols { + if Self::is_symbol_modified_cached(symbol_id, &mut modified_cache, loop_info, ctx) { + continue; + } + if reported_symbols.insert(symbol_id) { + diagnostics.push(reference_node_id); + } + } + + for group_span in group_order { + if collector.dynamic_groups.contains(&group_span) { + continue; + } + let Some(symbols) = grouped_symbols.get(&group_span) else { + continue; + }; + let has_modified_symbol = symbols.iter().any(|(symbol_id, _)| { + Self::is_symbol_modified_cached(*symbol_id, &mut modified_cache, loop_info, ctx) + }); + if has_modified_symbol { + continue; + } + for (symbol_id, reference_node_id) in symbols { + if reported_symbols.insert(*symbol_id) { + diagnostics.push(*reference_node_id); + } + } + } + + for reference_node_id in diagnostics { + Self::report_condition(reference_node_id, ctx); + } + } + + fn report_condition(reference_node_id: NodeId, ctx: &LintContext<'_>) { + let node = ctx.nodes().get_node(reference_node_id); + let AstKind::IdentifierReference(ident) = node.kind() else { + return; + }; + ctx.diagnostic(no_unmodified_loop_condition_diagnostic(ident.name.as_str(), ident.span)); + } + + fn is_symbol_modified_in_loop( + symbol_id: SymbolId, + loop_info: &LoopInfo, + ctx: &LintContext<'_>, + ) -> bool { + for reference in ctx.scoping().get_resolved_references(symbol_id) { + if !reference.is_write() { + continue; + } + + if loop_info.is_in_loop(reference, ctx) + || Self::is_modified_via_called_function_declaration(loop_info, reference, ctx) + { + return true; + } + } + false + } + + fn is_symbol_modified_cached( + symbol_id: SymbolId, + modified_cache: &mut FxHashMap, + loop_info: &LoopInfo, + ctx: &LintContext<'_>, + ) -> bool { + if let Some(is_modified) = modified_cache.get(&symbol_id) { + return *is_modified; + } + let is_modified = Self::is_symbol_modified_in_loop(symbol_id, loop_info, ctx); + modified_cache.insert(symbol_id, is_modified); + is_modified + } + + fn is_modified_via_called_function_declaration( + loop_info: &LoopInfo, + modifier: &Reference, + ctx: &LintContext<'_>, + ) -> bool { + let Some(function_symbol_id) = + Self::get_enclosing_function_declaration_symbol_id(modifier.node_id(), ctx) + else { + return false; + }; + + for function_reference in ctx.scoping().get_resolved_references(function_symbol_id) { + if loop_info.is_in_loop(function_reference, ctx) { + return true; + } + } + + false + } + + fn get_enclosing_function_declaration_symbol_id( + node_id: NodeId, + ctx: &LintContext<'_>, + ) -> Option { + let nodes = ctx.nodes(); + let mut current_id = node_id; + + while current_id != NodeId::ROOT { + let current_node = nodes.get_node(current_id); + if let AstKind::Function(function) = current_node.kind() + && function.is_declaration() + { + return function.id.as_ref().map(oxc_ast::ast::BindingIdentifier::symbol_id); + } + current_id = nodes.parent_id(current_id); + } + + None + } +} + +struct ConditionSymbolsCollector<'a, 'ctx> { + ctx: &'ctx LintContext<'a>, + symbols: Vec, + group_stack: Vec, + dynamic_groups: FxHashSet, +} + +impl<'a, 'ctx> ConditionSymbolsCollector<'a, 'ctx> { + fn new(ctx: &'ctx LintContext<'a>) -> Self { + Self { ctx, symbols: vec![], group_stack: vec![], dynamic_groups: FxHashSet::default() } + } +} + +impl<'a> Visit<'a> for ConditionSymbolsCollector<'a, '_> { + fn visit_expression(&mut self, expression: &Expression<'a>) { + let is_group_expression = matches!( + expression, + Expression::BinaryExpression(_) | Expression::ConditionalExpression(_) + ); + if is_group_expression { + self.group_stack.push(expression.span()); + } + + if matches!( + expression, + Expression::CallExpression(_) + | Expression::StaticMemberExpression(_) + | Expression::ComputedMemberExpression(_) + | Expression::PrivateFieldExpression(_) + | Expression::NewExpression(_) + | Expression::TaggedTemplateExpression(_) + | Expression::YieldExpression(_) + ) { + if let Some(group_span) = self.group_stack.first().copied() { + self.dynamic_groups.insert(group_span); + } + if is_group_expression { + self.group_stack.pop(); + } + return; + } + + if matches!( + expression, + Expression::FunctionExpression(_) + | Expression::ArrowFunctionExpression(_) + | Expression::ClassExpression(_) + ) { + if is_group_expression { + self.group_stack.pop(); + } + return; + } + + walk::walk_expression(self, expression); + if is_group_expression { + self.group_stack.pop(); + } + } + + fn visit_identifier_reference(&mut self, identifier: &IdentifierReference<'a>) { + let reference_id = identifier.reference_id(); + let reference = self.ctx.scoping().get_reference(reference_id); + let Some(symbol_id) = reference.symbol_id() else { + return; + }; + self.symbols.push(ConditionSymbolInfo { + symbol_id, + reference_node_id: reference.node_id(), + group_span: self.group_stack.first().copied(), + }); + } +} + +struct ConditionSymbolInfo { + symbol_id: SymbolId, + reference_node_id: NodeId, + group_span: Option, +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "var foo = 0; while (foo) { ++foo; }", + "let foo = 0; while (foo) { ++foo; }", // { "ecmaVersion": 6 }, + "var foo = 0; while (foo) { foo += 1; }", + "var foo = 0; while (foo++) { }", + "var foo = 0; while (foo = next()) { }", + "var foo = 0; while (ok(foo)) { }", + "var foo = 0, bar = 0; while (++foo < bar) { }", + "var foo = 0, obj = {}; while (foo === obj.bar) { }", + "var foo = 0, f = {}, bar = {}; while (foo === f(bar)) { }", + "var foo = 0, f = {}; while (foo === f()) { }", + "var foo = 0, tag = 0; while (foo === tag`abc`) { }", // { "ecmaVersion": 6 }, + "function* foo() { var foo = 0; while (yield foo) { } }", // { "ecmaVersion": 6 }, + "function* foo() { var foo = 0; while (foo === (yield)) { } }", // { "ecmaVersion": 6 }, + "var foo = 0; while (foo.ok) { }", + "var foo = 0; while (foo) { update(); } function update() { ++foo; }", + "var foo = 0, bar = 9; while (foo < bar) { foo += 1; }", + "var foo = 0, bar = 1, baz = 2; while (foo ? bar : baz) { foo += 1; }", + "var foo = 0, bar = 0; while (foo && bar) { ++foo; ++bar; }", + "var foo = 0, bar = 0; while (foo || bar) { ++foo; ++bar; }", + "var foo = 0; do { ++foo; } while (foo);", + "var foo = 0; do { } while (foo++);", + "for (var foo = 0; foo; ++foo) { }", + "for (var foo = 0; foo;) { ++foo }", + "var foo = 0, bar = 0; for (bar; foo;) { ++foo }", + "var foo; if (foo) { }", + "var a = [1, 2, 3]; var len = a.length; for (var i = 0; i < len - 1; i++) {}", + ]; + + let fail = vec![ + "var foo = 0; while (foo) { } foo = 1;", + "var foo = 0; while (!foo) { } foo = 1;", + "var foo = 0; while (foo != null) { } foo = 1;", + "var foo = 0, bar = 9; while (foo < bar) { } foo = 1;", + "var foo = 0, bar = 0; while (foo && bar) { ++bar; } foo = 1;", + "var foo = 0, bar = 0; while (foo && bar) { ++foo; } foo = 1;", + "var a, b, c; while (a < c && b < c) { ++a; } foo = 1;", + "var foo = 0; while (foo ? 1 : 0) { } foo = 1;", + "var foo = 0; while (foo) { update(); } function update(foo) { ++foo; }", + "var foo; do { } while (foo);", + "for (var foo = 0; foo < 10; ) { } foo = 1;", + ]; + + Tester::new(NoUnmodifiedLoopCondition::NAME, NoUnmodifiedLoopCondition::PLUGIN, pass, fail) + .test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/eslint_no_unmodified_loop_condition.snap b/crates/oxc_linter/src/snapshots/eslint_no_unmodified_loop_condition.snap new file mode 100644 index 0000000000000..44c9940cec529 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_no_unmodified_loop_condition.snap @@ -0,0 +1,81 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:21] + 1 │ var foo = 0; while (foo) { } foo = 1; + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:22] + 1 │ var foo = 0; while (!foo) { } foo = 1; + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:21] + 1 │ var foo = 0; while (foo != null) { } foo = 1; + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:30] + 1 │ var foo = 0, bar = 9; while (foo < bar) { } foo = 1; + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'bar' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:36] + 1 │ var foo = 0, bar = 9; while (foo < bar) { } foo = 1; + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:30] + 1 │ var foo = 0, bar = 0; while (foo && bar) { ++bar; } foo = 1; + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'bar' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:37] + 1 │ var foo = 0, bar = 0; while (foo && bar) { ++foo; } foo = 1; + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'b' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:30] + 1 │ var a, b, c; while (a < c && b < c) { ++a; } foo = 1; + · ─ + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'c' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:34] + 1 │ var a, b, c; while (a < c && b < c) { ++a; } foo = 1; + · ─ + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:21] + 1 │ var foo = 0; while (foo ? 1 : 0) { } foo = 1; + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:21] + 1 │ var foo = 0; while (foo) { update(); } function update(foo) { ++foo; } + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:24] + 1 │ var foo; do { } while (foo); + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:19] + 1 │ for (var foo = 0; foo < 10; ) { } foo = 1; + · ─── + ╰──── From 96fc949d5bfb0b4aead2a1b8676e5902e4149ca1 Mon Sep 17 00:00:00 2001 From: Limerio <44976231+Limerio@users.noreply.github.com> Date: Thu, 12 Feb 2026 23:41:48 +0100 Subject: [PATCH 2/4] fix(linter): optimize group symbol insertion in no-unmodified-loop-condition rule --- .../src/rules/eslint/no_unmodified_loop_condition.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs b/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs index b5f2f1dc1b901..58f065253f703 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs @@ -124,8 +124,10 @@ impl NoUnmodifiedLoopCondition { let mut group_order: Vec = vec![]; for symbol in collector.symbols { if let Some(group_span) = symbol.group_span { - if !grouped_symbols.contains_key(&group_span) { - grouped_symbols.insert(group_span, vec![]); + if let std::collections::hash_map::Entry::Vacant(entry) = + grouped_symbols.entry(group_span) + { + entry.insert(vec![]); group_order.push(group_span); } let seen = grouped_seen.entry(group_span).or_default(); From fde70c91f802bbe17f9fcc58b33a97397d55ae93 Mon Sep 17 00:00:00 2001 From: Cameron Clark Date: Fri, 13 Feb 2026 17:25:04 +0000 Subject: [PATCH 3/4] u --- .../eslint/no_unmodified_loop_condition.rs | 58 +++++++++++++++++-- .../eslint_no_unmodified_loop_condition.snap | 13 +++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs b/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs index 58f065253f703..14fb4be971292 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs @@ -1,5 +1,6 @@ use rustc_hash::{FxHashMap, FxHashSet}; +use oxc_allocator::GetAddress; use oxc_ast::{ AstKind, ast::{Expression, IdentifierReference}, @@ -55,12 +56,19 @@ impl Rule for NoUnmodifiedLoopCondition { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { match node.kind() { AstKind::WhileStatement(statement) => { - let loop_info = LoopInfo { loop_span: statement.span, loop_kind: LoopKind::While }; + let loop_info = LoopInfo { + loop_span: statement.span, + loop_node_id: node.id(), + loop_kind: LoopKind::While, + }; Self::check_loop_condition(&statement.test, &loop_info, ctx); } AstKind::DoWhileStatement(statement) => { - let loop_info = - LoopInfo { loop_span: statement.span, loop_kind: LoopKind::DoWhile }; + let loop_info = LoopInfo { + loop_span: statement.span, + loop_node_id: node.id(), + loop_kind: LoopKind::DoWhile, + }; Self::check_loop_condition(&statement.test, &loop_info, ctx); } AstKind::ForStatement(statement) => { @@ -69,6 +77,7 @@ impl Rule for NoUnmodifiedLoopCondition { }; let loop_info = LoopInfo { loop_span: statement.span, + loop_node_id: node.id(), loop_kind: LoopKind::For { init_span: statement.init.as_ref().map(GetSpan::span), }, @@ -83,6 +92,7 @@ impl Rule for NoUnmodifiedLoopCondition { #[derive(Debug, Clone, Copy)] struct LoopInfo { loop_span: Span, + loop_node_id: NodeId, loop_kind: LoopKind, } @@ -99,12 +109,28 @@ impl LoopInfo { if !self.loop_span.contains_inclusive(reference_span) { return false; } - match self.loop_kind { + let is_in_loop_range = match self.loop_kind { LoopKind::For { init_span: Some(init_span) } => { !init_span.contains_inclusive(reference_span) } _ => true, + }; + + is_in_loop_range && !self.is_in_nested_function_scope(reference, ctx) + } + + fn is_in_nested_function_scope(&self, reference: &Reference, ctx: &LintContext<'_>) -> bool { + for (ancestor_id, ancestor) in ctx.nodes().ancestors_enumerated(reference.node_id()) { + if ancestor_id == self.loop_node_id { + return false; + } + if matches!(ancestor.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_)) + { + return true; + } } + + false } } @@ -233,7 +259,9 @@ impl NoUnmodifiedLoopCondition { }; for function_reference in ctx.scoping().get_resolved_references(function_symbol_id) { - if loop_info.is_in_loop(function_reference, ctx) { + if loop_info.is_in_loop(function_reference, ctx) + && Self::is_function_invocation_reference(function_reference, ctx) + { return true; } } @@ -260,6 +288,24 @@ impl NoUnmodifiedLoopCondition { None } + + fn is_function_invocation_reference(reference: &Reference, ctx: &LintContext<'_>) -> bool { + let reference_node_id = reference.node_id(); + let reference_node = ctx.nodes().get_node(reference_node_id); + + match ctx.nodes().parent_kind(reference_node_id) { + AstKind::CallExpression(call_expression) => { + call_expression.callee.address() == reference_node.address() + } + AstKind::NewExpression(new_expression) => { + new_expression.callee.address() == reference_node.address() + } + AstKind::TaggedTemplateExpression(tagged_template_expression) => { + tagged_template_expression.tag.address() == reference_node.address() + } + _ => false, + } + } } struct ConditionSymbolsCollector<'a, 'ctx> { @@ -385,6 +431,8 @@ fn test() { "var a, b, c; while (a < c && b < c) { ++a; } foo = 1;", "var foo = 0; while (foo ? 1 : 0) { } foo = 1;", "var foo = 0; while (foo) { update(); } function update(foo) { ++foo; }", + "var foo = 0; while (foo < 10) { function update() { foo++; } }", + "var foo = 0; while (foo < 10) { const fn = update; } function update() { foo++; }", "var foo; do { } while (foo);", "for (var foo = 0; foo < 10; ) { } foo = 1;", ]; diff --git a/crates/oxc_linter/src/snapshots/eslint_no_unmodified_loop_condition.snap b/crates/oxc_linter/src/snapshots/eslint_no_unmodified_loop_condition.snap index 44c9940cec529..f9b7c8b77b28b 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_unmodified_loop_condition.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_unmodified_loop_condition.snap @@ -1,5 +1,6 @@ --- source: crates/oxc_linter/src/tester.rs +assertion_line: 444 --- ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. @@ -68,6 +69,18 @@ source: crates/oxc_linter/src/tester.rs · ─── ╰──── + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:21] + 1 │ var foo = 0; while (foo < 10) { function update() { foo++; } } + · ─── + ╰──── + + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. + ╭─[no_unmodified_loop_condition.tsx:1:21] + 1 │ var foo = 0; while (foo < 10) { const fn = update; } function update() { foo++; } + · ─── + ╰──── + ⚠ eslint(no-unmodified-loop-condition): 'foo' is not modified in this loop. ╭─[no_unmodified_loop_condition.tsx:1:24] 1 │ var foo; do { } while (foo); From 0455859cce6fc5f942ddd1361f187420eefa75a4 Mon Sep 17 00:00:00 2001 From: Cameron Clark Date: Fri, 13 Feb 2026 17:32:12 +0000 Subject: [PATCH 4/4] clippy --- .../eslint/no_unmodified_loop_condition.rs | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs b/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs index 14fb4be971292..852080bdb6b9c 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs @@ -56,19 +56,13 @@ impl Rule for NoUnmodifiedLoopCondition { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { match node.kind() { AstKind::WhileStatement(statement) => { - let loop_info = LoopInfo { - loop_span: statement.span, - loop_node_id: node.id(), - loop_kind: LoopKind::While, - }; + let loop_info = + LoopInfo { span: statement.span, node_id: node.id(), kind: LoopKind::While }; Self::check_loop_condition(&statement.test, &loop_info, ctx); } AstKind::DoWhileStatement(statement) => { - let loop_info = LoopInfo { - loop_span: statement.span, - loop_node_id: node.id(), - loop_kind: LoopKind::DoWhile, - }; + let loop_info = + LoopInfo { span: statement.span, node_id: node.id(), kind: LoopKind::DoWhile }; Self::check_loop_condition(&statement.test, &loop_info, ctx); } AstKind::ForStatement(statement) => { @@ -76,11 +70,9 @@ impl Rule for NoUnmodifiedLoopCondition { return; }; let loop_info = LoopInfo { - loop_span: statement.span, - loop_node_id: node.id(), - loop_kind: LoopKind::For { - init_span: statement.init.as_ref().map(GetSpan::span), - }, + span: statement.span, + node_id: node.id(), + kind: LoopKind::For { init_span: statement.init.as_ref().map(GetSpan::span) }, }; Self::check_loop_condition(test, &loop_info, ctx); } @@ -91,9 +83,9 @@ impl Rule for NoUnmodifiedLoopCondition { #[derive(Debug, Clone, Copy)] struct LoopInfo { - loop_span: Span, - loop_node_id: NodeId, - loop_kind: LoopKind, + span: Span, + node_id: NodeId, + kind: LoopKind, } #[derive(Debug, Clone, Copy)] @@ -106,10 +98,10 @@ enum LoopKind { impl LoopInfo { fn is_in_loop(&self, reference: &Reference, ctx: &LintContext<'_>) -> bool { let reference_span = ctx.semantic().reference_span(reference); - if !self.loop_span.contains_inclusive(reference_span) { + if !self.span.contains_inclusive(reference_span) { return false; } - let is_in_loop_range = match self.loop_kind { + let is_in_loop_range = match self.kind { LoopKind::For { init_span: Some(init_span) } => { !init_span.contains_inclusive(reference_span) } @@ -121,7 +113,7 @@ impl LoopInfo { fn is_in_nested_function_scope(&self, reference: &Reference, ctx: &LintContext<'_>) -> bool { for (ancestor_id, ancestor) in ctx.nodes().ancestors_enumerated(reference.node_id()) { - if ancestor_id == self.loop_node_id { + if ancestor_id == self.node_id { return false; } if matches!(ancestor.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_))