/* eslint-expect-error */ — continuous verification of eslint config

[jtbandes]

(Forgive me if this has been discussed before, but I didn't find any issues or discussions about it.)

The problem you want to solve.

I'd like to be able to write examples of bad code in my project, and enforce that they continue to be rejected by eslint, to guard against accidental changes to eslint configs/rule implementations.

I'm not proposing this as a rule, because of the "atomic" requirement in the core rule guidelines. It's more of a meta-feature that lives outside the realm of specific rules, similar to existing ignore-directives.

Your take on the correct solution to problem.

A comment would be the simplest way to annotate expected warnings/errors, and would match nicely with the existing comments for eslint-disable:

a == b // eslint-expect-error             <-- expect any error to be raised
a == b // eslint-expect-error eqeqeq      <-- expect an error to be raised from a specific rule
a == b // eslint-expect-warning eqeqeq    <-- expect a warning to be raised
let a == b // eslint-expect-error eqeqeq, prefer-const   <-- expect two errors to be raised

// eslint-expect-error-next-line eqeqeq
a == b
// eslint-expect-warning-next-line eqeqeq
a == b

I think it would also be valuable to allow these assertions to mention the specific error message. This could be done with strings or regular expressions:

a == b // eslint-expect-error /^Expected '===' and instead saw '=='\.$/   <-- regex match
a == b // eslint-expect-error "Expected '==='"                            <-- substring match

It doesn't make as much sense to have an expected error at the whole file level; I think "same line" and "next line" options would be sufficient.

These checks would be run as part of the normal eslint execution. If an error/warning would normally be emitted that matches the expectation comment, instead nothing would be emitted (the check would be considered a success). If the expected error/warning was not raised on the indicated line, then an error would be emitted.

Variations/alternatives to consider

• There could be a config option to choose whether expectation failures are errors or just warnings. Since this guards against configuration changes, I don't personally see much value in making errors a warning
• There could also be a "no warning" comment, such as // eslint-expect-no-warning. I think this option would only be useful when applied to warnings — since "expect no error" is already how eslint behaves.
• Instead of running during normal execution, there could be a separate --verify mode. Since it's likely that these expect-error comments would only be used in non-production code, it could make sense for the verification step to be a separate kind of eslint invocation. However, I thought this was a needlessly complicated design.

Prior art

• TypeScript has // ts-expect-error comments (documentation, implementing PR). These are a part of the normal TS compilation process, not a separate mode. They don't allow you to specify exactly which error is expected, though that enhancement has been requested by the community.

• The clang compiler has a -verify mode which uses special comments to indicate which diagnostic output is expected. A substring or regular expression can be used to match against the error text. For example:

  int A = B; // expected-error {{use of undeclared identifier 'B'}}

  Clang's verify mode also has some advanced features such as custom prefixes, which allows the same source file to provide test cases for different compiler invocations.

Are you willing to submit a pull request to implement this change?

If the team/community can agree on the details of the design, I'd be open to implementing this, though I would also like some guidance on where to make the changes.

[mdjermanovic]

Hi @jtbandes!

I think that ESLint already provides this functionality with the --report-unused-disable-directives cli/config option.

For example, if you make a test.js file with this content:

a == b; // eslint-disable-line eqeqeq

and run:

eslint test.js --report-unused-disable-directives

ESLint will report an error if the eqeqeq rule isn't enabled.

[jtbandes]

Thanks for the tip, that is useful and does seem to address the majority of use cases. I guess the missing features would be around assertions on the error/warning text rather than the rule name.

[jtbandes]

I also found this related issue proposal about making --report-unused-disable-directives something you can enable in the eslint config file. I think this would be really useful because it would improve editor/IDE integration. #9382

Although I found that with VS Code, it is possible to enable this by adding the following settings:

  "eslint.options": {
    "reportUnusedDisableDirectives": true
  }

[mdjermanovic]

I also found this related issue proposal about making --report-unused-disable-directives something you can enable in the eslint config file. I think this would be really useful because it would improve editor/IDE integration. #9382

This is already implemented by #12151, per eslint/rfcs#22.

The .eslintrc.* config option is reportUnusedDisableDirectives. It works the same as the --report-unused-disable-directives cli option, except that it produces warnings instead of errors.

[jtbandes]

I see, that's cool. Maybe it would be valuable to add an option for it to produce errors.

[mdjermanovic]

Maybe it would be valuable to add an option for it to produce errors.

The decision to produce only warnings was made because of our semver policy that patch releases are intended to not break builds. Patch release can contain a bug fix that results in ESLint reporting fewer linting errors, but that could actually become more errors (and thus break the build) if reportUnusedDisableDirectives is enabled and an eslint-disable-* comment was used to suppress the false positive.

The --report-unused-disable-directives cli option is considered as an explicit opt-in to this possibility, and there's a warning about this in the documentation, while the reportUnusedDisableDirectives config option can be enabled in a shareable config so the user might be unaware of it.

[jtbandes]

As for the ability to match on a specific error message, I guess this enhancement could be made to the eslint-disable comment. Do you think that proposal would have a chance? If so, I can file a separate issue.

[mdjermanovic]

As for the ability to match on a specific error message, I guess this enhancement could be made to the eslint-disable comment. Do you think that proposal would have a chance? If so, I can file a separate issue.

Hmm, I'm not sure if that would be accepted. Messages can change, so any feature that would account for a fixed message text wouldn't be reliable for end users. Then, I'd rather vote for the new test-oriented type of comments as in this proposal.

[mdjermanovic]

For detailed tests, you can also consider ESLint API.

For example, this would load .eslintrc.* config file from the file system and check the eqeqeq rule:

const { ESLint } = require("eslint");
const assert = require("assert");

async function test() {
    const eslint = new ESLint();
    const code = "a == b";
    const [result] = await eslint.lintText(code);
    const lintMessage = result.messages.find(({ ruleId }) => ruleId === "eqeqeq");

    assert(lintMessage); // "eqeqeq" reports a problem
    assert.strictEqual(lintMessage.severity, 2); // it's set to "error"
    assert.strictEqual(lintMessage.message, "Expected '===' and instead saw '=='."); // message text
};