feat(regular_expression): improve error messages#16953
feat(regular_expression): improve error messages#16953graphite-app[bot] merged 1 commit intomainfrom
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
CodSpeed Performance ReportMerging #16953 will not alter performanceComparing Summary
Footnotes
|
29407b9 to
c70e2c8
Compare
966eea2 to
6d2d18c
Compare
6d2d18c to
b068bbf
Compare
51f0fa2 to
10bef7f
Compare
10bef7f to
6ba13c3
Compare
b068bbf to
bac65ed
Compare
6ba13c3 to
6642764
Compare
bac65ed to
5a2af88
Compare
6642764 to
b6368a6
Compare
2b94d9e to
9d8630a
Compare
There was a problem hiding this comment.
Pull request overview
This PR enhances error messages for regular expression parsing failures by adding helpful, actionable guidance to diagnostic messages. The changes improve developer experience by providing specific suggestions for fixing regex errors, such as explaining valid flags, suggesting fixes for common mistakes, and showing available group names for invalid references.
Key changes:
- Added help messages to all regex parser diagnostics with specific, actionable guidance
- Enhanced reference error messages to show available groups/names
- Improved error specificity by splitting generic errors into more targeted ones (e.g., duplicated vs invalid modifiers)
Reviewed changes
Copilot reviewed 7 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
crates/oxc_regular_expression/src/diagnostics.rs |
Added help messages to all diagnostic functions with actionable guidance for fixing regex errors |
crates/oxc_regular_expression/src/parser/pattern_parser/pattern_parser_impl.rs |
Updated to pass context (group counts, available names) to diagnostics for better error messages |
crates/oxc_regular_expression/src/parser/flags_parser.rs |
Exported constants for valid flags and modifiers to use in help messages |
crates/oxc_regular_expression/src/parser/mod.rs |
Re-exported flags/modifiers constants for use in diagnostics |
crates/oxc_regular_expression/tests/diagnostics.rs |
Added test cases for new error scenarios with better context |
| Snapshot files | Updated to reflect new help messages in diagnostic output |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
leaysgur
left a comment
There was a problem hiding this comment.
Good work as always. 🕺🏻
I have 2 points:
- Want to keep
diagnostics()a pure function- Pass
VALID_XXXas an argument only where necessary
- Pass
- Use
VALID_XXXin the implementation as well- Some code changes might be needed, but I want to avoid double definitions
Does it make sense?
|
Would you elaborate what you mean by |
use xxx::VALID_FLAGS;
#[cold]
pub fn unknown_flag(span: Span, flag: &str) -> OxcDiagnostic {
OxcDiagnostic::error(format!("{PREFIX} Unknown flag: `{flag}` found"))
.with_label(span)
.with_help(format!("Valid flags are: {}", VALID_FLAGS.join(", ")))
}⬇️ #[cold]
pub fn unknown_flag(span: Span, flag: &str, valid_flags) -> OxcDiagnostic {
OxcDiagnostic::error(format!("{PREFIX} Unknown flag: `{flag}` found"))
.with_label(span)
.with_help(format!("Valid flags are: {}", valid_flags.join(", ")))
}However, this is nits. 😅 |
|
Ah, I see. |
ea162dd to
3804102
Compare
crates/oxc_regular_expression/src/parser/pattern_parser/pattern_parser_impl.rs
Outdated
Show resolved
Hide resolved
3804102 to
6e13eda
Compare
Merge activity
|
Improved error messages for regex parse failures.
6e13eda to
3e2ae7b
Compare
|
I've just noticed that our release flow is now running. I'll add the merge label again once the release has finished. |

Improved error messages for regex parse failures.