Breaking: RuleTester Improvements (refs eslint/rfcs#25) (#12955)

* Breaking: RuleTester Improvements (refs eslint/rfcs#25)

* Use Object.defineProperties

* Rename comment to token in error messages

docs/developer-guide/nodejs-api.md

@@ -886,7 +886,7 @@ In addition to the properties above, invalid test cases can also have the follow
     * `suggestions` (array): An array of objects with suggestion details to check. See [Testing Suggestions](#testing-suggestions) for details
 
     If a string is provided as an error instead of an object, the string is used to assert the `message` of the error.
-* `output` (string, optional): Asserts the output that will be produced when using this rule for a single pass of autofixing (e.g. with the `--fix` command line flag). If this is `null`, asserts that none of the reported problems suggest autofixes.
+* `output` (string, required if the rule fixes code): Asserts the output that will be produced when using this rule for a single pass of autofixing (e.g. with the `--fix` command line flag). If this is `null`, asserts that none of the reported problems suggest autofixes.
 
 Any additional properties of a test case will be passed directly to the linter as config options. For example, a test case can have a `parserOptions` property to configure parser behavior:
 

lib/rule-tester/rule-tester.js

@@ -45,16 +45,20 @@ const
     path = require("path"),
     util = require("util"),
     lodash = require("lodash"),
+    Traverser = require("../../lib/shared/traverser"),
     { getRuleOptionsSchema, validate } = require("../shared/config-validator"),
     { Linter, SourceCodeFixer, interpolate } = require("../linter");
 
 const ajv = require("../shared/ajv")({ strictDefaults: true });
 
+const espreePath = require.resolve("espree");
 
 //------------------------------------------------------------------------------
 // Typedefs
 //------------------------------------------------------------------------------
 
+/** @typedef {import("../shared/types").Parser} Parser */
+
 /**
  * A test case that is expected to pass lint.
  * @typedef {Object} ValidTestCase
@@ -206,6 +210,70 @@ function sanitize(text) {
     );
 }
 
+/**
+ * Define `start`/`end` properties as throwing error.
+ * @param {string} objName Object name used for error messages.
+ * @param {ASTNode} node The node to define.
+ * @returns {void}
+ */
+function defineStartEndAsError(objName, node) {
+    Object.defineProperties(node, {
+        start: {
+            get() {
+                throw new Error(`Use ${objName}.range[0] instead of ${objName}.start`);
+            },
+            configurable: true,
+            enumerable: false
+        },
+        end: {
+            get() {
+                throw new Error(`Use ${objName}.range[1] instead of ${objName}.end`);
+            },
+            configurable: true,
+            enumerable: false
+        }
+    });
+}
+
+/**
+ * Define `start`/`end` properties of all nodes of the given AST as throwing error.
+ * @param {ASTNode} ast The root node to errorize `start`/`end` properties.
+ * @param {Object} [visitorKeys] Visitor keys to be used for traversing the given ast.
+ * @returns {void}
+ */
+function defineStartEndAsErrorInTree(ast, visitorKeys) {
+    Traverser.traverse(ast, { visitorKeys, enter: defineStartEndAsError.bind(null, "node") });
+    ast.tokens.forEach(defineStartEndAsError.bind(null, "token"));
+    ast.comments.forEach(defineStartEndAsError.bind(null, "token"));
+}
+
+/**
+ * Wraps the given parser in order to intercept and modify return values from the `parse` and `parseForESLint` methods, for test purposes.
+ * In particular, to modify ast nodes, tokens and comments to throw on access to their `start` and `end` properties.
+ * @param {Parser} parser Parser object.
+ * @returns {Parser} Wrapped parser object.
+ */
+function wrapParser(parser) {
+    if (typeof parser.parseForESLint === "function") {
+        return {
+            parseForESLint(...args) {
+                const ret = parser.parseForESLint(...args);
+
+                defineStartEndAsErrorInTree(ret.ast, ret.visitorKeys);
+                return ret;
+            }
+        };
+    }
+    return {
+        parse(...args) {
+            const ast = parser.parse(...args);
+
+            defineStartEndAsErrorInTree(ast);
+            return ast;
+        }
+    };
+}
+
 //------------------------------------------------------------------------------
 // Public Interface
 //------------------------------------------------------------------------------
@@ -450,9 +518,12 @@ class RuleTester {
 
             if (typeof config.parser === "string") {
                 assert(path.isAbsolute(config.parser), "Parsers provided as strings to RuleTester must be absolute paths");
-                linter.defineParser(config.parser, require(config.parser));
+            } else {
+                config.parser = espreePath;
             }
 
+            linter.defineParser(config.parser, wrapParser(require(config.parser)));
+
             if (schema) {
                 ajv.validateSchema(schema);
 
@@ -483,13 +554,9 @@ class RuleTester {
 
             // Verify the code.
             const messages = linter.verify(code, config, filename);
+            const fatalErrorMessage = messages.find(m => m.fatal);
 
-            // Ignore syntax errors for backward compatibility if `errors` is a number.
-            if (typeof item.errors !== "number") {
-                const errorMessage = messages.find(m => m.fatal);
-
-                assert(!errorMessage, `A fatal parsing error occurred: ${errorMessage && errorMessage.message}`);
-            }
+            assert(!fatalErrorMessage, `A fatal parsing error occurred: ${fatalErrorMessage && fatalErrorMessage.message}`);
 
             // Verify if autofix makes a syntax error or not.
             if (messages.some(m => m.fix)) {
@@ -772,6 +839,12 @@ class RuleTester {
                 } else {
                     assert.strictEqual(result.output, item.output, "Output is incorrect.");
                 }
+            } else {
+                assert.strictEqual(
+                    result.output,
+                    item.code,
+                    "The rule fixed the code. Please add 'output' property."
+                );
             }
 
             assertASTDidntChange(result.beforeAST, result.afterAST);