Add suppression information in the output of ESLint

[Yiwei-Ding]

The version of ESLint you are using.
v7.29.0

The problem you want to solve.
ESLint is a great and famous tool for identifying and reporting on patterns found in ECMAScript/JavaScript code. It helps a lot in our development. Sometimes we developers have to suppress warnings/errors in the code by adding eslint-disable, eslint-disable-line and eslint-disable-next-line comments, according to disabling-rules. We can also use .\node_modules\.bin\eslint.cmd 1.js -f @microsoft/eslint-formatter-sarif -o 1.sarif --no-inline-config to dump all information including suppressed ones into a SARIF file.

However in our scenarios, Security Development Lifecycle (SDL) tools, such as ESLint, are expected to export suppression justifications when warnings/errors are disabled. For example, a developer use eslint-disable-line no-console to suppress a warning:

console.log('foo'); // eslint-disable-line no-console -- Just for debug.

Then in DevOps, ESLint could record this message and dump it into file:

"results": [
        {
          "level": "warning",
          "message": {
            "text": "foobar"
          },
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "file:///C:/Users/test/1.js",
                  "index": 0
               },
                "region": {
                  "startLine": 1,
                  "startColumn": 1
                }
              }
            }
          ],
          "ruleId": "no-console",
          "ruleIndex": 0,
          "suppressions": [                      //
            {                                    //
              "kind": "inSource",                //
              "justification": "Just for debug." //  <-- This is what we are expected.
            }                                    //
          ]                                      //
        },
     ]

This suppression info would trigger a signal to our team, and thus we could judge whether the suppression is reasonable by the justification.

Your take on the correct solution to problem.
Since it might be a non-trivial change, I think we could have a discussion first.

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

[nzakas]

Thanks for the issue. Can you list out what information you would expect for each suppression? It’s not very clear from the snippet.

[Yiwei-Ding]

Hi @nzakas , thanks for the reply.

A suppression entry contains a kind attribute and a justification attribute. kind is either inSource (using configuration comments) or external (using configuration files). It could be an attribute of LintMessage.

When disabling rules using configuration comments, the description after two or more consecutive - characters can be treated as the justification. For example:

/* eslint-disable no-console -- Just for debug. */

/* eslint-disable no-console
    -------
    Just for debug. */

/* eslint-disable no-console
* -----
* Just for debug.
*/

// eslint-disable-line no-console -- Just for debug.

// eslint-disable-next-line no-console -- Just for debug.

Then we may get:

{"kind": "inSource", "justification": "Just for debug."}

When disabling rules using configuration files, it seems no good places to put a justification. We could have a discussion about it. Currently for example:

rules: {"no-console": "off"}

Then we may get:

{"kind": "external", "justification": ""}

If a line is suppressed by multiple rules, a suppression list would be preferred to gather all suppression info:

suppressions: [
  {"kind": "inSource", "justification": "foo"},
  {"kind": "external", "justification": ""}
]

We may have a new CLI option such as --suppression to output suppressions without prompting errors/warnings.

[nzakas]

The two cases you mention are handled very differently in ESLint: configuration files and inline comments.

When rules are disabled inside of configuration files, we don't do any tracking at all and that information is not returned to a formatter. We never run the rules at all so there are no lint messages to display.

When you use eslint-disable inside of a file, we still run the rules but then remove any lint messages that were triggered within the specified range.

I'm still having trouble understanding what a "suppression" actually means. Does it just mean you want to know when a rule was disabled and why? Or does it mean you want the lint message (including location information) but also a reason why it was disabled?

[Yiwei-Ding]

Here is our Dev Spec, which has a clearer description.

Problem

ESLint is a famous tool for identifying and reporting on patterns found in ECMAScript/JavaScript code. When codes violating one or more defined rules, ESLint will report warnings or errors:

"messages": [
  {
    "ruleId":"no-undef",
    "severity":2,
    "message":"'b' is not defined.",
    "line":2,
    "column":1,
    "nodeType":"Identifier",
    "messageId":"undef",
    "endLine":2,
    "endColumn":2
  }
]

However, in our scenarios, when violations are suppressed, Security Development Lifecycle (SDL) tools, such as ESLint, are expected to export suppression information, including its kind and justification. We could take advantage of this info to generate corresponding signals for auditing purposes.

"messages": [
  {
    "ruleId":"no-undef",
    "severity":2,
    "message":"'b' is not defined.",
    "line":2,
    "column":1,
    "nodeType":"Identifier",
    "messageId":"undef",
    "endLine":2,
    "endColumn":2,
    "suppressions": [
      {
        "kind":"inSource",
        "justification":"Justification example"
      }
    ]
  }
]

Suppression in ESLint

There are 2 ways to suppress (or disable in ESLint) warnings/errors (violations), according to disabling-rules:

1. inSource: use directive comments inside of a file
   1. Block comment - Codes between eslint-disable and eslint-enable are suppressed by defined rules. If rules are not specified, any warnings or errors will be suppressed.

   /* eslint-disable no-undef -- Justification example */
   a = 1;
   /* eslint-enable no-undef */

   2. Line comment - eslint-disable-line can be used to suppress current line of code, and eslint-disable-next-line to suppress the next line of code.

   console.log(‘foo’); // eslint-disable-line no-console -- Justification example

   // eslint-disable-next-line no-console -- Justification example
   console.log(‘bar’);

