Skip to content

Support for v flag & update regexpp #545

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

Closed
15 tasks done
ota-meshi opened this issue Jul 22, 2023 · 4 comments · Fixed by #643
Closed
15 tasks done

Support for v flag & update regexpp #545

ota-meshi opened this issue Jul 22, 2023 · 4 comments · Fixed by #643
Labels
enhancement New feature or request

Comments

@ota-meshi
Copy link
Owner

ota-meshi commented Jul 22, 2023

Description

regexpp added support for the v flag.
Since the AST has changed along with this, many Type errors have occurred. we need to fix them.
Also, some rules need to be fixed to understand the v flag.


At least the following TS errors should be fixed.

  • regexp/negation rule
  • regexp/no-contradiction-with-assertion rule
  • regexp/no-dupe-characters-character-class rule
  • regexp/no-dupe-disjunctions rule
  • regexp/no-misleading-capturing-group rule
  • regexp/optimal-quantifier-concatenation rule
  • regexp/prefer-character-class rule
  • regexp/sort-character-class-elements rule
  • regexp/use-ignore-case rule
  • lib/utils/index.ts
  • lib/utils/partial-parser.ts
  • lib/utils/regexp-ast/case-variation.ts
  • lib/utils/regexp-ast/index.ts
  • lib/utils/regexp-ast/is-equals.ts
  • lib/utils/regexp-ast/simplify-quantifier.ts
@RunDevelopment
Copy link
Collaborator

Fixing the type error won't be enough. refa and RAA can't deal with the v flag at all right now. I'll try to add support to both today.

@RunDevelopment
Copy link
Collaborator

Quick update: I have implemented support for the v flag in refa and RAA. This took a lot longer than expected. It's not released yet, because I still want to test it some more.

Also, while implementing this, the full scope of what we'll have to do dawned on me. We'll probably have to update all rules that interact with characters classes/sets in any way, not just the ones with type errors.

@ota-meshi
Copy link
Owner Author

ota-meshi commented Oct 1, 2023

In my opinion the no-empty-alternative rule should report empty alternatives. e.g. /[\q{a|}]/v
Presumably, if we support it, we can say that the existing rules supported the v flag. What do you think?

The following rules currently do not have test cases for the v flag:

  • confusing-quantifier
  • control-character-escape
  • hexadecimal-escape
  • letter-case
  • match-any
  • no-control-character
  • no-empty-alternative
  • no-empty-capturing-group
  • no-empty-character-class
  • no-empty-group
  • no-empty-lookarounds-assertion
  • no-escape-backspace
  • no-extra-lookaround-assertions
  • no-invalid-regexp
  • no-invisible-character
  • no-lazy-ends
  • no-legacy-features
  • no-missing-g-flag
  • no-obscure-range
  • no-octal
  • no-optional-assertion
  • no-potentially-useless-backreference
  • no-standalone-backslash
  • no-super-linear-backtracking
  • no-super-linear-move
  • no-trivially-nested-assertion
  • no-trivially-nested-quantifier
  • no-unused-capturing-group
  • no-useless-assertions
  • no-useless-backreference
  • no-useless-dollar-replacements
  • no-useless-flag
  • no-useless-lazy
  • no-useless-non-capturing-group
  • no-useless-quantifier
  • no-useless-range
  • no-useless-two-nums-quantifier
  • no-zero-quantifier
  • optimal-lookaround-quantifier
  • prefer-escape-replacement-dollar-char
  • prefer-lookaround
  • prefer-named-backreference
  • prefer-named-capture-group
  • prefer-named-replacement
  • prefer-plus-quantifier
  • prefer-quantifier
  • prefer-question-quantifier
  • prefer-range
  • prefer-regexp-exec
  • prefer-regexp-test
  • prefer-result-array-groups
  • prefer-star-quantifier
  • sort-flags

By the way, do you think we should add a rule to report empty string disjunctions like /[\q{}]/v? I think perhaps it can be replaced by using ? after the character class. Do you think there are any benefits to /[\q{}]/v that I haven't thought of?

@RunDevelopment
Copy link
Collaborator

In my opinion the no-empty-alternative rule should report empty alternatives. e.g. /[\q{a|}]/v
Presumably, if we support it, we can say that the existing rules supported the v flag. What do you think?

Agreed.

By the way, do you think we should add a rule to report empty string disjunctions like /[\q{}]/v? I think perhaps it can be replaced by using ? after the character class. Do you think there are any benefits to /[\q{}]/v that I haven't thought of?

I think so too. Using a simple ? is a lot more understandable than having a \q{} somewhere inside a character class IMO. Every regex novice knows what x? does, but \q{} is a lot less clear for beginners. Using a quantifier also means that our other rules about quantifiers can potentially simplify the regex futher (e.g. /(?:[ab\q{}])+/v -> /(?:[ab]?)+/v -> /[ab]*/v).

That being said, I think that this should be its own rule, and not part of no-empty-alternative. It's similar to no-empty-group in that regard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants