no-unused-disable rule breaks with ESLint 5.0.0-alpha.1

[ehmicky]

The rule throws:

Error while loading rule 'eslint-comments/no-unused-disable': Cannot read property 'report' of undefined
TypeError: Error while loading rule 'eslint-comments/no-unused-disable': Cannot read property 'report' of undefined
    at Object.create (.../node_modules/eslint-plugin-eslint-comments/lib/rules/no-unused-disable.js:33:39)
    at createRuleListeners (.../node_modules/eslint/lib/linter.js:676:21)
    at Object.keys.forEach.ruleId (.../node_modules/eslint/lib/linter.js:830:31)
    at Array.forEach (<anonymous>)
    at runRules (.../node_modules/eslint/lib/linter.js:788:34)
    at Linter._verifyWithoutProcessors (.../node_modules/eslint/lib/linter.js:981:31)
    at preprocess.map.textBlock (.../node_modules/eslint/lib/linter.js:1032:35)
    at Array.map (<anonymous>)
    at Linter.verify (.../node_modules/eslint/lib/linter.js:1031:42)
    at localVerify (.../node_modules/eslint-plugin-html/src/index.js:92:14)

The faulty source code is:

create(context) {
    const linter = context.eslint || context._linter
    const originalReport = linter.report

ESLint 5.0.0-alpha.1 removed context._linter being it considered it being a hack.

[mysticatea]

Thank you for the report.

Yeah, we can't use eslint-comments/no-unused-disable rule in ESLint 5.0.0. That rule will be replaced by --report-unused-disable-directives CLI option. See eslint/eslint#10140 for details.

I have to deprecate that rule in this plugin.

[ehmicky]

Thanks, that makes sense.

It's a little too bad though that this is available as a CLI flag only. Unless I'm wrong there would be no way to specify --report-unused-disable-directives in the configuration file instead.

[mysticatea]

You are right. It's inconvenient.

I have two other ways:

1. It's inspired by eslint-plugin-vue's processor. We can know what rules are reported in post-processor, so I can re-implement eslint-comments/no-unused-disable rule there, as similar to vue/comment-directive rule. This way is using only public API, but it has a restriction -- ESLint enables only one post-processor for each extension (e.g. .js). This means that this plugin might be going to conflict another plugins.
2. I might investigate another hack, E.g., it finds Linter class in require.cache and patch the public API Linter.prototype.verify with a logic which is similar to eslint-plugin-vue's processor. I'm not sure if this way can cover wide use cases.

[ehmicky]

Is there a specific reason ESLint allows only one post-processor per file extension? Otherwise maybe this could be improved first, then this can be fixed without resorting to another hack?

I also wanted to mention that having this rule as a CLI flag instead makes it not available in some IDEs. E.g. in Atom, it does not seem to be possible to specify CLI flags to the official ESLint Atom plugin. Although it could be argued that the real fix is to improve that plugin to support CLI flags.

[mysticatea]

Is there a specific reason ESLint allows only one post-processor per file extension? Otherwise maybe this could be improved first, then this can be fixed without resorting to another hack?

Yes, because processors are designed to extract multiple code blocks from a file. For example, .md can have some code blocks, .html can have some <script> tags. So pre-processor is (content: string) => string[] and post-processor is (messagesOfEachCode: Message[][]) => Message[]. The types of input and output are different, so it can't compose processor functions. The original purpose of post-processors is to modify the locations of the messages in order to rollback the extraction rather than filtering messages.

I also wanted to mention that having this rule as a CLI flag instead makes it not available in some IDEs. E.g. in Atom, it does not seem to be possible to specify CLI flags to the official ESLint Atom plugin. Although it could be argued that the real fix is to improve that plugin to support CLI flags.

CLIEngine has corresponding option reportUnusedDisableDirectives: true. I'm not familiar with Atom, but if Atom has options like eslint.options in vscode, we can use it.

However, we can't specify the option in shareable configs, it's inconvenient.

[ehmicky]

OK it makes sense that combining processors is going to be hard or impossible.

It seems to me the correct solution might be to allow specifying this CLI flag in shareable configs as well.

In the meantime I have raised an issue for Atom users to fix my immediate problem (which basically is: having my IDE notify me as I code that I'm using a unused eslint-disable comment).

[mysticatea]

I published v3.0.0-beta.0. I used a hack to make the eslint-comments/no-unused-disable rule aliving on ESLint v5.0.0. The new hack uses only public APIs, but it needs an assumption.

• The rule is loaded from CLIEngine class.

I believe that it can cover most usecases.
I'd like to see the response while 3.0.0-beta then I will publish it as stable after ESLint 5.0.0 stable is released.

[ehmicky]

This is great! Thanks for your work on this.

[mysticatea]

Closing as I have published v3.0.0. Thank you!