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

Change config and default #49

Merged
merged 2 commits into from
Aug 13, 2021
Merged

Change config and default #49

merged 2 commits into from
Aug 13, 2021

Conversation

ota-meshi
Copy link
Owner

@ota-meshi ota-meshi commented Mar 13, 2021

  • Change plugin:regexp/recommended config

    • Add regexp/confusing-quantifier rule.
    • Add regexp/control-character-escape rule.
    • Add regexp/negation rule.
    • Add regexp/no-dupe-disjunctions rule.
    • Add regexp/no-empty-alternative rule.
    • Add regexp/no-empty-capturing-group rule.
    • Add regexp/no-invalid-regexp rule.
    • Add regexp/no-lazy-ends rule.
    • Add regexp/no-legacy-features rule.
    • Add regexp/no-non-standard-flag rule.
    • Add regexp/no-obscure-range rule.
    • Add regexp/no-optional-assertion rule.
    • Add regexp/no-potentially-useless-backreference rule.
    • Add regexp/no-standalone-backslash rule.
    • Add regexp/no-super-linear-backtracking rule.
    • Add regexp/no-trivially-nested-assertion rule.
    • Add regexp/no-trivially-nested-quantifier rule.
    • Add regexp/no-unused-capturing-group rule.
    • Add regexp/no-useless-assertions rule.
    • Add regexp/no-useless-character-class rule.
    • Add regexp/no-useless-dollar-replacements rule.
    • Add regexp/no-useless-escape rule.
    • Add regexp/no-useless-flag rule.
    • Add regexp/no-useless-lazy rule.
    • Add regexp/no-useless-non-capturing-group rule.
    • Add regexp/no-useless-quantifier rule.
    • Add regexp/no-useless-range rule.
    • Add regexp/no-zero-quantifier rule.
    • Add regexp/optimal-lookaround-quantifier rule.
    • Add regexp/optimal-quantifier-concatenation rule.
    • Add regexp/prefer-character-class rule.
    • Add regexp/prefer-predefined-assertion rule.
    • Add regexp/prefer-range rule.
    • Add regexp/prefer-unicode-codepoint-escapes rule.
    • Add regexp/sort-flags rule.
    • Add regexp/strict rule.
    • Add no-empty-character-class rule.
    • Remove no-invalid-regexp rule.
  • Change default option

    • Change hexadecimalEscape option of regexp/letter-case rule from ignore to lowercase.
    • Change controlEscape option of regexp/letter-case rule from ignore to uppercase.

@ota-meshi ota-meshi force-pushed the configs branch 5 times, most recently from 65b6737 to 285a127 Compare March 22, 2021 06:24
@RunDevelopment RunDevelopment added this to the v1.0 milestone Apr 15, 2021
@ota-meshi ota-meshi force-pushed the configs branch 3 times, most recently from 059f721 to e7ee81e Compare April 18, 2021 16:18
Repository owner deleted a comment from coveralls Apr 18, 2021
@ota-meshi ota-meshi force-pushed the configs branch 2 times, most recently from fabfca0 to 968d9bb Compare April 21, 2021 00:49
@ota-meshi ota-meshi force-pushed the configs branch 2 times, most recently from 6884c13 to 305edc9 Compare June 11, 2021 09:40
@ota-meshi ota-meshi force-pushed the configs branch 4 times, most recently from 8a5f1ff to 69dab2a Compare August 10, 2021 23:55
@RunDevelopment
Copy link
Collaborator

I think there are 2 problems with the recommended config:

  1. regexp/strict overlaps with 3 other rules. regexp/strict is a superset of regexp/no-standalone-backslash, regexp/no-octal, and regexp/no-useless-escape.

    I don't know whether we should remove those 3 rules from the rec config or whether we should live with double reporting in those cases.

  2. regexp/no-useless-non-capturing-group: I love this rule but it's so annoying when writing regexes. It always tries to remove non-capturing groups and overshadows other errors while I write my regexes. It's great for code review but not so much for live editing.

    Maybe we should remove it from the rec config? I don't want people to install our plugin only to be annoyed by this rule.

    Another idea would be to make the rule less annoying. The problem is that it underlines the whole group. Maybe we could make the report range smaller (e.g. only the (?: part)? That way the rule wouldn't prevent me from seeing errors within the group.

@ota-meshi
Copy link
Owner Author

regexp/strict overlaps with 3 other rules. regexp/strict is a superset of regexp/no-standalone-backslash, regexp/no-octal, and regexp/no-useless-escape.

I think it's good to turn off regexp/no-standalone-backslash. It rule is a complete subset of regexp/strict.

However, there seems to be a slight difference between regexp/no-useless-escape and regexp/no-octal from regexp/strict.

var foo = /[\|]/; // Reported in `regexp/no-useless-escape`. But not reported in `regexp/strict`.
var foo = /[\1-\4]/; // Reported in `regexp/strict`. But not reported in `regexp/no-octal`.

These rules may report duplicates, but I think it's good to turn them on.

@RunDevelopment
Copy link
Collaborator

However, there seems to be a slight difference between regexp/no-useless-escape and regexp/no-octal from regexp/strict.

var foo = /[\|]/; // Reported in `regexp/no-useless-escape`. But not reported in `regexp/strict`.
var foo = /[\1-\4]/; // Reported in `regexp/strict`. But not reported in `regexp/no-octal`.

These rules may report duplicates, but I think it's good to turn them on.

I agree with you on regexp/no-useless-escape but I think we should remove regexp/no-octal from recommended. regexp/strict disallows all octal escapes while regexp/no-octal does allow some octal escapes. So regexp/no-octal is a subset of regexp/strict.

@ota-meshi
Copy link
Owner Author

I think we should remove regexp/no-octal from recommended.

You're right. I will change this PR.

@RunDevelopment RunDevelopment merged commit 7b4ad4b into master Aug 13, 2021
@RunDevelopment RunDevelopment deleted the configs branch August 13, 2021 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants