Remove comment attachment #736
Conversation
| "use strict"; | ||
|
|
||
| // comment fixes | ||
| module.exports = function(ast, comments, tokens) { |
There was a problem hiding this comment.
This is actually currently incorrectly named. Rather than doing comment attachment, it's actually calculating the location information for the top-level Program node. Renamed this to convertProgramNode and encapsulated all the logic of transforming the Program node to what ESLint expects in that function.
There was a problem hiding this comment.
Agreed, much less confusing :)
kaicataldo
force-pushed
the
remove-comment-attachment
branch
from
d741cf0
to
5dd961f
Compare
| delete node.innerComments; | ||
| } | ||
|
|
||
| if (node.trailingComments) { | ||
| convertComments(node.trailingComments); | ||
| delete node.trailingComments; |
There was a problem hiding this comment.
Just for my own understanding, does ESLint just not used these properties?
There was a problem hiding this comment.
That's right! We dropped the need for comment attachment to happen at the parser level in v4 (see docs here). We decided to move away from thinking about comments in the context of AST nodes (which had to be reimplemented by every parser and was buggy at best) and instead to just think of them in the context of tokens. ESLint now just takes the tokens and comments arrays provided by the parser and gives rules access to comments through the use of utility methods like sourceCode#getTokenBefore(nodeOrToken, { includeComments: true }).
There was a problem hiding this comment.
I was talking to Kai about being able to turn off even adding the comments in @babel/parser but if the only use case is for babel-eslint we probably don't need to make that option then? Unless there's a case for it being much faster (would need to test)
There was a problem hiding this comment.
I could explore that - I haven't looked at how comment attachment works in @babel/parser in a long time now, but if it's not too crazy of a thing to add I'm happy to do it! That said, I don't think it needs to be tied to this release, since it won't result in any user-facing changes.
kaicataldo
force-pushed
the
remove-comment-attachment
branch
from
5dd961f
to
5f16839
Compare
* Remove comment attachment * Simplify error messaging in tests
fixes #707
ESLint no longer uses attached comments provided by the parser. This behavior has already been removed from the default parser. I also refactored some of this code to hopefully make it clearer/remove some commented out code.