fix(rule-finder): Support for unnamed scoped packages

[bradzacher]

ESLint supports "unnamed" scoped plugins. @scope translates to @scope/eslint-plugin.

This PR adds support for that case.

[sweetlikepete]

Hey Brad, this fix is awesome!

I found something that might be a problem. When you run this version of eslint-find-rules over an eslint config that contains all the rules for @getify/eslint-plugin-proper-arrows all rules come up as missing.

I'm thinking this has something to do with this eslint-plugin using rule names in the format "@getify/proper-arrows/{rule-name}" instead of what every other plugin I've ever used does "proper-arrows/{ rule-name }".

Thanks again for updating this script. I hope this information is helpful.

[bradzacher]

Are you sure that's correct, @sweetlikepete?

Looking at the Readme for that plugin, it says you have to prefix the rule name with the scope

https://github.com/getify/eslint-plugin-proper-arrows/blob/master/README.md#eslintrcjson

[ljharb]

You definitely do; all scoped plugins and shareable configs work that way.

[sweetlikepete]

It's the only eslint plugin I have that is part of a scoped package and when I run eslint-find-rules all the rules it has are returned as unused in the format 'proper-arrows/params proper-arrows/name' when they added in my eslint rules config as '@getify/proper-arrows/params' and '@getify/proper-arrows/name'. The configuration is working correctly in eslint.

[bradzacher]

In regards to the failing tests - this is because @scope was only properly supported in ESLint v5. Is it worth deleting the v4 test cases? Or should we try and move them into a separate structure so v4 and v5 can be tested separately? (is it worth the effort considering v5.0.0 is almost a year old now?)

[ljharb]

Yes, it is still worth the effort to support as far back as we can.

Certainly if some functionality only is supported in eslint 5, we shouldn’t test it in < 5.

[bradzacher]

Ah small problem @ljharb.

By not testing scoped plugins/packages for < v5: the code coverage can never reach 100%, so the CI will always fail. Off the top of my head, I don't think that there's any way to make this work without creating a file that's conditionally required?

Should I reconfigure travis to disable coverage checks for < v4?

[bradzacher]

I used an istanbul comment to specifically disable coverage on the if statement in question, as it cannot be tested on <5 due to the differing implementations.

Note that I removed npm ls > /dev/null from the CI script, as it broke on an unrelated dependency on the node4 test.

[ljharb]

Why does npm ls break on node 4? It shouldn't.

[bradzacher]

https://travis-ci.org/sarbbottam/eslint-find-rules/jobs/519370506#L851

One of the dependencies (har-validator) of dependencies (request) of dependencies (all-contributors-cli) uses a different AJV version and it causes it to error.

I think that the dependency resolution algorithm must be different in node v4?

What's the reason that it was running npm ls?

[ljharb]

In that case, please add nvm install-latest-npm to before_install in travis.yml, rather than removing npm ls (which, if it fails, means that nothing can be expected to work, because the dependency graph is invalid)

[bradzacher]

Done! That worked, so it must have been a change in the super old npm algorithm. All tests are passing now!

[noamokman]

Any updates on this?

[noamokman]

@sarbbottam are you no longer maintaining this project? Could you give permissions to other team members to advance this? Thanks!

[ljharb]

@noamokman we're quite capable of merging it; it just needs a second review.

[ljharb]

to clarify: from a collaborator :-)

[noamokman]

@ljharb thanks for merging this! Do you mind releasing a new version to npm? Thanks again 😄