Feature/add comment in ast
Fixes #2241 Added support to add comments in lexer itself. Modified the tests as the root structure of the response is changed. Added some tests. Please comment if more test cases are required.
Edit: Resolved the issue by abstracting advance in parser... Will update the PR soon.
@IvanGoncharov while migrating the logic of comment skipping to the parser, I noticed that we are using this._lexer.token to get the location for many nodes... This might cause an issue when called after this._lexer.advance()/this._lexer.lookahead()(through parser) it would mean that we would need to change the logic to skip comments at all the places where this._lexer.advance()/this._lexer.lookahead() is called in parser, since next token might be a comment... (which means lots of changes, and lots of tests to satisfy coverage check)
Is it desired, or should I follow some other approach towards moving skipping functionality to parser like creating lexer.lookahead in the parser itself and renaming lexer.lookahead function to lexer._lookahed or something else?
Reference:-
I added skipping functionality to peek, expectToken, expectKeyword, expectOptionalKeyword and expectOptionalToken but
parses type with description multi-line string failed because we are using this._lexer.lookahead() and checking whether keywordToken.kind === TokenKind.NAME where keywordToken is lookahead, it fails because keywordToken is comment token...
Is it desired, or should I follow some other approach towards moving skipping functionality to parser like creating lexer.lookahead in the parser itself and renaming
lexer.lookaheadfunction tolexer._lookahedor something else?
@nveenjain If comments now included in AST as first-class nodes it means comments should be handled by the parser. So adding lookahead into parser is an appropriate solution for this problem.
But please keep comments inside the double-linked list.
P.S. Based on our Slack conversation please don't forget to investigate how comments are handled inside Babel, TS, etc. parsers. I'm especially interested in having some mechanism to allow comments to survive at least some AST modification.
Based on our Slack conversation please don't forget to investigate how comments are handled inside Babel, TS, etc. parsers
@IvanGoncharov, For acorn, comments are not parsed, i.e. skipped. For babel, comments are parsed, added to top-level as well as to internal nodes ( each node has leadingComments, and trailingComments depending on how comments are present ).
But please keep comments inside the double-linked list.
Sure will add the comments to the linked list.
Also, Do we need to add Support for leading and trailing comments too?
But if we keep nodes in linked list, there might be some edge cases:- Like for finding location, we rely on lastNode, consider the test case present in repo:-
"""
Description
"""
# Even with comments between them
type Hello {
world: String
}
Here the description string literal will has endPosition=19 according to current implementation now, but if we add comments to linked list endPosition would become 53 (since we check the last token and it will include the comment token)... What should I do in such case?
Ping @nveenjain, @IvanGoncharov Hey guys! Any update about this feature? Having the comments in the AST would be awesome to parse/dump. It seems this PR was almost finish, no?
@nveenjain @IvanGoncharov I'd also love this feature - are you open to someone continuing this work?
I'm working on some codemod tooling to handle schema deprecations in a fleet of UI apps and a lot of the apps leverage comments in their .graphql files. This would really help with helping preserve the original document structure after transforming the AST.