fix: remove trailing \n from ast.Comment.Text#637
Conversation
…tibility Each ast.Comment node represents a single line of comment text and should not contain a trailing newline character. The go/printer in newer versions of gogen strictly follows the ast.Comment specification, causing invalid Go code when comments have trailing \n. Changes: - Remove `+ "\n"` from ParseComment in parser.go, skip empty lines - Add defensive strings.TrimRight in NewCommentGroupFromC - Update test expectation file Fixes #635 Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: luoliwoshang <51194195+luoliwoshang@users.noreply.github.com>
|
@xgopilot |
Completed @luoliwoshang's requestCreated issue #638 to track updating
This will prevent AI from incorrectly marking |
| goDoc.List = append(goDoc.List, | ||
| &goast.Comment{ | ||
| Slash: token.NoPos, Text: comment.Text, | ||
| Slash: token.NoPos, Text: strings.TrimRight(comment.Text, "\n"), |
There was a problem hiding this comment.
Removed the redundant strings.TrimRight from NewCommentGroupFromC in cl/internal/convert/comments.go. The trailing \n is already stripped at the source in ParseComment (_xtool/internal/parser/parser.go), so the defensive trim here was unnecessary. Pushed in commit 5f8b74d.
The trailing \n is already removed at the source in ParseComment, so the defensive TrimRight in comments.go is unnecessary. Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: luoliwoshang <51194195+luoliwoshang@users.noreply.github.com>
| if line == "" { | ||
| continue | ||
| } |
There was a problem hiding this comment.
remove reduant logic @xgopilot we current not need,and the TestParserCMode/typeof also need update expect case!
There was a problem hiding this comment.
Done. Removed the redundant if line == "" { continue } empty-line skip from ParseComment in _xtool/internal/parser/parser.go:198-200, and updated _xtool/internal/parser/testdata/typeof/expect.json to remove the trailing \n from the comment text. Pushed in commit e2db36d.
Remove the redundant `if line == "" { continue }` logic from
ParseComment that is not currently needed, and update the typeof
test expectation to remove trailing \n from ast.Comment.Text.
Generated with [codeagent](https://github.com/qbox/codeagent)
Co-authored-by: luoliwoshang <51194195+luoliwoshang@users.noreply.github.com>
2 participants
Requested by @luoliwoshang
This PR fixes the issue where
ast.Comment.Textcontains additional trailing\ncharacters, which causesgo/printerto produce invalid Go code when using newer versions of gogen.Summary
+ "\n"fromParseCommentin_xtool/internal/parser/parser.goand skip empty lines from splitstrings.TrimRightinNewCommentGroupFromCincl/internal/convert/comments.goto strip trailing\nwhen converting C AST comments to Go AST comments_xtool/internal/parser/testdata/comment/expect.jsonto reflect the fixRoot Cause
Each
ast.Commentlist item represents a single line of comment text. TheParseCommentfunction was splitting raw comment text by\nand then re-adding\nto each line. With newer versions ofgo/printer(via gogen v1.20.2), this trailing\ncauses the printer to produce invalid Go output like:Test plan
go build ./...passesgo fmt ./...produces no changesgo vet ./...passes./config ./internal/...)TestFromTestdatatests pass (requiresllcppsigfetchtool via CI)Fixes #635