Skip to content

Commit b7c3414

Browse files
committed
[gopls-release-branch.0.5] internal/lsp: fix handling of //-style comments in CRLF files
As part of the earlier changes, I didn't realize that multiple //-style comments would be grouped as one *ast.Comment, even though they are multiple comment tokens. Handle the possibility of multiple consecutive comment tokens. Fixes golang/go#42923 Change-Id: I6bc6cbdfb28a8e60c699288528566e406f27514c Reviewed-on: https://go-review.googlesource.com/c/tools/+/275012 Trust: Rebecca Stambler <[email protected]> Run-TryBot: Rebecca Stambler <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Robert Findley <[email protected]> (cherry picked from commit 73cf035) Reviewed-on: https://go-review.googlesource.com/c/tools/+/275437
1 parent 2527e7e commit b7c3414

File tree

2 files changed

+56
-42
lines changed

2 files changed

+56
-42
lines changed

gopls/internal/regtest/formatting_test.go

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -162,31 +162,26 @@ func TestFormattingOnSave(t *testing.T) {
162162
})
163163
}
164164

165-
// Reproduce golang/go#41057.
166-
func TestCRLF(t *testing.T) {
167-
runner.Run(t, "-- main.go --", func(t *testing.T, env *Env) {
168-
want := `package main
165+
// Tests various possibilities for comments in files with CRLF line endings.
166+
// Import organization in these files has historically been a source of bugs.
167+
func TestCRLFLineEndings(t *testing.T) {
168+
for _, tt := range []struct {
169+
issue, want string
170+
}{
171+
{
172+
issue: "41057",
173+
want: `package main
169174
170175
/*
171176
Hi description
172177
*/
173178
func Hi() {
174179
}
175-
`
176-
crlf := strings.ReplaceAll(want, "\n", "\r\n")
177-
env.CreateBuffer("main.go", crlf)
178-
env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 1))
179-
env.SaveBuffer("main.go")
180-
got := env.Editor.BufferText("main.go")
181-
if want != got {
182-
t.Errorf("unexpected content after save:\n%s", tests.Diff(want, got))
183-
}
184-
})
185-
}
186-
187-
func TestCRLF_42646(t *testing.T) {
188-
runner.Run(t, "-- main.go --", func(t *testing.T, env *Env) {
189-
want := `package main
180+
`,
181+
},
182+
{
183+
issue: "42646",
184+
want: `package main
190185
191186
import (
192187
"fmt"
@@ -211,15 +206,32 @@ func main() {
211206
const server_port = 8080
212207
fmt.Printf("port: %d\n", server_port)
213208
}
214-
`
215-
crlf := strings.ReplaceAll(want, "\n", "\r\n")
216-
env.CreateBuffer("main.go", crlf)
217-
env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 1))
218-
env.OrganizeImports("main.go")
219-
got := env.Editor.BufferText("main.go")
220-
got = strings.ReplaceAll(got, "\r\n", "\n") // convert everything to LF for simplicity
221-
if want != got {
222-
t.Errorf("unexpected content after save:\n%s", tests.Diff(want, got))
223-
}
224-
})
209+
`,
210+
},
211+
{
212+
issue: "42923",
213+
want: `package main
214+
215+
// Line 1.
216+
// aa
217+
type Tree struct {
218+
arr []string
219+
}
220+
`,
221+
},
222+
} {
223+
t.Run(tt.issue, func(t *testing.T) {
224+
run(t, "-- main.go --", func(t *testing.T, env *Env) {
225+
crlf := strings.ReplaceAll(tt.want, "\n", "\r\n")
226+
env.CreateBuffer("main.go", crlf)
227+
env.Await(CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 1))
228+
env.OrganizeImports("main.go")
229+
got := env.Editor.BufferText("main.go")
230+
got = strings.ReplaceAll(got, "\r\n", "\n") // convert everything to LF for simplicity
231+
if tt.want != got {
232+
t.Errorf("unexpected content after save:\n%s", tests.Diff(tt.want, got))
233+
}
234+
})
235+
})
236+
}
225237
}

internal/lsp/source/format.go

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -227,21 +227,23 @@ func importPrefix(src []byte) string {
227227
pkgEnd := f.Name.End()
228228
importEnd = maybeAdjustToLineEnd(pkgEnd, false)
229229
}
230-
for _, c := range f.Comments {
231-
if end := tok.Offset(c.End()); end > importEnd {
232-
startLine := tok.Position(c.Pos()).Line
233-
endLine := tok.Position(c.End()).Line
230+
for _, cgroup := range f.Comments {
231+
for _, c := range cgroup.List {
232+
if end := tok.Offset(c.End()); end > importEnd {
233+
startLine := tok.Position(c.Pos()).Line
234+
endLine := tok.Position(c.End()).Line
234235

235-
// Work around golang/go#41197 by checking if the comment might
236-
// contain "\r", and if so, find the actual end position of the
237-
// comment by scanning the content of the file.
238-
startOffset := tok.Offset(c.Pos())
239-
if startLine != endLine && bytes.Contains(src[startOffset:], []byte("\r")) {
240-
if commentEnd := scanForCommentEnd(tok, src[startOffset:]); commentEnd > 0 {
241-
end = startOffset + commentEnd
236+
// Work around golang/go#41197 by checking if the comment might
237+
// contain "\r", and if so, find the actual end position of the
238+
// comment by scanning the content of the file.
239+
startOffset := tok.Offset(c.Pos())
240+
if startLine != endLine && bytes.Contains(src[startOffset:], []byte("\r")) {
241+
if commentEnd := scanForCommentEnd(tok, src[startOffset:]); commentEnd > 0 {
242+
end = startOffset + commentEnd
243+
}
242244
}
245+
importEnd = maybeAdjustToLineEnd(tok.Pos(end), true)
243246
}
244-
importEnd = maybeAdjustToLineEnd(tok.Pos(end), true)
245247
}
246248
}
247249
if importEnd > len(src) {

0 commit comments

Comments
 (0)