-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
go/printer: Comment-LineBreak-LineBreak-SelectorExpr-Comment AST modification issue #70978
Comments
CC @griesemer |
FYI, this is not a valid Go identifier: ident.Name = "123456789012345" spec:
But the issue is still present if we make it a valid identifier. |
I would say that the root cause of this issue, is that Lines 343 to 346 in 772f024
So we are not able to set the correct position here ( Lines 1157 to 1159 in 772f024
Thus we are using the "default one", from Lines 324 to 335 in 772f024
|
Workarorund for issue golang#70978, see the same issue for more details. Fixes golang#70978 Change-Id: I7934d51af0679ac6fc10128d77001b0092bd7392
Change https://go.dev/cl/638636 mentions this issue: |
Mailed a workaround: CL 638636 for this issue. |
I also do not understand why we are setting Lines 1003 to 1021 in 772f024
and then setting the Lines 1028 to 1029 in 772f024
Removing the EDIT: this is basically what was described in the directly in bug report #70978 (comment), i just had to manually come to the conclusion to understand the issue :). |
…wlines Fixes golang#70978 Change-Id: I0cc54bd11bb4170235bb5a6d5eb6d8438beada58
Change https://go.dev/cl/638637 mentions this issue: |
Go version
go version go1.23.4 windows/amd64
Output of
go env
in your module/workspace:What did you do?
While refactoring a codebase of ours programmatically to rename a poorly named package we ran into this issue. The bug is triggered if you modify the
X
component of aSelectorExpr
to be longer than where a following comment starts, and the selector expression has a comment + double newline preceding it.What did you see happen?
If there is a selector expression with a comment + double newline preceding it (the second newline is required), and a comment following it, and you modify the
selectorExpr.X
to be longer than where the following comment starts,go/printer
will intersperse the following comment in the middle of the selector expression.This occurs due to this block in
go/printer
- https://github.com/golang/go/blob/master/src/go/printer/printer.go#L1018There is a variable
p.impliedSemi
(p = printer)
to indicate that a newline implies a semi colon. In this function (print
) there is also a local variableimpliedSemi
thatp.impliedSemi
is set to at the end of the function.Before a token is printed, we backtrack and
flush
is called,flush
callsintersperseComments
to print any comments preceding that token for comments whosecommentOffset > next.Offset
and ifp.impliedSemi
isfalse
(ignoring other parts of the conditional for the purpose of this bug report).When the
IDENT
(se.X
) token is encountered we set the local variableimpliedSemi
totrue
. The function then callsflush
where we print the preceding comment. Now, the linked block will print any newlines (up to two) after that comment, before our token. I am not sure why, but this block now overridesimpliedSemi
and sets it tofalse
. The function finishes andp.impliedSemi
is set tofalse
. Now the conditions for our bug are set.Aside: I am not 100% on the intention of why this block sets
impliedSemi
tofalse
. It has printed newlines, but those newlines come before the token (IDENT
) that affectedimpliedSemi
previously, and we have not yet updatedp.impliedSemi
.So, on the next token (the
.
) we enterflush
once again, which entersintersperseComments
. Now, the comment following the selector expression meets the conditional that its start comes before the next token (.
) andp.impliedSemi
isfalse
. So we print the comment before printing the rest of the selector expression.What did you expect to see?
If there is not the very specific case of a comment + double newline preceding the selector expression, this works as intended. If there is a statement, or no newline after the comment, the selector expression does not get split up.
Depending on the intention behind setting
impliedSemi
in the linked block, some solutions may be -impliedSemi
in this blockimpliedSemi
in this block if the newlines are the result of backtracking for commentsThank you!
The text was updated successfully, but these errors were encountered: