feat: implement rfc 2021-suppression-support

[Yiwei-Ding]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Implemented the RFC 2021-suppression-support.

Related issue #14784.

Is there anything you'd like reviewers to focus on?

[nzakas]

It looks like there are several file conflicts. Can you please take a look?

[Yiwei-Ding]

Conflicts resolved. Thanks.

[nzakas]

Overall looks pretty good. A few things that jumped out to me.

[nzakas]

Would it make sense to add location information to the suppression to show where the directive is?

[Yiwei-Ding]

It's a good idea. The location info can be leveraged for auditing purposes as well. I also find a location property in suppressions in the SARIF format.

But I'd like to add location info to the suppressions in another PR because:

1. The type location in suppressions is needed to be discussed. Shall we use the line and the column in unprocessedDirective directly or we use the similar structure in LintMessage (line/column/endLine/endColumn)?
2. This PR is huge enough due to a lot of modification in core logic and tests.

[mdjermanovic]

I think this doesn't work as expected with processors.

For example, I was testing with eslint-plugin-markdown.

.eslintrc.js:

module.exports = {
    plugins: ["markdown"],
    rules: {
        "no-undef": 2
    },
    overrides: [
        {
            files: ["**/*.md"],
            processor: "markdown/markdown"
        }
    ]
};

test.md:

```js
a;
```

```js
b;
```

D:\tmp\suppressions\eslint>npx eslint test.md

D:\tmp\suppressions\eslint\test.md
  2:1  error  'a' is not defined  no-undef
  6:1  error  'b' is not defined  no-undef

✖ 2 problems (2 errors, 0 warnings)

Then, if we suppress those two problems:

```js
a; // eslint-disable-line
```

```js
b; // eslint-disable-line
```

D:\tmp\suppressions\eslint>npx eslint test.md -f json > results.json

results.json:

[
  {
    "filePath": "D:\\tmp\\suppressions\\eslint\\test.md",
    "messages": [],
    "suppressedMessages": [
      {
        "ruleId": "no-undef",
        "severity": 2,
        "message": "'b' is not defined.",
        "line": 1,
        "column": 1,
        "nodeType": "Identifier",
        "messageId": "undef",
        "endLine": 1,
        "endColumn": 2,
        "suppressions": [
          {
            "kind": "directive",
            "justification": ""
          }
        ]
      }
    ],
    "errorCount": 0,
    "fatalErrorCount": 0,
    "warningCount": 0,
    "fixableErrorCount": 0,
    "fixableWarningCount": 0,
    "usedDeprecatedRules": []
  }
]

What doesn't look right in the output:

1. There's only one suppressed message instead of two (slots.lastSuppressedMessages had only messages from the last code block).
2. line and endLine should be 6 instead of 1 (the message was not postprocessed, so the location wasn't adjusted).

[Yiwei-Ding]

@mdjermanovic Thanks for pointing out! I moved distinguishSuppressedMessages() from applyDisableDirectives() to Linter#verify(), so that the linter with processors could preprocess - lint - postprocess - return non-suppressed messages.

[mdjermanovic]

This link is broken as there's no getsuppressedmessages section in the document.

[mdjermanovic]

ESLint#getRulesMetaForResults() returns meta of rules that appear in messages. Per the description of this method, it "returns meta objects for each rule represented in the lint results".

eslint/lib/eslint/eslint.js

Lines 514 to 519 in 851f1f1

	/**
	* Returns meta objects for each rule represented in the lint results.
	* @param {LintResult[]} results The results to fetch rules meta for.
	* @returns {Object} A mapping of ruleIds to rule meta objects.
	*/
	getRulesMetaForResults(results) {

I think the expected behavior would be to include rules that appear in suppressedMessages.

@nzakas what do you think about this?

[mdjermanovic]

ESLint.getErrorResults() / CLIEngine.getErrorResults() filters out active (nonsuppressed) warnings, so it should probably filter out suppressed warnings as well. I think it would be unexpected to entirely omit active warnings from the results, but keep the suppressed ones.

Thoughts?

[nzakas]

I think the expected behavior would be to include rules that appear in suppressedMessages.

I agree. We want to be sure all rules represented in a formatter have access to the related meta info.

[mdjermanovic]

LGTM, thanks!

Since this is a big change in the core, I'd certainly like someone else to review it too before merging.

[nzakas]

LGTM. Thanks so much for your hard work on this. Excellent job!

[mdjermanovic]

Thanks for contributing! This will be released tomorrow in ESLint v8.8.0