Fix: clarify AST and don't use node.start/node.end (fixes #8956) (#…

…8984)

* Fix: don't use `node.start`/`node.end` (refs #8956)

* Docs: clarify AST (fixes #8956)

* Docs: fix list style in markdown

* Fix: make it rising errors

* Docs: add about `Literal#raw` property

* fix for review.

docs/developer-guide/working-with-plugins.md

@@ -219,7 +219,7 @@ Add these keywords into your `package.json` file to make it easy for others to f
 
 * [npm Developer Guide](https://docs.npmjs.com/misc/developers)
 
-### Working with Custom Parsers
+## Working with Custom Parsers
 
 If you want to use your own parser and provide additional capabilities for your rules, you can specify your own custom parser. If a `parseForESLint` method is exposed on the parser, this method will be used to parse the code. Otherwise, the `parse` method will be used. Both methods should take in the the source code as the first argument, and an optional configuration object as the second argument (provided as `parserOptions` in a config file). The `parse` method should simply return the AST. The `parseForESLint` method should return an object that contains the required property `ast` and an optional `services` property. `ast` should contain the AST. The `services` property can contain any parser-dependent services (such as type checkers for nodes). The value of the `services` property is available to rules as `context.parserServices`.
 
@@ -246,4 +246,39 @@ exports.parseForESLint = function(code, options) {
 
 ```
 
+### The AST specification
 
+The AST that custom parsers should create is based on [ESTree](https://github.com/estree/estree). The AST requires some additional properties about detail information of the source code.
+
+#### All nodes:
+
+All nodes must have `range` property.
+
+* `range` (`number[]`) is an array of two numbers. Both numbers are a 0-based index which is the position in the array of source code characters. The first is the start position of the node, the second is the end position of the node. `code.slice(node.range[0], node.range[1])` must be the text of the node. This range does not include spaces/parentheses which are around the node.
+* `loc` (`SourceLocation`) must not be `null`. [The `loc` property is defined as nullable by ESTree](https://github.com/estree/estree/blob/25834f7247d44d3156030f8e8a2d07644d771fdb/es5.md#node-objects), but ESLint requires this property. On the other hand, `SourceLocation#source` property can be `undefined`. ESLint does not use the `SourceLocation#source` property.
+
+The `parent` property of all nodes must be rewriteable. ESLint sets each node's parent properties to its parent node while traversing.
+
+#### The `Program` node:
+
+The `Program` node must have `tokens` and `comments` properties. Both properties are an array of the below Token interface.
+
+```ts
+interface Token {
+    type: string;
+    loc: SourceLocation;
+    range: [number, number]; // See "All nodes:" section for details of `range` property.
+    value: string;
+}
+```
+
+* `tokens` (`Token[]`) is the array of tokens which affect the behavior of programs. Arbitrary spaces can exist between tokens, so rules check the `Token#range` to detect spaces between tokens. This must be sorted by `Token#range[0]`.
+* `comments` (`Token[]`) is the array of comment tokens. This must be sorted by `Token#range[0]`.
+
+The range indexes of all tokens and comments must not overlap with the range of other tokens and comments.
+
+#### The `Literal` node:
+
+The `Literal` node must have `raw` property.
+
+* `raw` (`string`) is the source code of this literal. This is the same as `code.slice(node.range[0], node.range[1])`.

lib/rules/arrow-parens.js

@@ -71,7 +71,7 @@ module.exports = {
                 // https://github.com/eslint/eslint/issues/8834
                 const closingParenToken = sourceCode.getTokenAfter(paramToken, astUtils.isClosingParenToken);
                 const asyncToken = isAsync ? sourceCode.getTokenBefore(firstTokenOfParam) : null;
-                const shouldAddSpaceForAsync = asyncToken && (asyncToken.end === firstTokenOfParam.start);
+                const shouldAddSpaceForAsync = asyncToken && (asyncToken.range[1] === firstTokenOfParam.range[0]);
 
                 return fixer.replaceTextRange([
                     firstTokenOfParam.range[0],

lib/rules/curly.js

@@ -238,7 +238,7 @@ module.exports = {
                     // `do while` expressions sometimes need a space to be inserted after `do`.
                     // e.g. `do{foo()} while (bar)` should be corrected to `do foo() while (bar)`
                     const needsPrecedingSpace = node.type === "DoWhileStatement" &&
-                        sourceCode.getTokenBefore(bodyNode).end === bodyNode.start &&
+                        sourceCode.getTokenBefore(bodyNode).range[1] === bodyNode.range[0] &&
                         !astUtils.canTokensBeAdjacent("do", sourceCode.getFirstToken(bodyNode, { skip: 1 }));
 
                     const openingBracket = sourceCode.getFirstToken(bodyNode);

lib/rules/key-spacing.js

@@ -433,9 +433,9 @@ module.exports = {
 
                     // Remove whitespace
                     if (isKeySide) {
-                        range = [tokenBeforeColon.end, tokenBeforeColon.end + diffAbs];
+                        range = [tokenBeforeColon.range[1], tokenBeforeColon.range[1] + diffAbs];
                     } else {
-                        range = [tokenAfterColon.start - diffAbs, tokenAfterColon.start];
+                        range = [tokenAfterColon.range[0] - diffAbs, tokenAfterColon.range[0]];
                     }
                     fix = function(fixer) {
                         return fixer.removeRange(range);

lib/rules/no-else-return.js

@@ -99,7 +99,7 @@ module.exports = {
                     // https://github.com/eslint/eslint/issues/8026
                     return new FixTracker(fixer, sourceCode)
                         .retainEnclosingFunction(node)
-                        .replaceTextRange([elseToken.start, node.end], fixedSource);
+                        .replaceTextRange([elseToken.range[0], node.range[1]], fixedSource);
                 }
             });
         }

lib/rules/no-regex-spaces.js

@@ -74,7 +74,7 @@ module.exports = {
                 nodeValue = token.value;
 
             if (nodeType === "RegularExpression") {
-                checkRegex(node, nodeValue, token.start);
+                checkRegex(node, nodeValue, token.range[0]);
             }
         }
 
@@ -100,7 +100,7 @@ module.exports = {
             const shadowed = regExpVar && regExpVar.defs.length > 0;
 
             if (node.callee.type === "Identifier" && node.callee.name === "RegExp" && isString(node.arguments[0]) && !shadowed) {
-                checkRegex(node, node.arguments[0].value, node.arguments[0].start + 1);
+                checkRegex(node, node.arguments[0].value, node.arguments[0].range[0] + 1);
             }
         }
 

lib/rules/object-curly-spacing.js

@@ -174,7 +174,7 @@ module.exports = {
                     options.arraysInObjectsException && astUtils.isClosingBracketToken(penultimate) ||
                     options.objectsInObjectsException && astUtils.isClosingBraceToken(penultimate)
                 );
-                const penultimateType = shouldCheckPenultimate && sourceCode.getNodeByRangeIndex(penultimate.start).type;
+                const penultimateType = shouldCheckPenultimate && sourceCode.getNodeByRangeIndex(penultimate.range[0]).type;
 
                 const closingCurlyBraceMustBeSpaced = (
                     options.arraysInObjectsException && penultimateType === "ArrayExpression" ||

lib/rules/padded-blocks.js

@@ -202,7 +202,7 @@ module.exports = {
                         node,
                         loc: { line: tokenBeforeFirst.loc.start.line, column: tokenBeforeFirst.loc.start.column },
                         fix(fixer) {
-                            return fixer.replaceTextRange([tokenBeforeFirst.end, firstBlockToken.start - firstBlockToken.loc.start.column], "\n");
+                            return fixer.replaceTextRange([tokenBeforeFirst.range[1], firstBlockToken.range[0] - firstBlockToken.loc.start.column], "\n");
                         },
                         message: NEVER_MESSAGE
                     });
@@ -215,7 +215,7 @@ module.exports = {
                         loc: { line: tokenAfterLast.loc.end.line, column: tokenAfterLast.loc.end.column - 1 },
                         message: NEVER_MESSAGE,
                         fix(fixer) {
-                            return fixer.replaceTextRange([lastBlockToken.end, tokenAfterLast.start - tokenAfterLast.loc.start.column], "\n");
+                            return fixer.replaceTextRange([lastBlockToken.range[1], tokenAfterLast.range[0] - tokenAfterLast.loc.start.column], "\n");
                         }
                     });
                 }

lib/rules/prefer-template.js

@@ -74,7 +74,7 @@ function startsWithTemplateCurly(node) {
         return startsWithTemplateCurly(node.left);
     }
     if (node.type === "TemplateLiteral") {
-        return node.expressions.length && node.quasis.length && node.quasis[0].start === node.quasis[0].end;
+        return node.expressions.length && node.quasis.length && node.quasis[0].range[0] === node.quasis[0].range[1];
     }
     return node.type !== "Literal" || typeof node.value !== "string";
 }
@@ -89,7 +89,7 @@ function endsWithTemplateCurly(node) {
         return startsWithTemplateCurly(node.right);
     }
     if (node.type === "TemplateLiteral") {
-        return node.expressions.length && node.quasis.length && node.quasis[node.quasis.length - 1].start === node.quasis[node.quasis.length - 1].end;
+        return node.expressions.length && node.quasis.length && node.quasis[node.quasis.length - 1].range[0] === node.quasis[node.quasis.length - 1].range[1];
     }
     return node.type !== "Literal" || typeof node.value !== "string";
 }

lib/rules/spaced-comment.js

@@ -303,9 +303,9 @@ module.exports = {
                 node,
                 fix(fixer) {
                     if (requireSpace) {
-                        return fixer.insertTextAfterRange([node.start, node.end - 2], " ");
+                        return fixer.insertTextAfterRange([node.range[0], node.range[1] - 2], " ");
                     }
-                    const end = node.end - 2,
+                    const end = node.range[1] - 2,
                         start = end - match[0].length;
 
                     return fixer.replaceTextRange([start, end], "");

lib/testers/test-parser.js

@@ -0,0 +1,48 @@
+/**
+ * @author Toru Nagashima <https://github.com/mysticatea>
+ */
+"use strict";
+
+const espree = require("espree");
+const Traverser = require("../util/traverser");
+
+/**
+ * Define `start`/`end` properties as throwing error.
+ * @param {ASTNode} node The node to define.
+ * @returns {void}
+ */
+function defineStartEndAsError(node) {
+    Object.defineProperty(node, "start", {
+        get() {
+            throw new Error("Use node.range[0] instead of node.start");
+        },
+        configurable: true,
+        enumerable: false
+    });
+    Object.defineProperty(node, "end", {
+        get() {
+            throw new Error("Use node.range[1] instead of node.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.
+ * @returns {void}
+ */
+function defineStartEndAsErrorInTree(ast) {
+    new Traverser().traverse(ast, { enter: defineStartEndAsError });
+    ast.tokens.forEach(defineStartEndAsError);
+    ast.comments.forEach(defineStartEndAsError);
+}
+
+module.exports.parse = (code, options) => {
+    const ret = espree.parse(code, options);
+
+    defineStartEndAsErrorInTree(ret.ast || ret);
+
+    return ret;
+};

tests/lib/rules/_set-default-parser.js

@@ -0,0 +1,20 @@
+/**
+ * @author Toru Nagashima <https://github.com/mysticatea>
+ *
+ * This file must be loaded before rule files.
+ *
+ * This file configures the default config of RuleTester to use TestParser
+ * instead of espree. The TestParser parses the given source code by espree,
+ * then remove the `start` and `end` properties of nodes, tokens, and comments.
+ *
+ * We have not endorsed that the properties exist on the AST of custom parsers,
+ * so we should check that core rules don't use the properties.
+ */
+"use strict";
+
+const path = require("path");
+const RuleTester = require("../../../lib/testers/rule-tester");
+
+RuleTester.setDefaultConfig({
+    parser: path.resolve(__dirname, "../../../lib/testers/test-parser")
+});