Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(regular_expression)!: Simplify public APIs #6262

Merged
merged 1 commit into from
Oct 3, 2024
Merged

refactor(regular_expression)!: Simplify public APIs #6262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 0 additions & 36 deletions crates/oxc_ast/src/generated/assert_layouts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1412,24 +1412,6 @@ const _: () = {
assert!(size_of::<LanguageVariant>() == 1usize);
assert!(align_of::<LanguageVariant>() == 1usize);

assert!(size_of::<RegularExpression>() == 72usize);
assert!(align_of::<RegularExpression>() == 8usize);
assert!(offset_of!(RegularExpression, span) == 0usize);
assert!(offset_of!(RegularExpression, pattern) == 8usize);
assert!(offset_of!(RegularExpression, flags) == 56usize);

assert!(size_of::<Flags>() == 16usize);
assert!(align_of::<Flags>() == 4usize);
assert!(offset_of!(Flags, span) == 0usize);
assert!(offset_of!(Flags, global) == 8usize);
assert!(offset_of!(Flags, ignore_case) == 9usize);
assert!(offset_of!(Flags, multiline) == 10usize);
assert!(offset_of!(Flags, unicode) == 11usize);
assert!(offset_of!(Flags, sticky) == 12usize);
assert!(offset_of!(Flags, dot_all) == 13usize);
assert!(offset_of!(Flags, has_indices) == 14usize);
assert!(offset_of!(Flags, unicode_sets) == 15usize);

