Skip to content

fix(minifier): validate RegExp patterns before marking as pure#18125

Merged
graphite-app[bot] merged 1 commit intomainfrom
fix/minifier-regexp-validation
Jan 18, 2026
Merged

fix(minifier): validate RegExp patterns before marking as pure#18125
graphite-app[bot] merged 1 commit intomainfrom
fix/minifier-regexp-validation

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Jan 17, 2026

Summary

Invalid RegExp patterns like RegExp("[") or invalid flags like RegExp("a", "xyz") throw SyntaxError at runtime, but the minifier was incorrectly removing them.

This change uses oxc_regular_expression to validate patterns at compile time:

  • Valid patterns are marked as pure and can be removed when unused
  • Invalid patterns or non-literal arguments are kept (preserving runtime errors)

Behavior

Input Valid? Removed?
RegExp()
RegExp('a', 'g')
RegExp(/foo/)
RegExp('[') ❌ (throws SyntaxError)
RegExp('a', 'xyz') ❌ (throws SyntaxError)
RegExp(variable) ? ❌ (can't determine)

Fixes #18050

Test plan

  • Updated existing tests to reflect correct behavior
  • Added tests for invalid patterns and flags
  • cargo test -p oxc_minifier passes
  • cargo test -p oxc_ecmascript passes

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings January 17, 2026 08:31
@github-actions github-actions bot added A-minifier Area - Minifier C-bug Category - Bug labels Jan 17, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug where the minifier incorrectly removed invalid RegExp patterns that should throw SyntaxError at runtime. The fix uses the oxc_regular_expression parser to validate patterns and flags at compile time, ensuring that only valid RegExp constructors are marked as pure and can be safely removed.

Changes:

  • Added is_valid_regexp function that validates RegExp patterns and flags using the regex parser
  • Updated minifier's normalize pass to use validation for RegExp constructor calls
  • Updated tests to reflect correct behavior for valid/invalid patterns

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/oxc_ecmascript/src/side_effects/expressions.rs Implements is_valid_regexp validation function using the regex parser
crates/oxc_ecmascript/src/side_effects/mod.rs Exports the new is_valid_regexp function
crates/oxc_minifier/src/peephole/normalize.rs Integrates RegExp validation in the normalize pass for NewExpression
crates/oxc_ecmascript/Cargo.toml Adds oxc_regular_expression dependency
Cargo.lock Updates lock file with new dependency
crates/oxc_minifier/tests/peephole/substitute_alternate_syntax.rs Updates tests with correct behavior for valid/invalid RegExp patterns
crates/oxc_minifier/tests/peephole/normalize.rs Updates tests to handle non-literal RegExp arguments correctly
crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs Adds tests for invalid patterns and flags

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Boshen Boshen force-pushed the fix/minifier-regexp-validation branch from b0db38f to 2e4911a Compare January 17, 2026 08:36
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 17, 2026

Merging this PR will not alter performance

✅ 42 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing fix/minifier-regexp-validation (38e4b53) with main (91126a0)2

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (2ae523c) during the generation of this report, so 91126a0 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@Boshen Boshen force-pushed the fix/minifier-regexp-validation branch from 2e4911a to 8563299 Compare January 17, 2026 08:48
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jan 18, 2026
Copy link
Member Author

Boshen commented Jan 18, 2026

Merge activity

  • Jan 18, 1:47 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jan 18, 1:47 AM UTC: Boshen added this pull request to the Graphite merge queue.
  • Jan 18, 1:58 AM UTC: The Graphite merge queue couldn't merge this PR because it was not satisfying all requirements (Failed CI: 'Conformance').
  • Jan 18, 4:28 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jan 18, 4:30 AM UTC: Boshen added this pull request to the Graphite merge queue.
  • Jan 18, 4:31 AM UTC: Merged by the Graphite merge queue.

graphite-app bot pushed a commit that referenced this pull request Jan 18, 2026
## Summary

Invalid RegExp patterns like `RegExp("[")` or invalid flags like `RegExp("a", "xyz")` throw `SyntaxError` at runtime, but the minifier was incorrectly removing them.

This change uses `oxc_regular_expression` to validate patterns at compile time:
- Valid patterns are marked as pure and can be removed when unused
- Invalid patterns or non-literal arguments are kept (preserving runtime errors)

### Behavior

| Input | Valid? | Removed? |
|-------|--------|----------|
| `RegExp()` | ✅ | ✅ |
| `RegExp('a', 'g')` | ✅ | ✅ |
| `RegExp(/foo/)` | ✅ | ✅ |
| `RegExp('[')` | ❌ | ❌ (throws SyntaxError) |
| `RegExp('a', 'xyz')` | ❌ | ❌ (throws SyntaxError) |
| `RegExp(variable)` | ? | ❌ (can't determine) |

Fixes #18050

## Test plan

- [x] Updated existing tests to reflect correct behavior
- [x] Added tests for invalid patterns and flags
- [x] `cargo test -p oxc_minifier` passes
- [x] `cargo test -p oxc_ecmascript` passes

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app bot force-pushed the fix/minifier-regexp-validation branch from 8563299 to 64e73a5 Compare January 18, 2026 01:52
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 18, 2026
## Summary

Invalid RegExp patterns like `RegExp("[")` or invalid flags like `RegExp("a", "xyz")` throw `SyntaxError` at runtime, but the minifier was incorrectly removing them.

This change uses `oxc_regular_expression` to validate patterns at compile time:
- Valid patterns are marked as pure and can be removed when unused
- Invalid patterns or non-literal arguments are kept (preserving runtime errors)

### Behavior

| Input | Valid? | Removed? |
|-------|--------|----------|
| `RegExp()` | ✅ | ✅ |
| `RegExp('a', 'g')` | ✅ | ✅ |
| `RegExp(/foo/)` | ✅ | ✅ |
| `RegExp('[')` | ❌ | ❌ (throws SyntaxError) |
| `RegExp('a', 'xyz')` | ❌ | ❌ (throws SyntaxError) |
| `RegExp(variable)` | ? | ❌ (can't determine) |

Fixes #18050

## Test plan

- [x] Updated existing tests to reflect correct behavior
- [x] Added tests for invalid patterns and flags
- [x] `cargo test -p oxc_minifier` passes
- [x] `cargo test -p oxc_ecmascript` passes

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@Boshen Boshen force-pushed the fix/minifier-regexp-validation branch from 64e73a5 to 38e4b53 Compare January 18, 2026 04:24
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jan 18, 2026
@graphite-app graphite-app bot merged commit 38e4b53 into main Jan 18, 2026
30 checks passed
@graphite-app graphite-app bot deleted the fix/minifier-regexp-validation branch January 18, 2026 04:31
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

minifier: regex constructor call with invalid pattern is removed incorrectly

3 participants