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.

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

feat(linter): eslint/no-invalid-regexp #611

Open
Tracked by #479
DonIsaac opened this issue Jul 25, 2023 · 12 comments
Open
Tracked by #479

feat(linter): eslint/no-invalid-regexp #611

DonIsaac opened this issue Jul 25, 2023 · 12 comments
Labels
A-linter Area - Linter good first issue Experience Level - Good for newcomers

Comments

@DonIsaac
Copy link
Collaborator

DonIsaac commented Jul 25, 2023

Implement ESLint's no-invalid-regexp rule.

To get started, run

just new-rule no-invalid-regexp
@DonIsaac DonIsaac changed the title no-invalid-regexp feat(linter): eslint/no-invalid-regexp Jul 25, 2023
@DonIsaac DonIsaac added good first issue Experience Level - Good for newcomers A-linter Area - Linter labels Jul 25, 2023
@Ubugeeei
Copy link
Contributor

Ubugeeei commented Nov 5, 2023

Hi. To implement this rule, it seems necessary to implement a parser like @eslint-community/regexpp.

Where is this planned to be implemented?
If it's within this repository, please let me know where in the repository it should be implemented.

@Boshen
Copy link
Member

Boshen commented Nov 5, 2023

Hi. To implement this rule, it seems necessary to implement a parser like @eslint-community/regexpp.

Where is this planned to be implemented? If it's within this repository, please let me know where in the repository it should be implemented.

We don't have a regex parser yet but it's open for contributions by veterans like you! If you are interested in learning Rust by implementing a regex parser, please reach out and I'll guide you. To get started, we have a small tutorial here where some of the chapters are already translated into Japanese 😁 https://oxc-project.github.io/javascript-parser-in-rust/ja/

@Ubugeeei
Copy link
Contributor

Ubugeeei commented Nov 5, 2023

Oh, thank you for the detailed information.
I've also been busy with other things in the past few days, so it might take some time, but I'm thinking of trying to implement a prototype of the Rust rewrite of @eslint-community/regexpp in my repository.
I might propose integrating it somewhere at some point.

To get started, we have a small tutorial here where some of the chapters are already translated into Japanese 😁 https://oxc-project.github.io/javascript-parser-in-rust/ja/

The Japanese version is great! As a native speaker, I might be able to contribute to the translation into Japanese.

@Ubugeeei
Copy link
Contributor

Ubugeeei commented Nov 5, 2023

Oops, I forgot to mention one thing.
I will try to implement the prototype, but if anyone reading this comment wants to work on this issue,
please don't hesitate to start. If your progress is faster, I will discard my project.
(Just because you've read this comment doesn't mean you need to stop your work.)

@Boshen
Copy link
Member

Boshen commented Nov 5, 2023

Oops, I forgot to mention one thing. I will try to implement the prototype, but if anyone reading this comment wants to work on this issue, please don't hesitate to start. If your progress is faster, I will discard my project. (Just because you've read this comment doesn't mean you need to stop your work.)

No no, we can merge small pieces and collectively iterate on it so no effort is lost. Just send in small PRs. You may wish to try https://graphite.dev for such tasks.

@Ubugeeei
Copy link
Contributor

Ubugeeei commented Nov 5, 2023

Thank you. For now, I'll give a small prototype a try.
Once I can see a roadmap of the smaller tasks I've broken down, I'll share it again!

You may wish to try https://graphite.dev/ for such tasks.

Oh, there's a tool like this? Thanks for introducing it to me. I didn't know about it, so I'll take a closer look!

@Maneren
Copy link
Contributor

Maneren commented Nov 5, 2023

Hi, I tried coding a prototype regex parser some time ago in my fork. It can (afaik correctly) parse all regexes I found in the MDN docs. The main issues are imo the lack of a lexer (which would help with some backslash edge cases and string-vs-literal normalization) and messy output when dealing with unicode.

I temporarily gave up on it due to encountering too many small tedious issues (escaping, spans, output, ...) and then later also starting projects for school. I'd like to continue working on it in around two weeks, once I'll hopefully finish the schoolwork.

Feel free to either pick it up in the meantime or take inspiration for your version. I can clean it up tomorrow and possibly PR it in here.

(and sorry, I should have written somewhere earlier that I was working on it)

@Ubugeeei
Copy link
Contributor

Ubugeeei commented Nov 6, 2023

I see. I didn't know that.
In that case, I'll refrain from implementing the prototype.
Let's prioritize and respect your achievements.

I have a suggestion: since you've made so much progress, why not create a feature branch in this repository for the regex-parser so everyone can collaborate there? And for issues related to this parser, it might be a good idea to leave documentation and address remaining concerns as separate issues. What do you think?
(I haven't looked closely at the Maneren branch, so I'm not sure if it can be integrated into oxc.)

In any case, it might be better not to have too many significant changes in the forked repository and merge them into the feature branch of this repository while sharing issues with everyone.

cc @Boshen

@Ubugeeei
Copy link
Contributor

Ubugeeei commented Nov 6, 2023

At the very least, since this issue originally pertained to eslint/no-invalid-regexp, I've created a separate issue for the regular expression parser.
I would like to conduct discussions related to the parser here.
How do you feel about that proposal?

#1164

@Maneren
Copy link
Contributor

Maneren commented Nov 6, 2023

I agree. Upon closer inspection (as in remembering what I did then) the branch can't be directly integrated to oxc since it also overwrites a linter rule for testing, making it unusable for now. Also the code structure isn't great as it was created hastily without preplanning and doesn't use existing oxc infra. That means we are probably better off starting from scratch, just reusing the parsing logic, which is imo ok.

@opiredev
Copy link

opiredev commented May 1, 2024

@nabby27 created a $50.00 reward using Opire

How to earn this reward?

Since this project does not have Opire bot installed yet 😞 you need to go to Opire and claim the rewards through your programmer's dashboard once you have your PR ready 💪

@nabby27
Copy link

nabby27 commented May 1, 2024

I created this reward to encourage this issue to be resolved as it hasn't received updates in a long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter good first issue Experience Level - Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants