Skip to content

Implement RegExp v-flag (unicodeSets) syntax validation - #36

Merged
adams85 merged 6 commits into
adams85:masterfrom
lahma:feature/regexp-v-flag
Apr 7, 2026
Merged

Implement RegExp v-flag (unicodeSets) syntax validation#36
adams85 merged 6 commits into
adams85:masterfrom
lahma:feature/regexp-v-flag

Conversation

@lahma

@lahma lahma commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements syntax validation for the RegExp v flag (unicodeSets) as specified in TC39 proposal-regexp-v-flag, now ratified in ES2024. Closes #9.

  • Adds ES15/ES2024 to EcmaVersion enum (now Latest)
  • New UnicodeSetsMode with recursive class set expression parser supporting:
    • Set operations: intersection (&&), subtraction (--), union
    • Nested character classes: [[a-z]&&[aeiou]]
    • String disjunctions: \q{abc|def}
    • Binary properties of strings: \p{Basic_Emoji}, \p{RGI_Emoji}, etc.
  • Updates CheckBracesBalance to handle nested [...] in v-flag mode
  • Validation-only: conversion to .NET Regex is not supported for v-flag patterns (returns conversion error in AdaptTo* modes)
  • Reference implementation: acorn PR #1218 / commit fb4c582

Test plan

  • All existing Acornima.Tests pass (11,104 on net10.0, 0 failures across net8.0/net9.0/net10.0/net462)
  • 65 new v-flag unit tests added (valid patterns, invalid patterns, conversion failure, u+v mutual exclusion)
  • regexp-v-flag removed from Test262 ExcludedFeatures — all 98,118 Test262 tests pass with 0 failures
  • Benchmark validation to confirm no performance regression on existing regex parsing

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds ES2024 (v / unicodeSets) RegExp parsing support to Acornima by introducing a validation-only unicode-sets parser and expanding Unicode property validation to cover ES2024 “binary properties of strings”, along with new unit tests and enabling Test262 coverage for the feature.

Changes:

  • Add EcmaVersion.ES15 / ES2024 and treat it as Latest.
  • Implement UnicodeSetsMode with recursive parsing/validation for v-flag class set expressions, plus v-flag conversion refusal in AdaptTo* modes.
  • Extend Unicode properties to include ES2024 binary properties of strings and add v-flag-focused tests; enable Test262 regexp-v-flag.

Reviewed changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/Acornima.Tests/RegExpTests.cs Adds v-flag validation, conversion-failure, and u/v mutual exclusion tests.
test/Acornima.Tests.Test262/Test262Harness.settings.json Enables Test262 coverage by removing regexp-v-flag from excluded features.
src/Acornima/EcmaVersion.cs Adds ES15/ES2024 and updates Latest.
src/Acornima/UnicodeProperties.cs Adds ES2024 binary properties-of-strings lookup and validation API.
src/Acornima/Tokenizer.RegExpParser.cs Routes v-flag to UnicodeSetsMode; blocks conversion for v-flag; updates brace-balance scanning for nested sets; adds IMode.TryParseCharacterClass.
src/Acornima/Tokenizer.RegExpParser.UnicodeSets.cs New unicodeSets (v-flag) class set expression validator.
src/Acornima/Tokenizer.RegExpParser.Unicode.cs Refactors Unicode property validation helper usage and implements new IMode member.
src/Acornima/Tokenizer.RegExpParser.Legacy.cs Implements new IMode member.
src/Acornima/Properties/SyntaxErrorMessages.resx + Designer.cs Adds v-flag-related syntax error messages.
src/Acornima/Properties/RegExpConversionErrorMessages.resx + Designer.cs Updates v-flag conversion error wording.
Files not reviewed (2)
  • src/Acornima/Properties/RegExpConversionErrorMessages.Designer.cs: Language not supported
  • src/Acornima/Properties/SyntaxErrorMessages.Designer.cs: Language not supported

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

