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

Do not throw error immediately when receiving null Regex from Esprima #931

Merged
merged 4 commits into from Jul 18, 2021

Conversation

KurtGokhan
Copy link
Contributor

@KurtGokhan KurtGokhan commented Jul 17, 2021

Sometimes Esprima will return null Regex. That means parsing did not fail, but the Regex is incompatible with C#.

With this PR, Jint will not fail during the parsing when such a Regex is received. It will only fail when that Regex is actually used. This way the user can handle the error, and also error can be avoided if that Regex is not used at all.

See also: sebastienros/esprima-dotnet#173

Keeping this as draft until Esprima PR is merged and released. (Test will fail without the new Esprima)

@lahma
Copy link
Collaborator

lahma commented Jul 18, 2021

@KurtGokhan 2.0.2 now available on NuGet.

@lahma
Copy link
Collaborator

lahma commented Jul 18, 2021

Will this resolve #506 ?

@lahma
Copy link
Collaborator

lahma commented Jul 18, 2021

I've updated main to have 2.0.2 as part of small bug fixing, new lib was required to get some test suite JS parsing working.

@KurtGokhan
Copy link
Contributor Author

@lahma that Regex is from stylis, which is the library I also noticed this issue.

This PR will not fix the issue. But at least the parsing will pass. The regex will still fail on runtime if it is executed. In my case the regex is never executed so it works for me.

code.Replace("[^]", "(?:.|\r|\n)") is a legit fix for this case. But as I said, if we apply this hack blindly, we may break already valid regex like [[^].

@KurtGokhan KurtGokhan marked this pull request as ready for review July 18, 2021 11:45
@lahma
Copy link
Collaborator

lahma commented Jul 18, 2021

Cool, thanks for the clarification. / My UI wasn't updating, thanks for the fix!

@lahma lahma merged commit e752354 into sebastienros:main Jul 18, 2021
@KurtGokhan KurtGokhan deleted the regex-parsing branch July 18, 2021 12:49
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.

None yet

2 participants