2. External: switch rules to off in configuration files
   Configuration files can be in JavaScript/YAML/JSON formats. For example, in JSON, rules can be switched to off:

   rules: {
     "no-undef": "off"
   }

   Then no-undef rule will be suppressed globally.

When an error or warning is suppressed, it will not be output. Particularly, an option --no-inline-config can disable all inSource suppressions, where all errors and warnings not suppressed globally will be output.

Currently, ESLint cannot dump these output messages into the SARIF format. A tool ESLint.Formatter can be used to convert the output of ESLint to SARIF format:

.\node_modules\.bin\eslint.cmd 1.js -f @microsoft/eslint-formatter-sarif -o 1.sarif

Figure Data Flow below is a high-level understanding of the current design.

• When rules are suppressed globally (external configuration files), ESLint does not do any tracking or run these suppressed rules at all, and suppression information will not be returned to output.
• When rules are suppressed in-source (‘eslint-disable’ inside of a file), ESLint will still run the rules but then remove any lint messages that were triggered with the specified rule(s).

What we need

As mentioned above, what we need is to allow engineers to provide justification in the suppression and output the suppression information.

To minimize changes, we propose to use the -- Description part of the suppression as ‘Justification’.

More specifically, for inSource suppression (or inline comments in ESLint), the description after two or more consecutive dashes can be treated as the justification.

For example:

/* eslint-disable no-undef -- Justification example */
a = 1;
/* eslint-enable no-undef */

console.log("foo"); // eslint-disable-line no-console -- Justification example

// eslint-disable-next-line no-console -- Justification example
console.log("bar");

Then we may get:

{"kind": "inSource", "justification": "Justification example"}

For external suppression (or configuration files in ESLint), there are no good places to put a justification. We may put something general as a justification, like “Globally disabled”.

For example:

{"kind": "external", "justification": "Globally disabled"}

If a line is suppressed by multiple rules, a suppression list would be preferred to gather all suppression information:

suppressions: [
  {"kind": "inSource", "justification": "foo"},
  {"kind": "external", "justification": "Globally disabled"}
]

Goals

1. Keep the current behavior of ESLint, i.e., errors/warnings will not be shown on the command line if the related rules are disabled, unless --no-inline-config is used.
2. When the flag --include-suppression is used, the rules that globally disabled will not be skipped and the violations (problems) that disabled by inline comments will not be removed.
3. Add suppression info in the output of ESLint
   1. For inSource suppressions, we generate suppression info where kind is inSource and justification is the description in the directive comments (words after two or more consecutive dashes mentioned above).
      For example:

      /* eslint-disable no-undef -- Justification example */
      a = 1;
      /* eslint-enable no-undef */

      console.log("foo"); // eslint-disable-line no-console -- Justification example

      // eslint-disable-next-line no-console -- Justification example
      console.log("bar");

      Output:

      {"kind": "inSource", "justification": "Justification example"}

   2. For external suppressions, we generate suppression info where kind is external and justification is a certain sentence such as Globally disabled.
      For example:

      rules: { 
        "no-undef": "off" 
      }

      Output:

      "kind": "external", "justification": "Globally disabled"}

Design

We are proposing to add a new CLI option --include-suppression to output suppressions without prompting disabled errors/warnings.

Architectural Diagram

Input:

.\node_modules\.bin\eslint.cmd 1.js --include-suppression -f @microsoft/eslint-formatter-sarif -o 1.sarif

Output:

Add suppressions attribute in LintMessage

LintMessage is the type that finally output to the formatter. Currently it contains ruleId, severity, message, etc. What we desire is a suppressions attribute in LintMessage. It should be a list that contains zero or more suppression entries ({kind: "external/inSource", justification: "Fake justification message."}).

Reserve external suppression

In the step of apply external suppression, ESLint will not skip rules disabled in configuration files if it gets the --include-suppression flag. Violations (or problems in ESLint) will be created with {kind: "external", justification: "Globally disabled"} if the related rules are switched to off.

More specifically, after getting severity in runRules, ESLint will not return if severity === 0 && includeSuppression. A dictionary {kind: "external", justification: ""} will be put into createReportTranslator and createProblem. These problems will go to the next step.

NOTE that globally disabled rules could be enabled in applying disable directives. In that case we should remove the corresponding suppression info.

Reserve inSource suppression

In the step of apply inSource suppression, ESLint will not remove the problems that should be disabled by directive comments if it gets the --include-suppression flag. These problems will be reserved and added {kind: "inSource", justification: "Fake justification message."}.

[nzakas]

Okay, that’s a lot of information to digest and it’s hard with an issue comment. What I’d like to suggest is that you create a formal RFC so we can review and make inline comments on areas where we need more information.

[github-actions]

Oops! It looks like we lost track of this issue. @eslint/eslint-tsc what do we want to do here? This issue will auto-close in 7 days without an update.

[nzakas]

Keeping this open while we work on the RFC