Comment thread src/Acornima/Tokenizer.RegExpParser.UnicodeSets.cs Outdated
Comment thread src/Acornima/Tokenizer.RegExpParser.UnicodeSets.cs Outdated
Comment thread src/Acornima/Properties/RegExpConversionErrorMessages.resx Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.

Files not reviewed (2)
  • src/Acornima/Properties/RegExpConversionErrorMessages.Designer.cs: Language not supported
  • src/Acornima/Properties/SyntaxErrorMessages.Designer.cs: Language not supported

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

Comment thread src/Acornima/Tokenizer.RegExpParser.UnicodeSets.cs Outdated
Comment thread src/Acornima/Tokenizer.RegExpParser.UnicodeSets.cs Outdated
@adams85

adams85 commented Apr 4, 2026

Copy link
Copy Markdown
Owner

Love it, thanks! ❤️

However, as a preparation step for implementing this, I did some refactoring around RegExpParser in the meantime, so we have some conflicts now. Could you ask Copilot to merge master and update the PR?

@lahma

lahma commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator Author

Already doing the rebase, no worries, I'm also running feedback loop and then marking this ready for review.

@lahma
lahma force-pushed the feature/regexp-v-flag branch 2 times, most recently from 341df19 to 7e074ad Compare April 4, 2026 09:06
@lahma
lahma requested a review from Copilot April 4, 2026 09:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.

Files not reviewed (2)
  • src/Acornima/Properties/RegExpConversionErrorMessages.Designer.cs: Language not supported
  • src/Acornima/Properties/SyntaxErrorMessages.Designer.cs: Language not supported

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

Comment thread src/Acornima/Tokenizer.RegExpParser.UnicodeSets.cs Outdated
Comment thread src/Acornima/Tokenizer.RegExpParser.UnicodeSets.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.

Files not reviewed (2)
  • src/Acornima/Properties/RegExpConversionErrorMessages.Designer.cs: Language not supported
  • src/Acornima/Properties/SyntaxErrorMessages.Designer.cs: Language not supported

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

Comment thread src/Acornima/Tokenizer.RegExpParser.cs
Comment thread src/Acornima/Tokenizer.RegExpParser.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.

Files not reviewed (2)
  • src/Acornima/Properties/RegExpConversionErrorMessages.Designer.cs: Language not supported
  • src/Acornima/Properties/SyntaxErrorMessages.Designer.cs: Language not supported

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

Comment thread src/Acornima/Tokenizer.RegExpParser.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.

Files not reviewed (2)
  • src/Acornima/Properties/RegExpConversionErrorMessages.Designer.cs: Language not supported
  • src/Acornima/Properties/SyntaxErrorMessages.Designer.cs: Language not supported

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

Comment thread src/Acornima/Tokenizer.RegExpParser.UnicodeSets.cs Outdated
Comment thread src/Acornima/Tokenizer.RegExpParser.UnicodeSets.cs Outdated
Comment thread src/Acornima/Tokenizer.RegExpParser.UnicodeSets.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.

Files not reviewed (2)
  • src/Acornima/Properties/RegExpConversionErrorMessages.Designer.cs: Language not supported
  • src/Acornima/Properties/SyntaxErrorMessages.Designer.cs: Language not supported

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

Comment thread test/Acornima.Tests/RegExpTests.cs

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.

Files not reviewed (2)
  • src/Acornima/Properties/RegExpConversionErrorMessages.Designer.cs: Language not supported
  • src/Acornima/Properties/SyntaxErrorMessages.Designer.cs: Language not supported

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

Comment thread src/Acornima/UnicodeProperties.cs
@lahma

lahma commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator Author

This loop starts to stabilize and now actionable items seem to rise. I'm running regex engine fixes on Jint side and asked for v-flag status (referencing this PR on Jint's side and depending on the changes):

