fix: unify LintMessage type (#17076)

* Update LintMessage type to reflect implementation

I made three changes to `LintMessage` and `SuppressedLintMessage`

1. Many places that create `LintMessage`s don't set `fatal`, so I made
   it optional. This would be a breaking change if we made types part of
   our exports, but in this case I believe it's updating JSDoc
   documentation to reflect reality.
2. Added an optional `messageId` property that's set by reports from rules.
3. Added an optional, nullable `nodeType` property.
    - Reports from rules set it to a value.
    - `applyDirectives()` explicitly sets it to `null` for unused
      disable directives.
    - `Linter`'s `createLintingProblem()` explicitly sets it to `null`.

* Add missing LintResult type import

* Use LintMessage type

All of these previously referenced a `Problem` type that does not have a
definition.

* Replace ReportInfo type with LintMessage type

`ReportInfo` was defined within `report-translator.js` to be very
similar to `LintMessage`. It had one extra property, `source`, which
wasn't ever set, so it would've been removed anyway. In
`Linter.runRules()`, the return value from `reportTranslator()` gets
pushed into a `lintingProblems` array and returned, and the return type
annotation is `LintMessage[]`, which gives me confidence that
`ReportInfo` was intended to be the same type as `LintMessage`
elsewhere.

* Make ruleId required but nullable

Originally, we talked about the reverse - making `ruleId` optional but
non-nullable and relying on `report-translator.js`'s `createProblem()`
to normalize it. However, the `LintMessage` type would differ from
`createProblem()`'s return value only by `null` vs `undefined` for
`ruleId`. So instead, I made `ruleId` required but nullable and
explicitly set it to `null` in the few remaining places that didn't
already.

* Add missing null nodeTypes

`LintMessage.nodeType` is currently defined as required but nullable.
Actual implementations explicitly set it to `null` in a couple places
and omit it in several.

After discussion in #16968, we initially leaned toward making it
non-nullable but optional. I pursued that, but it resulted in slightly
more runtime code changes, including some branching in
`report-translator` to set it conditionally.

Instead, I'm presenting the opposite solution of updating the remaining
implementations to match the existing type definition by explicitly
setting `nodeType` to `null`.

* Update LintMessage docs

This updates existing documented properties to match the updated `LintMessage`
type and implementations.

`messageId`, `nodeType`, and `suppressions` remain undocumented.

Author: btmills

docs/src/extend/custom-processors.md

@@ -59,19 +59,19 @@ Reported problems have the following location information in each lint message:
 type LintMessage = {
 
   /// The 1-based line number where the message occurs.
-  line: number;
+  line?: number;
 
    /// The 1-based column number where the message occurs.
-  column: number;
+  column?: number;
 
   /// The 1-based line number of the end location.
-  endLine: number;
+  endLine?: number;
 
   /// The 1-based column number of the end location.
-  endColumn: number;
+  endColumn?: number;
 
   /// If `true`, this is a fatal error.
-  fatal: boolean;
+  fatal?: boolean;
 
   /// Information for an autofix.
   fix: Fix;

lib/cli-engine/cli-engine.js

@@ -308,9 +308,11 @@ function createIgnoreResult(filePath, baseDir) {
         filePath: path.resolve(filePath),
         messages: [
             {
+                ruleId: null,
                 fatal: false,
                 severity: 1,
-                message
+                message,
+                nodeType: null
             }
         ],
         suppressedMessages: [],

lib/eslint/eslint-helpers.js

@@ -607,9 +607,11 @@ function createIgnoreResult(filePath, baseDir) {
         filePath: path.resolve(filePath),
         messages: [
             {
+                ruleId: null,
                 fatal: false,
                 severity: 1,
-                message
+                message,
+                nodeType: null
             }
         ],
         suppressedMessages: [],

lib/eslint/flat-eslint.js

@@ -55,6 +55,7 @@ const LintResultCache = require("../cli-engine/lint-result-cache");
 /** @typedef {import("../shared/types").ConfigData} ConfigData */
 /** @typedef {import("../shared/types").DeprecatedRuleInfo} DeprecatedRuleInfo */
 /** @typedef {import("../shared/types").LintMessage} LintMessage */
+/** @typedef {import("../shared/types").LintResult} LintResult */
 /** @typedef {import("../shared/types").ParserOptions} ParserOptions */
 /** @typedef {import("../shared/types").Plugin} Plugin */
 /** @typedef {import("../shared/types").ResultsMeta} ResultsMeta */

lib/linter/apply-disable-directives.js

@@ -5,6 +5,16 @@
 
 "use strict";
 
+//------------------------------------------------------------------------------
+// Typedefs
+//------------------------------------------------------------------------------
+
+/** @typedef {import("../shared/types").LintMessage} LintMessage */
+
+//------------------------------------------------------------------------------
+// Module Definition
+//------------------------------------------------------------------------------
+
 const escapeRegExp = require("escape-string-regexp");
 
 /**
@@ -196,7 +206,7 @@ function processUnusedDisableDirectives(allDirectives) {
  * @param {Object} options options for applying directives. This is the same as the options
  * for the exported function, except that `reportUnusedDisableDirectives` is not supported
  * (this function always reports unused disable directives).
- * @returns {{problems: Problem[], unusedDisableDirectives: Problem[]}} An object with a list
+ * @returns {{problems: LintMessage[], unusedDisableDirectives: LintMessage[]}} An object with a list
  * of problems (including suppressed ones) and unused eslint-disable directives
  */
 function applyDirectives(options) {

lib/linter/config-comment-parser.js

@@ -19,6 +19,12 @@ const levn = require("levn"),
 
 const debug = require("debug")("eslint:config-comment-parser");
 
+//------------------------------------------------------------------------------
+// Typedefs
+//------------------------------------------------------------------------------
+
+/** @typedef {import("../shared/types").LintMessage} LintMessage */
+
 //------------------------------------------------------------------------------
 // Public Interface
 //------------------------------------------------------------------------------
@@ -61,7 +67,7 @@ module.exports = class ConfigCommentParser {
      * Parses a JSON-like config.
      * @param {string} string The string to parse.
      * @param {Object} location Start line and column of comments for potential error message.
-     * @returns {({success: true, config: Object}|{success: false, error: Problem})} Result map object
+     * @returns {({success: true, config: Object}|{success: false, error: LintMessage})} Result map object
      */
     parseJsonConfig(string, location) {
         debug("Parsing JSON config");
@@ -109,7 +115,8 @@ module.exports = class ConfigCommentParser {
                     severity: 2,
                     message: `Failed to parse JSON from '${normalizedString}': ${ex.message}`,
                     line: location.start.line,
-                    column: location.start.column + 1
+                    column: location.start.column + 1,
+                    nodeType: null
                 }
             };
 

lib/linter/linter.js

@@ -364,7 +364,7 @@ function extractDirectiveComment(value) {
  * @param {ASTNode} ast The top node of the AST.
  * @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules
  * @param {string|null} warnInlineConfig If a string then it should warn directive comments as disabled. The string value is the config name what the setting came from.
- * @returns {{configuredRules: Object, enabledGlobals: {value:string,comment:Token}[], exportedVariables: Object, problems: Problem[], disableDirectives: DisableDirective[]}}
+ * @returns {{configuredRules: Object, enabledGlobals: {value:string,comment:Token}[], exportedVariables: Object, problems: LintMessage[], disableDirectives: DisableDirective[]}}
  * A collection of the directive comments that were found, along with any problems that occurred when parsing
  */
 function getDirectiveComments(ast, ruleMapper, warnInlineConfig) {
@@ -775,7 +775,7 @@ function analyzeScope(ast, languageOptions, visitorKeys) {
  * @param {string} text The text to parse.
  * @param {LanguageOptions} languageOptions Options to pass to the parser
  * @param {string} filePath The path to the file being parsed.
- * @returns {{success: false, error: Problem}|{success: true, sourceCode: SourceCode}}
+ * @returns {{success: false, error: LintMessage}|{success: true, sourceCode: SourceCode}}
  * An object containing the AST and parser services if parsing was successful, or the error if parsing failed
  * @private
  */
@@ -851,7 +851,8 @@ function parse(text, languageOptions, filePath) {
                 severity: 2,
                 message,
                 line: ex.lineNumber,
-                column: ex.column
+                column: ex.column,
+                nodeType: null
             }
         };
     }
@@ -921,7 +922,7 @@ const BASE_TRAVERSAL_CONTEXT = Object.freeze(
  * @param {boolean} disableFixes If true, it doesn't make `fix` properties.
  * @param {string | undefined} cwd cwd of the cli
  * @param {string} physicalFilename The full path of the file on disk without any code block information
- * @returns {Problem[]} An array of reported problems
+ * @returns {LintMessage[]} An array of reported problems
  */
 function runRules(sourceCode, configuredRules, ruleMapper, parserName, languageOptions, settings, filename, disableFixes, cwd, physicalFilename) {
     const emitter = createEmitter();
@@ -1253,7 +1254,8 @@ class Linter {
                     severity: 2,
                     message: `Configured parser '${config.parser}' was not found.`,
                     line: 0,
-                    column: 0
+                    column: 0,
+                    nodeType: null
                 }];
             }
             parserName = config.parser;
@@ -1464,7 +1466,8 @@ class Linter {
                     severity: 2,
                     message,
                     line: ex.lineNumber,
-                    column: ex.column
+                    column: ex.column,
+                    nodeType: null
                 }
             ];
         }
@@ -1729,7 +1732,8 @@ class Linter {
                     severity: 1,
                     message: `No matching configuration found for ${filename}.`,
                     line: 0,
-                    column: 0
+                    column: 0,
+                    nodeType: null
                 }
             ];
         }
@@ -1794,7 +1798,8 @@ class Linter {
                     severity: 2,
                     message,
                     line: ex.lineNumber,
-                    column: ex.column
+                    column: ex.column,
+                    nodeType: null
                 }
             ];
         }
@@ -1840,7 +1845,7 @@ class Linter {
     /**
      * Given a list of reported problems, distinguish problems between normal messages and suppressed messages.
      * The normal messages will be returned and the suppressed messages will be stored as lastSuppressedMessages.
-     * @param {Problem[]} problems A list of reported problems.
+     * @param {Array<LintMessage|SuppressedLintMessage>} problems A list of reported problems.
      * @returns {LintMessage[]} A list of LintMessage.
      */
     _distinguishSuppressedMessages(problems) {

lib/linter/report-translator.js

@@ -17,6 +17,8 @@ const interpolate = require("./interpolate");
 // Typedefs
 //------------------------------------------------------------------------------
 
+/** @typedef {import("../shared/types").LintMessage} LintMessage */
+
 /**
  * An error message description
  * @typedef {Object} MessageDescriptor
@@ -29,23 +31,6 @@ const interpolate = require("./interpolate");
  * @property {Array<{desc?: string, messageId?: string, fix: Function}>} suggest Suggestion descriptions and functions to create a the associated fixes.
  */
 
-/**
- * Information about the report
- * @typedef {Object} ReportInfo
- * @property {string} ruleId The rule ID
- * @property {(0|1|2)} severity Severity of the error
- * @property {(string|undefined)} message The message
- * @property {(string|undefined)} [messageId] The message ID
- * @property {number} line The line number
- * @property {number} column The column number
- * @property {(number|undefined)} [endLine] The ending line number
- * @property {(number|undefined)} [endColumn] The ending column number
- * @property {(string|null)} nodeType Type of node
- * @property {string} source Source text
- * @property {({text: string, range: (number[]|null)}|null)} [fix] The fix object
- * @property {Array<{text: string, range: (number[]|null)}|null>} [suggestions] Suggestion info
- */
-
 //------------------------------------------------------------------------------
 // Module Definition
 //------------------------------------------------------------------------------
@@ -239,7 +224,7 @@ function mapSuggestions(descriptor, sourceCode, messages) {
  * @param {{start: SourceLocation, end: (SourceLocation|null)}} options.loc Start and end location
  * @param {{text: string, range: (number[]|null)}} options.fix The fix object
  * @param {Array<{text: string, range: (number[]|null)}>} options.suggestions The array of suggestions objects
- * @returns {function(...args): ReportInfo} Function that returns information about the report
+ * @returns {LintMessage} Information about the report
  */
 function createProblem(options) {
     const problem = {
@@ -314,7 +299,7 @@ function validateSuggestions(suggest, messages) {
  * problem for the Node.js API.
  * @param {{ruleId: string, severity: number, sourceCode: SourceCode, messageIds: Object, disableFixes: boolean}} metadata Metadata for the reported problem
  * @param {SourceCode} sourceCode The `SourceCode` instance for the text being linted
- * @returns {function(...args): ReportInfo} Function that returns information about the report
+ * @returns {function(...args): LintMessage} Function that returns information about the report
  */
 
 module.exports = function createReportTranslator(metadata) {

lib/shared/types.js

@@ -96,10 +96,12 @@ module.exports = {};
  * @property {number|undefined} column The 1-based column number.
  * @property {number} [endColumn] The 1-based column number of the end location.
  * @property {number} [endLine] The 1-based line number of the end location.
- * @property {boolean} fatal If `true` then this is a fatal error.
+ * @property {boolean} [fatal] If `true` then this is a fatal error.
  * @property {{range:[number,number], text:string}} [fix] Information for autofix.
  * @property {number|undefined} line The 1-based line number.
  * @property {string} message The error message.
+ * @property {string} [messageId] The ID of the message in the rule's meta.
+ * @property {(string|null)} nodeType Type of node
  * @property {string|null} ruleId The ID of the rule which makes this message.
  * @property {0|1|2} severity The severity of this message.
  * @property {Array<{desc?: string, messageId?: string, fix: {range: [number, number], text: string}}>} [suggestions] Information for suggestions.
@@ -110,10 +112,12 @@ module.exports = {};
  * @property {number|undefined} column The 1-based column number.
  * @property {number} [endColumn] The 1-based column number of the end location.
  * @property {number} [endLine] The 1-based line number of the end location.
- * @property {boolean} fatal If `true` then this is a fatal error.
+ * @property {boolean} [fatal] If `true` then this is a fatal error.
  * @property {{range:[number,number], text:string}} [fix] Information for autofix.
  * @property {number|undefined} line The 1-based line number.
  * @property {string} message The error message.
+ * @property {string} [messageId] The ID of the message in the rule's meta.
+ * @property {(string|null)} nodeType Type of node
  * @property {string|null} ruleId The ID of the rule which makes this message.
  * @property {0|1|2} severity The severity of this message.
  * @property {Array<{kind: string, justification: string}>} suppressions The suppression info.

tests/lib/cli-engine/cli-engine.js

@@ -577,7 +577,8 @@ describe("CLIEngine", () => {
                                 severity: 2,
                                 message: "Parsing error: Unexpected token is",
                                 line: 1,
-                                column: 19
+                                column: 19,
+                                nodeType: null
                             }
                         ],
                         suppressedMessages: [],
@@ -622,7 +623,8 @@ describe("CLIEngine", () => {
                                 severity: 2,
                                 message: "Parsing error: Unexpected token",
                                 line: 1,
-                                column: 10
+                                column: 10,
+                                nodeType: null
                             }
                         ],
                         suppressedMessages: [],
@@ -713,7 +715,8 @@ describe("CLIEngine", () => {
                                 severity: 2,
                                 message: "Parsing error: Unexpected token is",
                                 line: 1,
-                                column: 19
+                                column: 19,
+                                nodeType: null
                             }
                         ],
                         suppressedMessages: [],