Closed Ubugeeei closed 1 week ago
Let's do this incrementally with proper testing, we can cherry-pick some of the code from Maneren's branch
My requirements:
oxc_parser
, i.e. allocate the AST on oxc_allocator
for maximum performance, have lexer / parser phase for maximum code control@Ubugeeei Nice work! I hope your are learning a lot.
Hello, I saw it has been 3 weeks since your last pr, are you still interested in this feature, or can I continue your work?
@Ubugeeei is too busy with other stuff (vapor mode I guess?). Feel free to assign this to yourself @IWANABETHATGUY
@IWANABETHATGUY
Hello, I saw it has been 3 weeks since your last pr, are you still interested in this feature, or can I continue your work?
Ah, sorry, I completely missed this comment. I was planning to work on it when I had time, if it was still pending, but I haven't been able to do anything yet, so please go ahead, there's no problem at all.
(My time bottleneck is my main job... Actually, Vapor often has discussions that are put on hold, so it's not that busy yet..)
This seems like the kind of self-inflicted torture that I'd enjoy. May I have a go at moving it forward?
@IWANABETHATGUY have you started working on this?
@IWANABETHATGUY have you started working on this?
WIP
@maurice it seems like @IWANABETHATGUY is working on it, I'll let him coordinate the tasks.
I have quick investigated the current status of this issue and the rules related to @eslint-community/regexpp
, so I am leaving a NOTE...
regexpp
ESLint uses an ECMAScript RegExp parser implementation called @eslint-community/regexpp
.
There are currently 10 rules implemented that depend on it.
u
flag, \q{}
with v
flagparsePattern()
, onCharacterClassEnter()
validatePattern()
, validateFlags()
parsePattern()
, onBackreferenceEnter()
parsePattern()
, onCapturingGroupEnter()
/abc/i
over new RegExp("abc", "i")
validatePattern()
, validateFlags()
, onCharacterEnter()
u
or v
flagsvalidatePattern()
onCharacter()
/a[]/
onCharacterClassEnter()
parsePattern()
, onCharacterClass(Enter|Leave)()
, onExpressionCharacterClass(Enter|Leave)()
, onCharacterEnter()
parsePattern()
, onCharacterClassEnter()
And there is a related plugin ota-meshi/eslint-plugin-regexp too. / #3263
oxlint
regex
crateregex
crateBut it should be re-implemented with regexpp
for better performance.
regexpp
port for Rust #3824 oxlint
Linting regexp may be doable without a parser, but it'll definitely be easier to lint by visiting an AST.
I still want to regex parser for oxc for maximum performance gains 😅 We should only parse the regex once and visit the AST once for linting.
There completeness, there is also https://github.com/ota-meshi/eslint-plugin-regexp from the same author.
We should only parse the regex once and visit the AST once
Ah~, I see, that makes sense! I've updated my comment.
I didn't know this... And this looks also... challenging! 😂
@leaysgur The current blocking task is the parser, we can always distribute the linter rules to future contributors. You don't need to implement the linter rules like you did with the jsdoc plugins 😅
Yes, I understand. I have finally grasped the situation now. 👍🏻
you did with the jsdoc plugins
🙈
Now, I will tackle the implementation of the parser. (Since it seems like many people are actively developing the playground. 👀 )
I'm going to read the regexpp
implementation and to spend a few days for the oxc_parser
architecture and #2030.
@Boshen
I'm going through the code and written a bit myself, then I'd like to confirm a few things.
In the original implementation of regexpp
, it seems to handle characters directly without using Tokens.
To create something that works for now, I'm thinking of following the original approach and not implementing a Lexer.
What do you think?
These options seem to significantly affect behavior and implementation. What should we do about them? One option might be to always support only the latest version... 🤔
Is a Lexer necessary?
Probably not, since there is no whitespace or semi colons like JavaScript.
We can try one without a lexer.
Support for strict mode (annexB) and various ecmaVersions?
We always support the latest ecma version, just like our parser.
For strict mode, you may leave them out from your first version.
OK, thanks! 🙏 I'll try it minimally for the first step.
@leaysgur Do you intend to start from scratch? You can start from a new branch by coping over some of the code from https://github.com/oxc-project/oxc/pull/2030
Once you have something, I'll help you with the test infrastructure - running the regex parser on all regexes from test262, babel and typescript.
I pushed current progress #3824 , of course it is WIP... 😴
Do you intend to start from scratch?
Maybe yes. But I will refer to or pick up on them as necessary.
Currently, they are in /_oxc_js_regex
dir, just for reference.
I'll help you with the test infrastructure - running the regex parser on all regexes from test262, babel and typescript.
That is very helpful! I was just thinking about what to do. 😅
Progress update:
Implementation is ongoing. 🚧 #3824
Most of the syntax is implemented, except for [character_class]
and v
flag related stuff.
But there's still some work to complete.(Early errors, test262 tests and integration to Linter, etc...)
While working on this, I went through the specification and the regexpp
implementation several times.
As a result, I realized that the regexpp
implementation may not be fully compliant with the specification.
Also, for ease of rewriting in Rust, it cannot be implemented as a complete 1-to-1 copy. (since it relies heavily on the dynamic nature of JavaScript)
For these reasons, I'm thinking of abandoning original goal of re-implementing regexpp
in Rust and focusing on implementing it as an original specification-compliant parser.
(It may not have cared about compatibility in the first place, though 🤔 )
Fortunately or unfortunately, there are no de-facts like ESTree, and I think there are any problems, but just in case, I'll look at the specific usecases of the Lint rules that depend on regexpp
first.
I'll look at the specific usecases of the Lint rules that depend on
regexpp
first.
All usecase were just as expected. I updated my comment above. ⬆️
and focusing on implementing it as an original specification-compliant parser
👍 always bet on the spec!
Progress update:
Finally completed all ES2024 specs! 🎉
However, we noticed that while it supports the literal pattern /(.)\1/
, it misbehaves the pattern for new RegExp("(.)\\1")
whose strings are escaped...
In JavaScript, \\
is treated as \
. But in Rust, \\
is \\
.
Now I'm thinking how to cope with this. Perhaps a pre-treatment, such as a lexer for RegExp parser is required?
RegExp parser is now integrated in oxc_parser
itself. It emits error when invalid RegExp literal is found.
But how to use parsed result in user land like linter is still considering.
@Boshen Could you please take a look these and give your feedback if you have time? 🙏🏻
Progress update:
Implementation part
3824
Finally completed all ES2024 specs! 🎉
However, we noticed that while it supports the literal pattern
/(.)\1/
, it misbehaves the pattern fornew RegExp("(.)\\1")
whose strings are escaped...In JavaScript,
\\
is treated as\
. But in Rust,\\
is\\
.Now I'm thinking how to cope with this. Perhaps a pre-treatment, such as a lexer for RegExp parser is required?
Integration part
4242
RegExp parser is now integrated in
oxc_parser
itself. It emits error when invalid RegExp literal is found.But how to use parsed result in user land like linter is still considering.
@Boshen Could you please take a look these and give your feedback if you have time? 🙏🏻
Thank you for the tremendous effort! I'll take a look at the problems soon.
Progress update:
oxc_regular_expression
crate is finally merged into main
branch ✨ oxc_parser
can emit syntax error if invalid regex literals are found
/literals/v
is checked, RegExp("string", "v")
is not checkedHowever, to officially use parsed AST in linter, there is a little more to do.
e.g. #5060 , how to use it with RegExp("string")
?, etc.
As for the remaining tasks, we can address them in a separate issue. But for now, can't we close this long-lived issue? 😃
Thank you again @leaysgur, and also everyone who participated in this. Thank you all!
Implement a regular expression parser equivalent to @eslint-community/regexpp.
Todo:
Ref: https://github.com/web-infra-dev/oxc/issues/611