Clarify non-standard properties of AST that custom parsers should create

[mysticatea]

I think we should clarify our position about non-standard properties of AST. Especially:

• node.range
• node.start
• node.end
• Properties of tokens and comments.

ESTree does not have those properties. However, we have been using node.range properties to write core rules since v0.x.y.

After ESLint v2.0.0, AST nodes have node.start and node.end properties as well for some reason. However, as far as I know, we have not been using those two properties in core rules, because the use of those two properties is a breaking change for custom parsers and we have not announced the change.

I thought so, but in fact, we have seemed to be using node.start and node.end in core rules, e.g. padded-blocks (1c123e2).

If we clarify about non-standard properties of AST, it will become an important resource for the developer of custom parsers.

My position is:

• AST nodes require node.range property.
• AST nodes don't require node.start and node.end properties (so we should not use node.start and node.end in core rules) at this time. We can add node.start and node.end in the next major release.
• Tokens and comments are should be the following shape:

interface Token {
    type: string
    range: [number, number]
    loc: SourceLocation    // SourceLocation is defined in ESTree.
    value: string
}

---

Tell us about your environment

• ESLint Version: v4.1.1
• Node Version: v8.1.4
• npm Version: v5.1.0

What parser (default, Babel-ESLint, etc.) are you using?

• typescript-eslint-parser

What did you do? Please include the actual source code causing the issue.

/*eslint padded-blocks: [error, never] */
do {
    foo()
    // comment

} while (a)

$ eslint test.js --no-eslintrc --no-ignore --fix --parser typescript-eslint-parser

What did you expect to happen?

/*eslint padded-blocks: [error, never] */
do {
    foo()
    // comment
} while (a)

What actually happened? Please include the actual, raw output from ESLint.

The most code was lost.

} while (a)

The root cause is that the comment tokens of typescript-eslint-parser don't have start and end properties but padded-blocks rule is using those.

I'm not sure which of padded-blocks and typescript-eslint-parser I should fix because the specification of AST is unclear.

[platinumazure]

Can we assume that node.range[0] === node.start and node.range[1] === node.end (when all of those properties are present and valued correctly)?

[not-an-aardvark]

@platinumazure Yes, I think so.

We could add an internal rule to prohibit node.start and node.end, but we would have to make sure it still allows node.loc.start and node.loc.end.

I don't think we need to officially add support for node.start and node.end -- if we consider it to be an implementation detail and always rely on node.range, then it will make it easier for parsers since they won't have to support both values.

[mysticatea]

I agree that it's good if we can detect the use of Node#start, but I guess difficult that static analysis (without type system, without other files) distinguishes Node#start and SourceLocation#start. Instead, I think we could add delete node.start into the preprocess of RuleTester. But it's a breaking change for plugins... :(

[not-an-aardvark]

We could use a custom parser when running tests on the ESLint codebase. The parser would parse the source text with espree, then traverse the AST and delete all the start and end properties. That way we could disallow start and end within the ESLint codebase without modifying RuleTester for everyone else.

[platinumazure]

Can we go back to ESTree and suggest that any of the properties be added (even if marked as optional) to lend some normative credibility?

[not-an-aardvark]

I think ESTree tries to avoid having properties that provide redundant info, so I have my doubts about whether it would be accepted.

[platinumazure]

@not-an-aardvark Well, I can certainly understand that position, but I also don't think range information is redundant. The location information tells you line and column information but wouldn't be much help with regard to things like extra whitespace; personally, I think range + line/column can give a better picture.

[not-an-aardvark]

I meant that node.start and node.end are redundant if you have node.range.

[platinumazure]

@not-an-aardvark Yes, agreed. ESTree currently does not specify either node.range or node.start/node.end as even an optional property. I believe it would help us to go back to ESTree and let them choose one (probably node.range?) and list it as an optional property. That way we can just support one of those properties.

[mysticatea]

@platinumazure After I searched, there seemed to be a large discussion about CST. There are the issues which mention about range property: estree/estree#41, estree/estree#53, and estree/estree#80.
I need more time to read the discussion since my reading speed is so slow. It might be the responsibility CST repo.

We look to have consensus that we use range, so I will fix eight core rules which are using node.start/node.end.