feat: Add and use SourceCode#getLoc/getRange (#18703)

* feat: Add and use SourceCode#getLoc/getRange * Update report-translator * Update rule-fixer * Upgrade @eslint/json * Fix FixTracker tests
eslint · Jul 24, 2024 · 13d0bd3 · 13d0bd3
1 parent 282df1a
commit 13d0bd3
Show file tree

Hide file tree

Showing 9 changed files with 244 additions and 58 deletions.
diff --git a/lib/languages/js/source-code/source-code.js b/lib/languages/js/source-code/source-code.js
@@ -916,6 +916,25 @@ class SourceCode extends TokenStore {
 
         return ancestorsStartingAtParent.reverse();
     }
+
+    /**
+     * Returns the locatin of the given node or token.
+     * @param {ASTNode|Token} nodeOrToken The node or token to get the location of.
+     * @returns {SourceLocation} The location of the node or token.
+     */
+    getLoc(nodeOrToken) {
+        return nodeOrToken.loc;
+    }
+
+    /**
+     * Returns the range of the given node or token.
+     * @param {ASTNode|Token} nodeOrToken The node or token to get the range of.
+     * @returns {[number, number]} The range of the node or token.
+     */
+    getRange(nodeOrToken) {
+        return nodeOrToken.range;
+    }
+
     /* eslint-enable class-methods-use-this -- node is owned by SourceCode */
 
     /**

diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js
@@ -61,15 +61,18 @@ function groupByParentDirective(directives) {
  * Creates removal details for a set of directives within the same comment.
  * @param {Directive[]} directives Unused directives to be removed.
  * @param {Token} node The backing Comment token.
+ * @param {SourceCode} sourceCode The source code object for the file being linted.
  * @returns {{ description, fix, unprocessedDirective }[]} Details for later creation of output Problems.
  */
