Skip to content

Conversation

@RunDevelopment
Copy link
Collaborator

This adds the logic I was talking about here. Many reports are fixable. Only reports that are likely a but are not fixable.

However, there is still one problem with this: the tests don't pass because RegExpValidator says that /(?<foo>)\k<foo>/ isn't valid. I have no idea why it rejects that regex. Do you know why @ota-meshi?

@RunDevelopment RunDevelopment requested a review from ota-meshi June 2, 2021 14:27
@ota-meshi
Copy link
Owner

/(?<foo>)\k<foo>/ isn't valid

I think it's probably an invalid regular expression because of backward compatibility.

However, the named backreference syntax, /\k<foo>/, is currently permitted in non-Unicode RegExps and matches the literal string "k<foo>". In Unicode RegExps, such escapes are banned.

https://github.com/tc39/proposal-regexp-named-groups#backwards-compatibility-of-new-syntax

I'm not sure if it's valid as a strict non-Unicode regular expression.

@RunDevelopment
Copy link
Collaborator Author

RunDevelopment commented Jun 3, 2021

v8 will happily execute that regex. Example:

Node.js v14.15.4.
> /^(?<foo>A)\k<foo>$/.test("AA")
true

It should be valid because:

In this proposal, \k<foo> in non-Unicode RegExps will continue to match the literal string "k<foo>" unless the RegExp contains a named group, in which case it will match that group or be a syntax error [...]

And I think, I figured out why regexpp throws an error. It's a bug in regexpp's validator: mysticatea/regexpp#23.

Now we have two option:

  1. Wait until the bug is fixed.
  2. Publish regexp/strict without RegExpValidator and re-add it later when the bug is fixed.

Which one do we want to choose?

@ota-meshi
Copy link
Owner

I heard a rumor that mysticatea seems to be busy these days and doesn't have time to maintain OSS.
I don't know if he will reply, but I will try to send him a message via chat.

@RunDevelopment
Copy link
Collaborator Author

I see. Thank you @ota-meshi!

In that case, I'll disable the RegExpValidator-based check for patterns with named backreferences. We can re-add it as soon as the bug is fixed.

Copy link
Owner

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

Thank you very much!

@ota-meshi ota-meshi merged commit 637ee67 into strict Jun 4, 2021
@ota-meshi ota-meshi deleted the strict-improvements branch June 4, 2021 02:46
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.

3 participants