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

Allow regex patterns in ignore lists #19

Closed
ivan-aksamentov opened this issue Jun 2, 2020 · 10 comments · Fixed by #24
Closed

Allow regex patterns in ignore lists #19

ivan-aksamentov opened this issue Jun 2, 2020 · 10 comments · Fixed by #24

Comments

@ivan-aksamentov
Copy link

ivan-aksamentov commented Jun 2, 2020

Add capability to add regex patterns along with strings in ignore lists.

For example, if I want to ignore all functions starting with "foo" (e.g. "foo()", "foo1()", "fooTwo()") and a function "bar()", I would add:

{ ignoreCallee: [/foo.*/, 'bar'] }

This would allow non-exhaustive checks, that is, to ignore a list of items where elements are not known in advance (or tedious to enumerate).

Related to #18 (the code will probably be shared)

@ivan-aksamentov ivan-aksamentov changed the title Allow regex patters in ignore lists Allow regex patterns in ignore lists Jun 2, 2020
@edvardchen
Copy link
Owner

  1. You can't write regex in yaml. So all ignore-options must be strings.
  2. Sure, we can use new RegExp(...) to transform from string, like I did with ignore option. But when user input i18next.t then would match i18nextxt or i18nextyt.
  3. The dot character is important in ignoreCallee that tell the plugin to do upward lookup.

@ivan-aksamentov
Copy link
Author

ivan-aksamentov commented Jun 4, 2020

@edvardchen Is there a way to allow regular expressions in .eslintrc.js only?

Like this:

{ ignoreCallee: [/foo.*/, 'bar'] }

or like this:

{ ignoreCallee: [RegExp('foo.*'), 'bar'] }

?

Or is this a limitation of the eslint config parser? Any way of workaround this?

@edvardchen
Copy link
Owner

@ivan-aksamentov
No, I would not support RegExp . Neither other eslint plugins did.

The other option is we can treat string as RegExp, like I did to ignore option.

{ ignoreCallee: ["foo.*", 'bar'] }

Sure, then we can skip function call like fooA(...) or fooB(...)

But this would violate current behavior.

For example in README

/*eslint i18next/no-literal-string: ["error", { "ignoreCallee": ["yourI18n.method"] }]*/
const bar = yourI18n.method('bar');

And users don't expect the plugin to skip yourI18nXmethod(...), do they?

@ivan-aksamentov
Copy link
Author

@edvardchen Yes, I understand the issues with treating the strings as regexes. Although if you are willing to implement it as it a breaking change (in version 4), then we could declare all strings regexes. So that when transitioning to version 4 users would have to escape their dots and other special characters.

Some popular plugins do treat strings as regexes: https://github.com/benmosher/eslint-plugin-import#importignore

No, I would not support RegExp . Neither other eslint plugins did.

Given the complications with strings-as-regexes, why not? Eslint and plugins support filename wildcards in some places (which probably requires using some library or a custom parser), so I don't see how regexes are fundamentally different.

See for example eslint's own override: https://eslint.org/docs/user-guide/configuring#disabling-rules-only-for-a-group-of-files

I've no idea how that works, but just want to understand the reasoning here.

@edvardchen
Copy link
Owner

@ivan-aksamentov You misunderstood me. What I said was that I wouldn't support JS type RegExp.

I agree that we can make a breaking change to treat ignoreCallee not literal strings but regular expression in strings

@ivan-aksamentov
Copy link
Author

@edvardchen

You misunderstood me. What I said was that I wouldn't support JS type RegExp.

This is how I understood it and then asked why :)

@edvardchen
Copy link
Owner

edvardchen commented Jun 7, 2020

Forget it. Actually this plugin would already have support it while supporting regexp-in-strings

Because new RegExp(...) can accept both string and RegExp like/.*/

edvardchen added a commit that referenced this issue Jun 28, 2020
allow user to ignore strings or callee by regular expression

BREAKING CHANGE: all patterns in ignoreCallee would be treated as regular expression

fix #19
@edvardchen
Copy link
Owner

@ivan-aksamentov plz see the following added test case is enough or not

// valid
    {
      code: 'foo.bar("taa");',
      options: [
        {
          ignoreCallee: [/foo.+/]
        }
      ]
    },

edvardchen added a commit that referenced this issue Jun 28, 2020
allow user to ignore strings or callee by regular expression

BREAKING CHANGE: all patterns in ignoreCallee would be treated as regular expression

fix #19
@ivan-aksamentov
Copy link
Author

@edvardchen Yes. Thanks so much!
Can't wait for the new release. :)

@edvardchen
Copy link
Owner

try version v4

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 a pull request may close this issue.

2 participants