assert!(size_of::<Pattern>() == 48usize);
assert!(align_of::<Pattern>() == 8usize);
assert!(offset_of!(Pattern, span) == 0usize);
Expand Down Expand Up @@ -2967,24 +2949,6 @@ const _: () = {
assert!(size_of::<LanguageVariant>() == 1usize);
assert!(align_of::<LanguageVariant>() == 1usize);

assert!(size_of::<RegularExpression>() == 56usize);
assert!(align_of::<RegularExpression>() == 4usize);
assert!(offset_of!(RegularExpression, span) == 0usize);
assert!(offset_of!(RegularExpression, pattern) == 8usize);
assert!(offset_of!(RegularExpression, flags) == 40usize);

assert!(size_of::<Flags>() == 16usize);
assert!(align_of::<Flags>() == 4usize);
assert!(offset_of!(Flags, span) == 0usize);
assert!(offset_of!(Flags, global) == 8usize);
assert!(offset_of!(Flags, ignore_case) == 9usize);
assert!(offset_of!(Flags, multiline) == 10usize);
assert!(offset_of!(Flags, unicode) == 11usize);
assert!(offset_of!(Flags, sticky) == 12usize);
assert!(offset_of!(Flags, dot_all) == 13usize);
assert!(offset_of!(Flags, has_indices) == 14usize);
assert!(offset_of!(Flags, unicode_sets) == 15usize);

assert!(size_of::<Pattern>() == 32usize);
assert!(align_of::<Pattern>() == 4usize);
assert!(offset_of!(Pattern, span) == 0usize);
Expand Down
49 changes: 17 additions & 32 deletions crates/oxc_linter/src/rules/eslint/no_control_regex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use oxc_allocator::Allocator;
use oxc_ast::{
ast::{Argument, RegExpFlags},
AstKind,
};
use oxc_ast::{ast::Argument, AstKind};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_regular_expression::{
Expand Down Expand Up @@ -92,25 +89,19 @@ impl Rule for NoControlRegex {
// get pattern from arguments. Missing or non-string arguments
// will be runtime errors, but are not covered by this rule.
let alloc = Allocator::default();
let pattern_with_slashes = format!("/{}/", &pattern.value);
let flags = extract_regex_flags(&expr.arguments);
let flags_text = flags.map_or(String::new(), |f| f.to_string());
let parser = Parser::new(
&alloc,
pattern_with_slashes.as_str(),
ParserOptions {
span_offset: expr
.arguments
.first()
.map_or(0, |arg| arg.span().start),
unicode_mode: flags
.is_some_and(|flags| flags.contains(RegExpFlags::U)),
unicode_sets_mode: flags
.is_some_and(|flags| flags.contains(RegExpFlags::V)),
},
pattern.value.as_str(),
ParserOptions::default()
.with_span_offset(
expr.arguments.first().map_or(0, |arg| arg.span().start),
)
.with_flags(&flags_text),
);

let Some(pattern) = parser.parse().ok().map(|pattern| pattern.pattern)
else {
let Ok(pattern) = parser.parse() else {
return;
};

Expand All @@ -133,25 +124,19 @@ impl Rule for NoControlRegex {
// get pattern from arguments. Missing or non-string arguments
// will be runtime errors, but are not covered by this rule.
let alloc = Allocator::default();
let pattern_with_slashes = format!("/{}/", &pattern.value);
let flags = extract_regex_flags(&expr.arguments);
let flags_text = flags.map_or(String::new(), |f| f.to_string());
let parser = Parser::new(
&alloc,
pattern_with_slashes.as_str(),
ParserOptions {
span_offset: expr
.arguments
.first()
.map_or(0, |arg| arg.span().start),
unicode_mode: flags
.is_some_and(|flags| flags.contains(RegExpFlags::U)),
unicode_sets_mode: flags
.is_some_and(|flags| flags.contains(RegExpFlags::V)),
},
pattern.value.as_str(),
ParserOptions::default()
.with_span_offset(
expr.arguments.first().map_or(0, |arg| arg.span().start),
)
.with_flags(&flags_text),
);

let Some(pattern) = parser.parse().ok().map(|pattern| pattern.pattern)
else {
let Ok(pattern) = parser.parse() else {
return;
};

Expand Down
105 changes: 49 additions & 56 deletions crates/oxc_linter/src/rules/eslint/no_invalid_regexp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,28 @@
use oxc_allocator::Allocator;
use oxc_ast::{ast::Argument, AstKind};
use oxc_diagnostics::{LabeledSpan, OxcDiagnostic};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_regular_expression::{FlagsParser, ParserOptions, PatternParser};
use oxc_regular_expression::{Parser, ParserOptions};
use oxc_span::Span;
use rustc_hash::FxHashSet;
use serde::Deserialize;

use crate::{context::LintContext, rule::Rule, AstNode};

// Use the same prefix with `oxc_regular_expression` crate
fn duplicated_flag_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Invalid regular expression: Duplicated flag").with_label(span)
}

fn unknown_flag_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Invalid regular expression: Unknown flag").with_label(span)
}

fn invalid_unicode_flags_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Invalid regular expression: `u` and `v` flags should be used alone")
.with_label(span)
}

#[derive(Debug, Default, Clone)]
pub struct NoInvalidRegexp(Box<NoInvalidRegexpConfig>);

Expand Down Expand Up @@ -72,75 +86,54 @@ impl Rule for NoInvalidRegexp {
return;
}

let allocator = Allocator::default();

// Validate flags first if exists
let mut parsed_flags = None;
if let Some((flags_span_start, flags_text)) = flags_arg {
// Check for duplicated flags
// For compatibility with ESLint, we need to check "user-defined duplicated" flags here
// "valid duplicated" flags are also checked
let (mut u_flag_found, mut v_flag_found) = (false, false);
let mut unique_flags = FxHashSet::default();
let mut violations = vec![];
for (idx, ch) in flags_text.char_indices() {
if !unique_flags.insert(ch) {
violations.push(idx);
}
}
if !violations.is_empty() {
return ctx.diagnostic(
// Use the same prefix with `oxc_regular_expression`
OxcDiagnostic::warn("Invalid regular expression: Duplicated flag").with_labels(
violations
.iter()
.map(|&start| {
#[allow(clippy::cast_possible_truncation)]
let start = flags_span_start + start as u32;
LabeledSpan::new_with_span(None, Span::new(start, start))
})
.collect::<Vec<_>>(),
),
);
}
#[allow(clippy::cast_possible_truncation)]
let start = flags_span_start + idx as u32;

// Omit user defined invalid flags
for flag in &self.0.allow_constructor_flags {
match flag {
// Keep valid flags, even if they are defined
'd' | 'g' | 'i' | 'm' | 's' | 'u' | 'v' | 'y' => continue,
_ => {
unique_flags.remove(flag);
// Invalid combination: u+v
if ch == 'u' {
if v_flag_found {
return ctx
.diagnostic(invalid_unicode_flags_diagnostic(Span::new(start, start)));
}
u_flag_found = true;
}
if ch == 'v' {
if u_flag_found {
return ctx
.diagnostic(invalid_unicode_flags_diagnostic(Span::new(start, start)));
}
v_flag_found = true;
}

// Duplicated: user defined, invalid or valid
if !unique_flags.insert(ch) {
return ctx.diagnostic(duplicated_flag_diagnostic(Span::new(start, start)));
}
}

// Use parser to check:
// - Unknown invalid flags
// - Invalid flags combination: u+v
// - (Valid duplicated flags are already checked above)
// It can be done without `FlagsParser`, though
let flags_text = unique_flags.iter().collect::<String>();
let options = ParserOptions::default().with_span_offset(flags_span_start);
match FlagsParser::new(&allocator, flags_text.as_str(), options).parse() {
Ok(flags) => parsed_flags = Some(flags),
Err(diagnostic) => return ctx.diagnostic(diagnostic),
// Unknown: not valid, not user defined
if !(matches!(ch, 'd' | 'g' | 'i' | 'm' | 's' | 'u' | 'v' | 'y')
|| self.0.allow_constructor_flags.contains(&ch))
{
return ctx.diagnostic(unknown_flag_diagnostic(Span::new(start, start)));
}
}
}

// Then, validate pattern if exists
// Pattern check is skipped when 1st argument is NOT a `StringLiteral`
// e.g. `new RegExp(var)`, `RegExp("str" + var)`
let allocator = Allocator::default();
if let Some((pattern_span_start, pattern_text)) = pattern_arg {
let mut options = ParserOptions::default().with_span_offset(pattern_span_start);
if let Some(flags) = parsed_flags {
if flags.unicode || flags.unicode_sets {
options = options.with_unicode_mode();
}
if flags.unicode_sets {
options = options.with_unicode_sets_mode();
}
}
match PatternParser::new(&allocator, pattern_text, options).parse() {
let options = ParserOptions::default()
.with_span_offset(pattern_span_start)
.with_flags(flags_arg.map_or("", |(_, flags_text)| flags_text));

match Parser::new(&allocator, pattern_text, options).parse() {
Ok(_) => {}
Err(diagnostic) => ctx.diagnostic(diagnostic),
}
Expand Down
12 changes: 7 additions & 5 deletions crates/oxc_linter/src/rules/eslint/no_regex_spaces.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,14 @@ impl NoRegexSpaces {
}

let alloc = Allocator::default();
let pattern_with_slashes = format!("/{}/", &pattern.value);
let parser = Parser::new(&alloc, pattern_with_slashes.as_str(), ParserOptions::default());
let regex = parser.parse().ok()?;
let parser = Parser::new(
&alloc,
pattern.value.as_str(),
ParserOptions::default().with_span_offset(pattern.span.start + 1),
);
let parsed_pattern = parser.parse().ok()?;

find_consecutive_spaces(&regex.pattern)
.map(|span| Span::new(span.start + pattern.span.start, span.end + pattern.span.start))
find_consecutive_spaces(&parsed_pattern)
}

fn is_regexp_new_expression(expr: &NewExpression<'_>) -> bool {
Expand Down
26 changes: 13 additions & 13 deletions crates/oxc_linter/src/snapshots/no_invalid_regexp.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ source: crates/oxc_linter/src/tester.rs
╰────

⚠ eslint(no-invalid-regexp): Invalid regular expression: Unknown flag
╭─[no_invalid_regexp.tsx:1:18]
╭─[no_invalid_regexp.tsx:1:19]
1 │ new RegExp('.', 'aA');
· ▲
· ▲
╰────

⚠ eslint(no-invalid-regexp): Invalid regular expression: Duplicated flag
Expand All @@ -91,10 +91,10 @@ source: crates/oxc_linter/src/tester.rs
· ▲
╰────

⚠ eslint(no-invalid-regexp): Invalid regular expression: Duplicated flag
╭─[no_invalid_regexp.tsx:1:20]
⚠ eslint(no-invalid-regexp): Invalid regular expression: Unknown flag
╭─[no_invalid_regexp.tsx:1:18]
1 │ new RegExp('.', 'ouo');
· ▲
· ▲
╰────

⚠ eslint(no-invalid-regexp): Invalid regular expression: Could not parse the entire pattern
Expand Down Expand Up @@ -164,9 +164,9 @@ source: crates/oxc_linter/src/tester.rs
╰────

⚠ eslint(no-invalid-regexp): Invalid regular expression: Unknown flag
╭─[no_invalid_regexp.tsx:1:22]
╭─[no_invalid_regexp.tsx:1:23]
1 │ new RegExp(pattern, 'az');
· ▲
· ▲
╰────

⚠ eslint(no-invalid-regexp): Invalid regular expression: Unterminated character class
Expand All @@ -175,16 +175,16 @@ source: crates/oxc_linter/src/tester.rs
· ───
╰────

⚠ eslint(no-invalid-regexp): Invalid regular expression: Invalid flags, `u` and `v` should be used alone
╭─[no_invalid_regexp.tsx:1:18]
⚠ eslint(no-invalid-regexp): Invalid regular expression: `u` and `v` flags should be used alone
╭─[no_invalid_regexp.tsx:1:19]
1 │ new RegExp('.', 'uv');
· ──
· ▲
╰────

⚠ eslint(no-invalid-regexp): Invalid regular expression: Invalid flags, `u` and `v` should be used alone
╭─[no_invalid_regexp.tsx:1:22]
⚠ eslint(no-invalid-regexp): Invalid regular expression: `u` and `v` flags should be used alone
╭─[no_invalid_regexp.tsx:1:23]
1 │ new RegExp(pattern, 'uv');
· ──
· ▲
╰────

⚠ eslint(no-invalid-regexp): Invalid regular expression: Character class atom range out of order
Expand Down
Loading