Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SR-9616] SwiftSyntax is not following trivia rule #438

Closed
swift-ci opened this issue Jan 8, 2019 · 4 comments · Fixed by #985
Closed

[SR-9616] SwiftSyntax is not following trivia rule #438

swift-ci opened this issue Jan 8, 2019 · 4 comments · Fixed by #985
Labels
bug Something isn't working

Comments

@swift-ci
Copy link
Contributor

swift-ci commented Jan 8, 2019

Previous ID SR-9616
Radar None
Original Reporter inamiy (JIRA User)
Type Bug

Attachment: Download

Environment
Additional Detail from JIRA
Votes 0
Component/s Compiler, SwiftSyntax
Labels Bug, Parser, Syntax
Assignee None
Priority Medium

md5: b19e5c1a45f2ad0872b99a66f9211c27

relates to:

  • SR-2373 Trailing doc comment should not be attached to the next declaration

Issue Description:

According to the libSyntax documentation:

1. A token owns all of its trailing trivia up to, but not including, the next newline character.
2. Looking backward in the text, a token owns all of the leading trivia up to and including the first contiguous sequence of newlines characters.

The following code should have leading / trailing trivias as follows:

let x = 1 // This should be "1"'s trailing trivia
/// This should be `var`'s leading trivia
var y = 2

But actually, both comments are bundled as `var`'s leading trivia.

You can test the code at https://swift-ast-explorer.kishikawakatsumi.com/ (screenshot attached).

By the way, in TypeScript, it seems to work as is:
https://github.com/basarat/typescript-book/blob/master/docs/compiler/ast-trivia.md
@rintaro
Copy link
Mannequin

rintaro mannequin commented Jan 8, 2019

I agree this is a bug.

https://github.com/apple/swift/blob/75033ed05a8f5ecf79e77b41dabd2c3863cbc9e5/lib/Parse/Lexer.cpp#L2560
I intentionally did this because, currently, we want such comments to be attached to the next token (i.e. possible start of a declaration on the next line).

https://bugs.swift.org/browse/SR-2373

Lexing trailing-comment as trailingTrivia makes parsing difficult as long as we keep current "trailing doc-comment" behavior. @nkcsgexi @akyrtzi What do you think?

@nkcsgexi
Copy link
Member

nkcsgexi commented Jan 8, 2019

This example inamiy (JIRA User) gave here is an obvious bug since the comments are in different forms ("//" and "///"), so it's reasonable to assume they're
commenting different pieces.

If they are both "///", i have no strong opinion on whether we split them as trailing and leading trivia or combine them as leading trivia, because the lexer cannot
be always right in either way.

@swift-ci
Copy link
Contributor Author

swift-ci commented Jan 9, 2019

Comment by Yasuhiro Inami (JIRA)

@nkcsgexi

If they are both "///", i have no strong opinion on whether we split them as trailing and leading trivia or combine them as leading trivia, because the lexer cannot
be always right in either way.

I think trailing and leading trivias should be separated in the above case too, and it's probably possible to distinguish them by checking if token exists in the same line or not.

If this can be done, it's exciting to see SR-2373 trailing-doc support can be implemented as well 🙂

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 9, 2022
@ahoppen
Copy link
Collaborator

ahoppen commented Jun 21, 2022

rdar://95639215

@ahoppen ahoppen removed SwiftParser Bugs in the (new) Parser written in Swift Syntax labels Aug 23, 2022
ahoppen added a commit to ahoppen/swift-syntax that referenced this issue Oct 19, 2022
@ahoppen
Change trivia attribution rule to make comments trailing trivia unles…
f479a9e 
…s separated by a newline

This changes the trivia attribution rule in the new parser to consider all trivia up to the next newline as trailing trivia. Previously, comments were always considered trailing trivia.

This was discussed last year in https://forums.swift.org/t/changing-comment-trivia-attribution-from-trailing-trivia-to-leading-trivia/50773.

My idea is that right now is probably one of the best times to change this because switching from the old to the new parser is an active source change for all clients.

Resolves rdar://68234477
Resolves rdar://95639215
Fixes apple#438
@ahoppen ahoppen mentioned this issue Oct 19, 2022
Merged
ahoppen added a commit to ahoppen/swift-syntax that referenced this issue Oct 19, 2022
@ahoppen
Change trivia attribution rule to make comments trailing trivia unles…
60c7037 
…s separated by a newline

This changes the trivia attribution rule in the new parser to consider all trivia up to the next newline as trailing trivia. Previously, comments were always considered trailing trivia.

This was discussed last year in https://forums.swift.org/t/changing-comment-trivia-attribution-from-trailing-trivia-to-leading-trivia/50773.

My idea is that right now is probably one of the best times to change this because switching from the old to the new parser is an active source change for all clients.

Resolves rdar://68234477
Resolves rdar://95639215
Fixes apple#438
adevress pushed a commit to adevress/swift-syntax that referenced this issue Jan 14, 2024
@ahoppen
Merge pull request apple#438 from ahoppen/ahoppen/install
da31c21 
Add support in build-script-helper.py to install swift-format into an open source toolchain
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
4 participants
@nkcsgexi @shahmishal @ahoppen @swift-ci