From f32cefee6fe16bba6dfb2b1c1b1af40cc81a63c0 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 25 Sep 2024 22:15:49 -0400 Subject: [PATCH] refactor(linter): use regex visitor in `no-regex-spaces` --- .../src/rules/eslint/no_regex_spaces.rs | 117 +++++++----------- 1 file changed, 45 insertions(+), 72 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_regex_spaces.rs b/crates/oxc_linter/src/rules/eslint/no_regex_spaces.rs index ea318d527e1bac..287de65b870e65 100644 --- a/crates/oxc_linter/src/rules/eslint/no_regex_spaces.rs +++ b/crates/oxc_linter/src/rules/eslint/no_regex_spaces.rs @@ -8,7 +8,8 @@ use oxc_ast::{ use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_regular_expression::{ - ast::{Alternative, Disjunction, Pattern, Term}, + ast::{Character, Pattern}, + visit::{RegexAstKind, Visit}, Parser, ParserOptions, }; use oxc_span::Span; @@ -72,7 +73,6 @@ impl Rule for NoRegexSpaces { ctx.diagnostic(no_regex_spaces_diagnostic(span)); // new RegExp('a b') } } - _ => {} } } @@ -87,7 +87,7 @@ impl NoRegexSpaces { } let pattern = literal.regex.pattern.as_pattern()?; - Self::find_consecutive_spaces(pattern) + find_consecutive_spaces(pattern) } fn find_expr_to_report(args: &Vec<'_, Argument<'_>>) -> Option { @@ -109,48 +109,7 @@ impl NoRegexSpaces { let parser = Parser::new(&alloc, pattern_with_slashes.as_str(), ParserOptions::default()); let regex = parser.parse().ok()?; - Self::find_consecutive_spaces(®ex.pattern) - .map(|span| Span::new(span.start + pattern.span.start, span.end + pattern.span.start)) - } - - fn find_consecutive_spaces(pattern: &Pattern) -> Option { - let mut last_space_span: Option = None; - let mut in_quantifier = false; - visit_terms(pattern, &mut |term| { - if let Term::Quantifier(_) = term { - in_quantifier = true; - return; - } - let Term::Character(ch) = term else { - return; - }; - if in_quantifier { - in_quantifier = false; - return; - } - if ch.value != u32::from(b' ') { - return; - } - if let Some(ref mut space_span) = last_space_span { - // If this is consecutive with the last space, extend it - if space_span.end == ch.span.start { - space_span.end = ch.span.end; - } - // If it is not consecutive, and the last space is only one space, move it up - else if space_span.size() == 1 { - last_space_span.replace(ch.span); - } - } else { - last_space_span = Some(ch.span); - } - }); - - // return None if last_space_span length is only 1 - if last_space_span.is_some_and(|span| span.size() > 1) { - last_space_span - } else { - None - } + find_consecutive_spaces(®ex.pattern) } fn is_regexp_new_expression(expr: &NewExpression<'_>) -> bool { @@ -168,39 +127,53 @@ impl NoRegexSpaces { } } -/// Calls the given closure on every [`Term`] in the [`Pattern`]. -fn visit_terms<'a, F: FnMut(&'a Term<'a>)>(pattern: &'a Pattern, f: &mut F) { - visit_terms_disjunction(&pattern.body, f); +fn find_consecutive_spaces(pattern: &Pattern) -> Option { + let mut finder = + ConsecutiveSpaceFinder { last_space_span: None, in_quantifier_or_class: false }; + finder.visit_pattern(pattern); + + // return none if span is only one space + finder + .last_space_span + .map(|span| Span::new(span.start + pattern.span.start, span.end + pattern.span.start)) + .filter(|span| span.size() > 1) } -/// Calls the given closure on every [`Term`] in the [`Disjunction`]. -fn visit_terms_disjunction<'a, F: FnMut(&'a Term<'a>)>(disjunction: &'a Disjunction, f: &mut F) { - for alternative in &disjunction.body { - visit_terms_alternative(alternative, f); - } +struct ConsecutiveSpaceFinder { + last_space_span: Option, + in_quantifier_or_class: bool, } -/// Calls the given closure on every [`Term`] in the [`Alternative`]. -fn visit_terms_alternative<'a, F: FnMut(&'a Term<'a>)>(alternative: &'a Alternative, f: &mut F) { - for term in &alternative.body { - match term { - Term::LookAroundAssertion(lookaround) => { - f(term); - visit_terms_disjunction(&lookaround.body, f); - } - Term::Quantifier(quant) => { - f(term); - f(&quant.body); - } - Term::CapturingGroup(group) => { - f(term); - visit_terms_disjunction(&group.body, f); +impl<'a> Visit<'a> for ConsecutiveSpaceFinder { + fn enter_node(&mut self, kind: RegexAstKind<'a>) { + if let RegexAstKind::Quantifier(_) | RegexAstKind::CharacterClass(_) = kind { + self.in_quantifier_or_class = true; + } + } + fn leave_node(&mut self, kind: RegexAstKind<'a>) { + if let RegexAstKind::Quantifier(_) | RegexAstKind::CharacterClass(_) = kind { + self.in_quantifier_or_class = false; + } + } + + fn visit_character(&mut self, ch: &Character) { + if self.in_quantifier_or_class { + return; + } + if ch.value != u32::from(b' ') { + return; + } + if let Some(ref mut space_span) = self.last_space_span { + // If this is consecutive with the last space, extend it + if space_span.end == ch.span.start { + space_span.end = ch.span.end; } - Term::IgnoreGroup(group) => { - f(term); - visit_terms_disjunction(&group.body, f); + // If it is not consecutive, and the last space is only one space, move it up + else if space_span.size() == 1 { + self.last_space_span.replace(ch.span); } - _ => f(term), + } else { + self.last_space_span = Some(ch.span); } } }