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..852080bdb6b9c --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_unmodified_loop_condition.rs @@ -0,0 +1,434 @@ +use rustc_hash::{FxHashMap, FxHashSet}; + +use oxc_allocator::GetAddress; +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 { 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 { span: statement.span, node_id: node.id(), 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 { + 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); + } + _ => {} + } + } +} + +#[derive(Debug, Clone, Copy)] +struct LoopInfo { + span: Span, + node_id: NodeId, + 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.span.contains_inclusive(reference_span) { + return false; + } + let is_in_loop_range = match self.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.node_id { + return false; + } + if matches!(ancestor.kind(), AstKind::Function(_) | AstKind::ArrowFunctionExpression(_)) + { + return true; + } + } + + false + } +} + +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 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(); + 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) + && Self::is_function_invocation_reference(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 + } + + 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> { + 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 = 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;", + ]; + + 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..f9b7c8b77b28b --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_no_unmodified_loop_condition.snap @@ -0,0 +1,94 @@ +--- +source: crates/oxc_linter/src/tester.rs +assertion_line: 444 +--- + + ⚠ 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: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); + · ─── + ╰──── + + ⚠ 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; + · ─── + ╰────