All runtime failures (JavaScriptException), no parse failures (ScriptPreparationException) — Acornima parsing works fine. The issues are in our custom engine.

I'll hopefully soon mark this as ready for review, just some running with Jint context remaining..

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated no new comments.

Files not reviewed (2)
  • src/Acornima/Properties/RegExpConversionErrorMessages.Designer.cs: Language not supported
  • src/Acornima/Properties/SyntaxErrorMessages.Designer.cs: Language not supported

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

@lahma
lahma marked this pull request as ready for review April 4, 2026 12:34
@lahma

lahma commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator Author

@adams85 I've verified that this now works on Jint's side - all tests except unicode pass now, I'm checking the unicode minimal fix next. This PR is ready from my side, feel free to use/change how you will.

Add 11 new scripts from Unicode 17.0 with their 4-letter aliases:
Beria_Erfe, Garay, Gurung_Khema, Kirat_Rai, Ol_Onal, Sidetic,
Sunuwar, Tai_Yo, Todhri, Tolong_Siki, Tulu_Tigalari.

This unblocks 44 test262 tests (22 Script + 22 Script_Extensions)
that were excluded because the parser rejected these names at
parse time.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@lahma

lahma commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator Author

I added Unicode 17.0 script names to the property escape validation in my fork (lahma/acornima@0197c29). This is a small addition — just 11 new scripts with their 4-letter aliases added to s_scriptValueLookup in UnicodeProperties.cs.

This unblocks 44 test262 tests (22 Script + 22 Script_Extensions) that were failing because the parser rejected names like Garay, Todhri, Beria_Erfe etc. at parse time. Seems like a good small addition to include in this regex PR.

Scripts added: Beria_Erfe, Garay, Gurung_Khema, Kirat_Rai, Ol_Onal, Sidetic, Sunuwar, Tai_Yo, Todhri, Tolong_Siki, Tulu_Tigalari.

@adams85

adams85 commented Apr 4, 2026

Copy link
Copy Markdown
Owner

I've verified that this now works on Jint's side - all tests except unicode pass now, I'm checking the unicode minimal fix next. This PR is ready from my side, feel free to use/change how you will.

Awesome, this is a dream coming true! 😄

I'll review and finalize the PR soon and probably release a new version before extracting the conversion logic into a separate package (so we can move step by step and coordinate our efforts more easily).

I added Unicode 17.0 script names to the property escape validation in my fork (lahma@0197c29). This is a small addition — just 11 new scripts with their 4-letter aliases added to s_scriptValueLookup in UnicodeProperties.cs.

This data is necessary for validation so it's perfectly okay to add.

Thank you very much!

@lahma

lahma commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator Author

The Jint PR already sets Acornima only to validate, not adapt - all problematic are directed to the (slower) custom engjne. So it aligns with your idea of removing the feature- maybe mark option obsolete?

@adams85

adams85 commented Apr 4, 2026

Copy link
Copy Markdown
Owner

👍 I'm going to do that.

The current conversion logic will still be available separate from Acornima for anyone who needs it, I just don't plan to put any more effort into it.

@adams85
adams85 force-pushed the feature/regexp-v-flag branch 2 times, most recently from c6c9875 to a7c5b93 Compare April 7, 2026 22:02
@adams85
adams85 force-pushed the feature/regexp-v-flag branch from a7c5b93 to 550a03a Compare April 7, 2026 22:12
@adams85

adams85 commented Apr 7, 2026

Copy link
Copy Markdown
Owner

The source file implementing the Unicode sets mode turned out to be exactly 666 lines long... This cannot be a coincidence. Now I'm sure that flag v is the work of the devil himself! 😈

@adams85
adams85 merged commit 7ea64b5 into adams85:master Apr 7, 2026
3 checks passed
@lahma
lahma deleted the feature/regexp-v-flag branch April 8, 2026 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement validation of unicode sets (flag v) regexps

3 participants