Change Request: Config to Disallow Disable

[Brian-Bartels]

ESLint version

8.0.0

What problem do you want to solve?

ESLint currently allows users to disable rules. In large codebases with MANY Prs, some users disable lint rules without really knowing what they are doing. This slips by PRs because reviewers sometimes don't actually review ¯\_(ツ)_/¯

There exists rules such as no-abusive-eslint-disable but that in and of itself can be disabled

What do you think is the correct solution?

I'd like ESLint to allow users to configure certain rules to be exempt from all comment style disables

eslintrc: "noExtraSemi": ["error", "dissalowDisable"]
eslintrc: "noExtraSemi": ["forbid"]

so in code

// eslint-disable-next-line noExtraSemi
const foo = 5;;

Would still result in an error

Participation

• I am willing to submit a pull request for this change.

Additional comments

No response

[nzakas]

Would noInlineConfig solve this problem for you?
https://eslint.org/docs/user-guide/configuring/rules#disabling-inline-comments

[Brian-Bartels]

@nzakas That would work for some. But for us where we have custom lint rules to enforce security or other critical pieces. Disabling some rules is okay where disabling other is not okay

[Pzixel]

Just for a reference, in rust there are 3 levels of enabling lints/rules/...
allow(x) - allow violation of rule x
deny(x) - forbids violation of rule x
forbid(x) - forbids violation of rule x and makes it hard error to do allow(x)

See link for details.

[Brian-Bartels]

ohhhh @Pzixel I do like that implementation much better than what I had proposed

[nzakas]

I’m on the fence as to whether there’s enough of a use case here to rationalize the additional complexity to the config system. At the moment, the tradeoff here doesn’t seem worth it. I’m curious what other team members think.

As a note: we aren’t making any further changes to the current config system as we shift to a new config system, so any potential changes would be a bit in the future.

[Brian-Bartels]

As evidence for people being bad PR reviewers following lines are in a repo that I work on over 30 times. And yes it's ridiculous that this get's checked in. That's just the nature of incredibly large repos though :(

// eslint-disable-next-line foo/no-abusive-eslint-disable
// eslint-disable-next-line foo/no-eslint-disable
// eslint-disable-next-line foo/no-foo-disable-with-exceptions

[mdjermanovic]

I wouldn't be opposed to adding this feature to the core if there are no alternatives. In my understanding, some plugin rules like eslint-comments/no-restricted-disable provide the same functionality, but the only problem is that those rules can be disabled as well. Would it work for you to run eslint again with the --no-inline-config flag and a config file that has only rules related to eslint comments enabled?

[ljharb]

Disabling all inline config doesn’t seem like it solves the problem - some rules should be disable-able with inline config; some should not.

[mdjermanovic]

Disabling all inline config doesn’t seem like it solves the problem - some rules should be disable-able with inline config; some should not.

I agree with that, the solution I suggested is another run only with rules that provide this functionality, assuming that the only problem is how to prevent disabling them.

npx eslint . --no-inline-config --no-eslintrc -c .eslintrc2.js

//--------- .eslintrc2.js -----------

module.exports = {
    plugins: ["eslint-comments"],
    rules: {
        "eslint-comments/no-restricted-disable": ["error", "no-undef" /*, ... */]
    }
};

[Brian-Bartels]

@mdjermanovic One concern with that workaround. How would it integrate into VSCode? I worry that people would disable a rule expecting it to pass PR build step. But then we'd sneak in a second run of ESlint and fail their builds unexpectedly.

I do also think that that workaround falls into the "slightly hacky" category. Works perfectly fine, but adding a second config file isn't ideal