5187 new cap wildcard support

[diki]

wildcard support for new-cap rule

[mention-bot]

By analyzing the blame information on this pull request, we identified @roadhump, @ljharb and @nzakas to be potential reviewers

[eslintbot]

Thanks for the pull request, @diki! I took a look to make sure it's ready for merging and found some changes are needed:

• The commit summary needs to begin with a tag (such as Fix: or Update:). Please check out our guide for how to properly format your commit summary and update it on this pull request.
• Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

[platinumazure]

Hi @diki, thanks for the pull request!

Besides the comments left by @eslintbot, this PR also has two commits in it now. Per our pull request guide, we like to have one commit per pull request. Could you please squash your commits and force-push to your branch when you have cleaned up the other items? Thanks a lot!

[diki]

@platinumazure this is my first PR ever, made the all changes necessary now watching excitedly :)

[platinumazure]

No worries (and congrats on putting together your first PR!).

Just one more thing: We're a little picky about the formatting of the issue number in the commit message (see Travis build result).

All you need to do is edit the commit message and re-force push.

git commit --amend -m "Update: Wildcard support for the new-cap rule (fixes #5187)"
git push -f origin 5187-new-cap-wildcard-support

Uniform commit messages mean pretty auto-generated changelogs. 😄

Thanks so much for your patience! You're very nearly there, I promise.

[ljharb]

Does this rule currently only cover identifiers? In other words, foo['Bar']() seems like it would error out also. If so, doesn't that mean that this wildcard support would break that? For example, what if I had a literal identifier Foo* I wanted an exception for?

[diki]

@platinumazure done!

@ljharb i made some modifications to support foo['Bar'] case.

[ljharb]

@diki i'm not sure you understood what I meant. Let's say I want it to warn on var obj = {}; obj['Foo*'] = function () {}; obj['Foo*']() - how would I do that with your wildcard support?

[diki]

@ljharb you are right i got it wrong but at least now i can handle obj['Foo'] type expressions.

Let's say I want it to warn on var obj = {}; obj['Foo*'] = function () {}; obj'Foo*' - how would I do that with your wildcard support?

It may be a noob question (sorry for that already) could you do that before or cannot do it after this patch? Current new-cap rule already does not deal with expressions like obj['Foo'] or obj['Foo*'].

[ljharb]

@diki that's a fair point - I'm certainly not claiming you're breaking an existing use case - but what you are doing is permanently preventing it from being supported.

I wonder if it would be better, in the wildcard case, for the rule option to be in a different format? Either capIsNewWildcardExceptions (and the corresponding support for newIsCapWildcardExceptions), or instead of a string, an object with a "wildcard" property, or something similar?

[diki]

@ljharb agree. capIsNewWildcardExceptions and newIsCapWildcardExceptions options fit much better. I updated the pull request accordingly, thanks for advice.

[ljharb]

i wonder if it would be a better API to allow this to be an array of RegExp source strings - ie, pass it into new RegExp() and use the resulting regular expression to do the test.

That way things like ^ and $ and $ etc all work, and the plugin doesn't have to implement any of them :-)

[nzakas]

Is that level of granularity needed? The issue asked for wildcard support only.

[ljharb]

By granularity do you mean a separate option? my question here hopefully explains why I don't think it should be folded into the existing option.

As for the regex support - i don't think it's needed, but including it gets us a lot of functionality for free without having to implement anything special.

[nzakas]

Let's discuss back on the issue.

[nzakas]

@diki Looks like we lost track of this. Are you interested finishing this up?

[nzakas]

@diki one last ping. Can you let us know if you plan on finishing this? Thanks

[nzakas]

No response and without a signed CLA, we are unable to carry over this pull request, so closing.