From 6abde0a69fdfdc35fd294542d32e0973db6eea3b Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Thu, 5 Sep 2024 16:04:42 +0000 Subject: [PATCH] refactor(transformer): clarify match in RegExp transform (#5498) Refactor match on `Term` in RegExp transform. This is personal taste, really. I prefer this style: ```rs match enum { Enum::Variant(x) => x.is_something(), _ => false, } ``` over: ```rs match enum { Enum::Variant(x) if x.is_something() => true, _ => false, } ``` because then you don't have to reason about what happens when the `if` is false and it "falls through" to the next match arm. The compiler *might* also find the former easier to optimize, though likely in most cases it can figure out that they're equivalent. Also rename a couple of vars for clarity. --- crates/oxc_transformer/src/regexp/mod.rs | 28 +++++++++++------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/crates/oxc_transformer/src/regexp/mod.rs b/crates/oxc_transformer/src/regexp/mod.rs index a23464abd..2ad1e5e22 100644 --- a/crates/oxc_transformer/src/regexp/mod.rs +++ b/crates/oxc_transformer/src/regexp/mod.rs @@ -45,7 +45,6 @@ mod options; use std::borrow::Cow; pub use options::RegExpOptions; -use oxc_allocator::Vec; use oxc_ast::ast::*; use oxc_diagnostics::Result; use oxc_regular_expression::ast::{ @@ -198,36 +197,35 @@ impl<'a> RegExp<'a> { /// /// Based on parsed regular expression pattern. fn has_unsupported_regular_expression_pattern(&self, pattern: &Pattern<'a>) -> bool { - let check_terms = |terms: &Vec<'a, Term>| { - terms.iter().any(|element| match element { - Term::CapturingGroup(_) if self.named_capture_groups => true, - Term::UnicodePropertyEscape(_) if self.unicode_property_escapes => true, - Term::CharacterClass(character_class) if self.unicode_property_escapes => { - has_unicode_property_escape_character_class(character_class) + let terms_contains_unsupported = |terms: &[Term]| { + terms.iter().any(|term| match term { + Term::CapturingGroup(_) => self.named_capture_groups, + Term::UnicodePropertyEscape(_) => self.unicode_property_escapes, + Term::CharacterClass(character_class) => { + self.unicode_property_escapes + && character_class_has_unicode_property_escape(character_class) } - Term::LookAroundAssertion(assertion) - if self.look_behind_assertions + Term::LookAroundAssertion(assertion) => { + self.look_behind_assertions && matches!( assertion.kind, LookAroundAssertionKind::Lookbehind | LookAroundAssertionKind::NegativeLookbehind - ) => - { - true + ) } _ => false, }) }; - pattern.body.body.iter().any(|alternative| check_terms(&alternative.body)) + pattern.body.body.iter().any(|alternative| terms_contains_unsupported(&alternative.body)) } } -fn has_unicode_property_escape_character_class(character_class: &CharacterClass) -> bool { +fn character_class_has_unicode_property_escape(character_class: &CharacterClass) -> bool { character_class.body.iter().any(|element| match element { CharacterClassContents::UnicodePropertyEscape(_) => true, CharacterClassContents::NestedCharacterClass(character_class) => { - has_unicode_property_escape_character_class(character_class) + character_class_has_unicode_property_escape(character_class) } _ => false, })