False-positive for require-meta-docs-url rule

[sindresorhus]

I'm getting:

  rules/utils/get-docs-url.js:1:1
  ✖    1:1   Rules should export a meta.docs.url property.  eslint-plugin/require-meta-docs-url

  rules/utils/resolve-variable-name.js:1:1
  ✖    1:1   Rules should export a meta.docs.url property.  eslint-plugin/require-meta-docs-url

  rules/utils/avoid-capture.js:1:1
  ✖    1:1   Rules should export a meta.docs.url property.  eslint-plugin/require-meta-docs-url

For these files which are not rules: https://github.com/sindresorhus/eslint-plugin-unicorn/tree/master/rules/utils

Just remove https://github.com/sindresorhus/eslint-plugin-unicorn/blob/a7b1bdb8749d615ffdd05c87c7df3b971925abd8/package.json#L77-L84 and run npm test there to reproduce.

[not-an-aardvark]

In general, it's nontrivial to detect whether a file contains a rule. I'd be open to improving the heuristic that we use, but the most robust way to avoid these errors would be to use glob configuration to only run require-meta-docs-url (and other rules in the rules preset) on files that actually contain rules.

[sindresorhus]

I think you could just add a check that the export is an object literal, which rules have to be. That would solve my problem, as all the false-positive files export a function, not object.

[not-an-aardvark]

Rules can also be functions if they're created using the deprecated format. (Despite being deprecated, that format still seems to be somewhat common -- for example, all of Node's rules seem to use it.)

[sindresorhus]

I don't think this plugin should care about deprecated features. Actually, there should be a rule here that disallows deprecated features. Node is also not using this plugin.

[not-an-aardvark]

I'm aware that Node isn't using this plugin. I was just pointing it out as an example off the top of my head of a project that uses the deprecated rule format, as evidence for the claim that the deprecated format is still somewhat common.

I think it would be reasonable to have a rule that disallows the deprecated rule format. Note that in some sense the require-meta-docs-url rule is effectively already doing that in this case, since the deprecated format doesn't allow the use of a meta object. In either case, the rule would end up with the same false positive that you're seeing here if it misidentified a random exported function as a rule in the deprecated format. So I think the recommendation from #81 (comment) would still be the best way to avoid this issue.

[sindresorhus]

as evidence for the claim that the deprecated format is still somewhat common.

That's not evidence that it's common. It's just evidence that at least one project is using the deprecated format.

the rule would end up with the same false positive that you're seeing here if it misidentified a random exported function as a rule in the deprecated format.

Not if you use a helper utility in every rule to identify whether it's actually a (non-deprecated) proper rule file. For example, in eslint-plugin-ava, every rule ensures it's running in an AVA test file.

So I think the recommendation from #81 (comment) would still be the best way to avoid this issue.

Would be better to actually fix the issue than to recommend a boilerplaty config workaround.

Even if you insist on supporting the deprecated format, which IMHO makes no sense for a plugin meant to enforce plugin best practices, you could still detect a rule function vs a utility function by checking whether the first parameter name is context.

[not-an-aardvark]

That's not evidence that it's common. It's just evidence that at least one project is using the deprecated format.

Unfortunately, I don't have time to survey a very large sample of third-party ESLint rules to conclusively determine whether the deprecated format is common, so we're stuck with anecdotal evidence at this point. From what I've seen, the deprecated format is still relatively common to an extend that I wouldn't want the plugin to completely ignore files with rules in the deprecated format. (Even if the plugin's goal is to enforce best practices, it doesn't seem like it useful to simply ignore files containing deprecated rules, since this would effectively prevent people from noticing that they're not using best practices.)

the rule would end up with the same false positive that you're seeing here if it misidentified a random exported function as a rule in the deprecated format.

Not if you use a helper utility in every rule to identify whether it's actually a (non-deprecated) proper rule file.

To be clear, I was referring to the idea of having a rule that warns when it encounters the deprecated rule format. Naturally, a rule like this would need to be able to detect deprecated rule files, leading to the potential for false positives like the one you've reported in this issue.

Would be better to actually fix the issue than to recommend a boilerplaty config workaround.

Even if you insist on supporting the deprecated format, which IMHO makes no sense for a plugin meant to enforce plugin best practices, you could still detect a rule function vs a utility function by checking whether the first parameter name is context.

Broadly speaking, I disagree that using overrides in this case is a "workaround" -- I think it's the most direct way to achieve what you want given that it's not possible to perfectly make this determination automatically. It doesn't make sense to run a rule intended to lint ESLint rules on a file which doesn't contain ESLint rules, so it seems appropriate to finely it at your rule files.

I think something like the context heuristic could nonetheless be a worthwhile improvement, although we would also probably want to detect things like destructuring to obtain a report method without explicitly naming a context parameter.

[bmish]

I've implemented improved heuristics in #211 and #213 that should solve these false positives with function-style rules.