Fix: refactor RuleContext to not modify report locations (fixes #8980) (

#8997)

This refactors `RuleContext` and updates it to simply pass report locations directly to `Linter`, rather than modifying them beforehand. `Linter` already contains all the necessary functionality for handling locations, so it's not necessary to duplicate any logic in `RuleContext`. The duplication caused a bug where `Linter` was modified and `RuleContext` was not.

lib/rule-context.js

@@ -20,6 +20,7 @@ const PASSTHROUGHS = [
     "getDeclaredVariables",
     "getFilename",
     "getScope",
+    "getSourceCode",
     "markVariableAsUsed",
 
     // DEPRECATED
@@ -58,7 +59,7 @@ const PASSTHROUGHS = [
  */
 
 //------------------------------------------------------------------------------
-// Rule Definition
+// Module Definition
 //------------------------------------------------------------------------------
 
 /**
@@ -132,13 +133,13 @@ function getFix(descriptor, sourceCode) {
 
 /**
  * Rule context class
- * Acts as an abstraction layer between rules and the main eslint object.
+ * Acts as an abstraction layer between rules and the main linter object.
  */
 class RuleContext {
 
     /**
      * @param {string} ruleId The ID of the rule using this object.
-     * @param {eslint} eslint The eslint object.
+     * @param {Linter} linter The linter object.
      * @param {number} severity The configured severity level of the rule.
      * @param {Array} options The configuration information to be added to the rule.
      * @param {Object} settings The configuration settings passed from the config file.
@@ -147,7 +148,7 @@ class RuleContext {
      * @param {Object} meta The metadata of the rule
      * @param {Object} parserServices The parser services for the rule.
      */
-    constructor(ruleId, eslint, severity, options, settings, parserOptions, parserPath, meta, parserServices) {
+    constructor(ruleId, linter, severity, options, settings, parserOptions, parserPath, meta, parserServices) {
 
         // public.
         this.id = ruleId;
@@ -161,22 +162,14 @@ class RuleContext {
         this.parserServices = Object.freeze(Object.assign({}, parserServices));
 
         // private.
-        this.eslint = eslint;
-        this.severity = severity;
+        this._linter = linter;
+        this._severity = severity;
 
         Object.freeze(this);
     }
 
     /**
-     * Passthrough to eslint.getSourceCode().
-     * @returns {SourceCode} The SourceCode object for the code.
-     */
-    getSourceCode() {
-        return this.eslint.getSourceCode();
-    }
-
-    /**
-     * Passthrough to eslint.report() that automatically assigns the rule ID and severity.
+     * Passthrough to Linter#report() that automatically assigns the rule ID and severity.
      * @param {ASTNode|MessageDescriptor} nodeOrDescriptor The AST node related to the message or a message
      *      descriptor.
      * @param {Object=} location The location of the error.
@@ -192,38 +185,57 @@ class RuleContext {
             const descriptor = nodeOrDescriptor;
             const fix = getFix(descriptor, this.getSourceCode());
 
-            this.eslint.report(
+            if (descriptor.loc) {
+                this._linter.report(
+                    this.id,
+                    this._severity,
+                    descriptor.node,
+                    descriptor.loc,
+                    descriptor.message,
+                    descriptor.data,
+                    fix,
+                    this.meta
+                );
+            } else {
+                this._linter.report(
+                    this.id,
+                    this._severity,
+                    descriptor.node,
+
+                    /* loc not provided */
+                    descriptor.message,
+                    descriptor.data,
+                    fix,
+                    this.meta
+                );
+            }
+
+        } else {
+
+            // old style call
+            this._linter.report(
                 this.id,
-                this.severity,
-                descriptor.node,
-                descriptor.loc || descriptor.node.loc.start,
-                descriptor.message,
-                descriptor.data,
-                fix,
+                this._severity,
+                nodeOrDescriptor,
+                location,
+                message,
+                opts,
                 this.meta
             );
-
-            return;
         }
-
-        // old style call
-        this.eslint.report(
-            this.id,
-            this.severity,
-            nodeOrDescriptor,
-            location,
-            message,
-            opts,
-            this.meta
-        );
     }
 }
 
-// Copy over passthrough methods. All functions will have 5 or fewer parameters.
-PASSTHROUGHS.forEach(function(name) {
-    this[name] = function(a, b, c, d, e) {
-        return this.eslint[name](a, b, c, d, e);
-    };
-}, RuleContext.prototype);
+// Copy over passthrough methods.
+PASSTHROUGHS.forEach(name => {
+    Object.defineProperty(RuleContext.prototype, name, {
+        value() {
+            return this._linter[name].apply(this._linter, arguments);
+        },
+        configurable: true,
+        writable: true,
+        enumerable: false
+    });
+});
 
 module.exports = RuleContext;

tests/lib/cli-engine.js

@@ -355,6 +355,8 @@ describe("CLIEngine", () => {
                                 message: "'foo' is not defined.",
                                 line: 1,
                                 column: 11,
+                                endLine: 1,
+                                endColumn: 14,
                                 nodeType: "Identifier",
                                 source: "var bar = foo"
                             }
@@ -1413,6 +1415,8 @@ describe("CLIEngine", () => {
                                 {
                                     column: 18,
                                     line: 1,
+                                    endColumn: 21,
+                                    endLine: 1,
                                     message: "'foo' is not defined.",
                                     nodeType: "Identifier",
                                     ruleId: "no-undef",

tests/lib/linter.js

@@ -1105,7 +1105,7 @@ describe("Linter", () => {
             assert.strictEqual(messages[0].endColumn, 2);
         });
 
-        it("should not have 'endLine' and 'endColumn' when 'loc' property doe not have 'end' property.", () => {
+        it("should not have 'endLine' and 'endColumn' when 'loc' property does not have 'end' property.", () => {
             linter.on("Program", node => {
                 linter.report(
                     "test-rule",
@@ -1122,6 +1122,13 @@ describe("Linter", () => {
             assert.strictEqual(messages[0].endColumn, void 0);
         });
 
+        it("should infer an 'endLine' and 'endColumn' property when using the object-based context.report API", () => {
+            const messages = linter.verify("foo", { rules: { "no-undef": "error" } });
+
+            assert.strictEqual(messages.length, 1);
+            assert.strictEqual(messages[0].endLine, 1);
+            assert.strictEqual(messages[0].endColumn, 4);
+        });
     });
 
     describe("when evaluating code", () => {