-function createIndividualDirectivesRemoval(directives, node) {
+function createIndividualDirectivesRemoval(directives, node, sourceCode) {
+
+    const range = sourceCode.getRange(node);
 
     /*
      * `node.value` starts right after `//` or `/*`.
      * All calculated offsets will be relative to this index.
      */
-    const commentValueStart = node.range[0] + "//".length;
+    const commentValueStart = range[0] + "//".length;
 
     // Find where the list of rules starts. `\S+` matches with the directive name (e.g. `eslint-disable-line`)
     const listStartOffset = /^\s*\S+\s+/u.exec(node.value)[0].length;
@@ -165,10 +168,11 @@ function createIndividualDirectivesRemoval(directives, node) {
  * Creates a description of deleting an entire unused disable directive.
  * @param {Directive[]} directives Unused directives to be removed.
  * @param {Token} node The backing Comment token.
+ * @param {SourceCode} sourceCode The source code object for the file being linted.
  * @returns {{ description, fix, unprocessedDirective }} Details for later creation of an output problem.
  */
-function createDirectiveRemoval(directives, node) {
-    const { range } = node;
+function createDirectiveRemoval(directives, node, sourceCode) {
+    const range = sourceCode.getRange(node);
     const ruleIds = directives.filter(directive => directive.ruleId).map(directive => `'${directive.ruleId}'`);
 
     return {
@@ -186,9 +190,10 @@ function createDirectiveRemoval(directives, node) {
 /**
  * Parses details from directives to create output Problems.
  * @param {Iterable<Directive>} allDirectives Unused directives to be removed.
+ * @param {SourceCode} sourceCode The source code object for the file being linted.
  * @returns {{ description, fix, unprocessedDirective }[]} Details for later creation of output Problems.
  */
-function processUnusedDirectives(allDirectives) {
+function processUnusedDirectives(allDirectives, sourceCode) {
     const directiveGroups = groupByParentDirective(allDirectives);
 
     return directiveGroups.flatMap(
@@ -201,8 +206,8 @@ function processUnusedDirectives(allDirectives) {
             }
 
             return remainingRuleIds.size
-                ? createIndividualDirectivesRemoval(directives, parentDirective.node)
-                : [createDirectiveRemoval(directives, parentDirective.node)];
+                ? createIndividualDirectivesRemoval(directives, parentDirective.node, sourceCode)
+                : [createDirectiveRemoval(directives, parentDirective.node, sourceCode)];
         }
     );
 }
@@ -309,6 +314,7 @@ function collectUsedEnableDirectives(directives) {
 function applyDirectives(options) {
     const problems = [];
     const usedDisableDirectives = new Set();
+    const { sourceCode } = options;
 
     for (const problem of options.problems) {
         let disableDirectivesForProblem = [];
@@ -370,8 +376,8 @@ function applyDirectives(options) {
         }
     }
 
-    const processed = processUnusedDirectives(unusedDisableDirectivesToReport)
-        .concat(processUnusedDirectives(unusedEnableDirectivesToReport));
+    const processed = processUnusedDirectives(unusedDisableDirectivesToReport, sourceCode)
+        .concat(processUnusedDirectives(unusedEnableDirectivesToReport, sourceCode));
     const columnOffset = options.language.columnStart === 1 ? 0 : 1;
     const lineOffset = options.language.lineStart === 1 ? 0 : 1;
 
@@ -390,11 +396,14 @@ function applyDirectives(options) {
                     ? `Unused eslint-disable directive (no problems were reported from ${description}).`
                     : "Unused eslint-disable directive (no problems were reported).";
             }
+
+            const loc = sourceCode.getLoc(parentDirective.node);
+
             return {
                 ruleId: null,
                 message,
-                line: type === "disable-next-line" ? parentDirective.node.loc.start.line + lineOffset : line,
-                column: type === "disable-next-line" ? parentDirective.node.loc.start.column + columnOffset : column,
+                line: type === "disable-next-line" ? loc.start.line + lineOffset : line,
+                column: type === "disable-next-line" ? loc.start.column + columnOffset : column,
                 severity: options.reportUnusedDisableDirectives === "warn" ? 1 : 2,
                 nodeType: null,
                 ...options.disableFixes ? {} : { fix }
@@ -409,6 +418,7 @@ function applyDirectives(options) {
  * of reported problems, adds the suppression information to the problems.
  * @param {Object} options Information about directives and problems
  * @param {Language} options.language The language being linted.
+ * @param {SourceCode} options.sourceCode The source code object for the file being linted.
  * @param {{
  *      type: ("disable"|"enable"|"disable-line"|"disable-next-line"),
  *      ruleId: (string|null),
@@ -427,7 +437,7 @@ function applyDirectives(options) {
  * @returns {{ruleId: (string|null), line: number, column: number, suppressions?: {kind: string, justification: string}}[]}
  * An object with a list of reported problems, the suppressed of which contain the suppression information.
  */
-module.exports = ({ language, directives, disableFixes, problems, configuredRules, ruleFilter, reportUnusedDisableDirectives = "off" }) => {
+module.exports = ({ language, sourceCode, directives, disableFixes, problems, configuredRules, ruleFilter, reportUnusedDisableDirectives = "off" }) => {
     const blockDirectives = directives
         .filter(directive => directive.type === "disable" || directive.type === "enable")
         .map(directive => Object.assign({}, directive, { unprocessedDirective: directive }))
@@ -477,6 +487,7 @@ module.exports = ({ language, directives, disableFixes, problems, configuredRule
 
     const blockDirectivesResult = applyDirectives({
         language,
+        sourceCode,
         problems,
         directives: blockDirectives,
         disableFixes,
@@ -485,6 +496,7 @@ module.exports = ({ language, directives, disableFixes, problems, configuredRule
     });
     const lineDirectivesResult = applyDirectives({
         language,
+        sourceCode,
         problems: blockDirectivesResult.problems,
         directives: lineDirectives,
         disableFixes,

diff --git a/lib/linter/linter.js b/lib/linter/linter.js
@@ -323,9 +323,10 @@ function createLintingProblem(options) {
  * @param {ASTNode|token} options.node The Comment node/token.
  * @param {function(string): {create: Function}} ruleMapper A map from rule IDs to defined rules
  * @param {Language} language The language to use to adjust the location information.
+ * @param {SourceCode} sourceCode The SourceCode object to get comments from.
  * @returns {Object} Directives and problems from the comment
  */
-function createDisableDirectives({ type, value, justification, node }, ruleMapper, language) {
+function createDisableDirectives({ type, value, justification, node }, ruleMapper, language, sourceCode) {
     const ruleIds = Object.keys(commentParser.parseListConfig(value));
     const directiveRules = ruleIds.length ? ruleIds : [null];
     const result = {
@@ -336,11 +337,15 @@ function createDisableDirectives({ type, value, justification, node }, ruleMappe
 
     for (const ruleId of directiveRules) {
 
+        const loc = sourceCode.getLoc(node);
+
         // push to directives, if the rule is defined(including null, e.g. /*eslint enable*/)
         if (ruleId === null || !!ruleMapper(ruleId)) {
+
+
             if (type === "disable-next-line") {
                 const { line, column } = updateLocationInformation(
-                    node.loc.end,
+                    loc.end,
                     language
                 );
 
@@ -354,7 +359,7 @@ function createDisableDirectives({ type, value, justification, node }, ruleMappe
                 });
             } else {
                 const { line, column } = updateLocationInformation(
-                    node.loc.start,
+                    loc.start,
                     language
                 );
 
@@ -368,7 +373,7 @@ function createDisableDirectives({ type, value, justification, node }, ruleMappe
                 });
             }
         } else {
-            result.directiveProblems.push(createLintingProblem({ ruleId, loc: node.loc, language }));
+            result.directiveProblems.push(createLintingProblem({ ruleId, loc, language }));
         }
     }
     return result;
@@ -410,25 +415,27 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
             return;
         }
 
+        const loc = sourceCode.getLoc(comment);
+
         if (warnInlineConfig) {
             const kind = comment.type === "Block" ? `/*${directiveText}*/` : `//${directiveText}`;
 
             problems.push(createLintingProblem({
                 ruleId: null,
                 message: `'${kind}' has no effect because you have 'noInlineConfig' setting in ${warnInlineConfig}.`,
-                loc: comment.loc,
+                loc,
                 severity: 1
             }));
             return;
         }
 
-        if (directiveText === "eslint-disable-line" && comment.loc.start.line !== comment.loc.end.line) {
+        if (directiveText === "eslint-disable-line" && loc.start.line !== loc.end.line) {
             const message = `${directiveText} comment should not span multiple lines.`;
 
             problems.push(createLintingProblem({
                 ruleId: null,
                 message,
-                loc: comment.loc
+                loc
             }));
             return;
         }
@@ -446,7 +453,7 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
                     value: directiveValue,
                     justification: justificationPart,
                     node: comment
-                }, ruleMapper, jslang);
+                }, ruleMapper, jslang, sourceCode);
 
                 disableDirectives.push(...directives);
                 problems.push(...directiveProblems);
@@ -467,7 +474,7 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
                     } catch (err) {
                         problems.push(createLintingProblem({
                             ruleId: null,
-                            loc: comment.loc,
+                            loc,
                             message: err.message
                         }));
                         continue;
@@ -494,14 +501,14 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
                         const ruleValue = parseResult.config[name];
 
                         if (!rule) {
-                            problems.push(createLintingProblem({ ruleId: name, loc: comment.loc }));
+                            problems.push(createLintingProblem({ ruleId: name, loc }));
                             return;
                         }
 
                         if (Object.hasOwn(configuredRules, name)) {
                             problems.push(createLintingProblem({
                                 message: `Rule "${name}" is already configured by another configuration comment in the preceding code. This configuration is ignored.`,
-                                loc: comment.loc
+                                loc
                             }));
                             return;
                         }
@@ -563,7 +570,7 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
                             problems.push(createLintingProblem({
                                 ruleId: name,
                                 message: err.message,
-                                loc: comment.loc
+                                loc
                             }));
 
                             // do not apply the config, if found invalid options.
@@ -575,7 +582,7 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
                 } else {
                     const problem = createLintingProblem({
                         ruleId: null,
-                        loc: comment.loc,
+                        loc,
                         message: parseResult.error.message
                     });
 
@@ -623,7 +630,7 @@ function getDirectiveCommentsForFlatConfig(sourceCode, ruleMapper, language) {
         })));
 
         directivesSources.forEach(directive => {
-            const { directives, directiveProblems } = createDisableDirectives(directive, ruleMapper, language);
+            const { directives, directiveProblems } = createDisableDirectives(directive, ruleMapper, language, sourceCode);
 
             disableDirectives.push(...directives);
             problems.push(...directiveProblems);
@@ -1473,7 +1480,7 @@ class Linter {
             debug("An error occurred while traversing");
             debug("Filename:", options.filename);
             if (err.currentNode) {
-                const { line } = err.currentNode.loc.start;
+                const { line } = sourceCode.getLoc(err.currentNode).start;
 
                 debug("Line:", line);
                 err.message += `:${line}`;
@@ -1491,6 +1498,7 @@ class Linter {
 
         return applyDisableDirectives({
             language: jslang,
+            sourceCode,
             directives: commentDirectives.disableDirectives,
             disableFixes: options.disableFixes,
             problems: lintingProblems
@@ -1770,10 +1778,14 @@ class Linter {
             if (options.warnInlineConfig) {
                 if (sourceCode.getInlineConfigNodes) {
                     sourceCode.getInlineConfigNodes().forEach(node => {
+
+                        const loc = sourceCode.getLoc(node);
+                        const range = sourceCode.getRange(node);
+
                         inlineConfigProblems.push(createLintingProblem({
                             ruleId: null,
-                            message: `'${sourceCode.text.slice(node.range[0], node.range[1])}' has no effect because you have 'noInlineConfig' setting in ${options.warnInlineConfig}.`,
-                            loc: node.loc,
+                            message: `'${sourceCode.text.slice(range[0], range[1])}' has no effect because you have 'noInlineConfig' setting in ${options.warnInlineConfig}.`,
+                            loc,
                             severity: 1,
                             language: config.language
                         }));
@@ -1953,7 +1965,7 @@ class Linter {
             debug("An error occurred while traversing");
             debug("Filename:", options.filename);
             if (err.currentNode) {
-                const { line } = err.currentNode.loc.start;
+                const { line } = sourceCode.getLoc(err.currentNode).start;
 
                 debug("Line:", line);
                 err.message += `:${line}`;
@@ -1972,6 +1984,7 @@ class Linter {
 
         return applyDisableDirectives({
             language: config.language,
+            sourceCode,
             directives: commentDirectives.disableDirectives,
             disableFixes: options.disableFixes,
             problems: lintingProblems

diff --git a/lib/linter/report-translator.js b/lib/linter/report-translator.js
@@ -10,7 +10,7 @@
 //------------------------------------------------------------------------------
 
 const assert = require("node:assert");
-const ruleFixer = require("./rule-fixer");
+const { RuleFixer } = require("./rule-fixer");
 const { interpolate } = require("./interpolate");
 
 //------------------------------------------------------------------------------
@@ -91,13 +91,10 @@ function assertValidNodeInfo(descriptor) {
  * from the `node` of the original descriptor, or infers the `start` from the `loc` of the original descriptor.
  */
 function normalizeReportLoc(descriptor) {
-    if (descriptor.loc) {
-        if (descriptor.loc.start) {
-            return descriptor.loc;
-        }
-        return { start: descriptor.loc, end: null };
+    if (descriptor.loc.start) {
+        return descriptor.loc;
     }
-    return descriptor.node.loc;
+    return { start: descriptor.loc, end: null };
 }
 
 /**
@@ -190,6 +187,8 @@ function normalizeFixes(descriptor, sourceCode) {
         return null;
     }
 
+    const ruleFixer = new RuleFixer({ sourceCode });
+
     // @type {null | Fix | Fix[] | IterableIterator<Fix>}
     const fix = descriptor.fix(ruleFixer);
 
@@ -335,6 +334,7 @@ module.exports = function createReportTranslator(metadata) {
     return (...args) => {
         const descriptor = normalizeMultiArgReportCall(...args);
         const messages = metadata.messageIds;
+        const { sourceCode } = metadata;
 
         assertValidNodeInfo(descriptor);
 
@@ -367,9 +367,9 @@ module.exports = function createReportTranslator(metadata) {
             node: descriptor.node,
             message: interpolate(computedMessage, descriptor.data),
             messageId: descriptor.messageId,
-            loc: normalizeReportLoc(descriptor),
-            fix: metadata.disableFixes ? null : normalizeFixes(descriptor, metadata.sourceCode),
-            suggestions: metadata.disableFixes ? [] : mapSuggestions(descriptor, metadata.sourceCode, messages),
+            loc: descriptor.loc ? normalizeReportLoc(descriptor) : sourceCode.getLoc(descriptor.node),
+            fix: metadata.disableFixes ? null : normalizeFixes(descriptor, sourceCode),
+            suggestions: metadata.disableFixes ? [] : mapSuggestions(descriptor, sourceCode, messages),
             language: metadata.language
         });
     };