From cf7880770c1e80a90b5973b87c1583154b05af5f Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Wed, 2 Sep 2020 00:43:45 -0400 Subject: [PATCH] internal/lsp: reproduce and fix golang/go#41057 Fix the comment end position for multi-line comments to account for the closing "*/". Fixes golang/go#41057 Change-Id: I5dd3886a81d274514e78f47ac2e7194fd9cceb06 Reviewed-on: https://go-review.googlesource.com/c/tools/+/252457 Run-TryBot: Rebecca Stambler Reviewed-by: Heschi Kreinick TryBot-Result: Gobot Gobot --- internal/lsp/regtest/formatting_test.go | 22 +++++++++++++++++++ internal/lsp/source/format.go | 9 +++++++- internal/lsp/source/format_test.go | 29 +++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/internal/lsp/regtest/formatting_test.go b/internal/lsp/regtest/formatting_test.go index 792797b574..f0d3afa098 100644 --- a/internal/lsp/regtest/formatting_test.go +++ b/internal/lsp/regtest/formatting_test.go @@ -1,6 +1,7 @@ package regtest import ( + "strings" "testing" "golang.org/x/tools/internal/lsp/tests" @@ -159,3 +160,24 @@ func TestFormattingOnSave(t *testing.T) { } }) } + +// Reproduce golang/go#41057. +func TestCRLF(t *testing.T) { + runner.Run(t, "-- main.go --", func(t *testing.T, env *Env) { + want := `package main + +/* +Hi description +*/ +func Hi() { +} +` + crlf := strings.ReplaceAll(want, "\n", "\r\n") + env.OpenFileWithContent("main.go", crlf) + env.SaveBuffer("main.go") + got := env.Editor.BufferText("main.go") + if want != got { + t.Errorf("unexpected content after save:\n%s", tests.Diff(want, got)) + } + }) +} diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index 1b4cb4246b..5114d50fcf 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -227,7 +227,14 @@ func importPrefix(src []byte) string { } for _, c := range f.Comments { if end := tok.Offset(c.End()); end > importEnd { - importEnd = maybeAdjustToLineEnd(c.End(), true) + // Work-around golang/go#41197: For multi-line comments add +2 to + // the offset. The end position does not account for the */ at the + // end. + endLine := tok.Position(c.End()).Line + if end+2 <= tok.Size() && tok.Position(tok.Pos(end+2)).Line == endLine { + end += 2 + } + importEnd = maybeAdjustToLineEnd(tok.Pos(end), true) } } if importEnd > len(src) { diff --git a/internal/lsp/source/format_test.go b/internal/lsp/source/format_test.go index 50308064a9..e0dfe15296 100644 --- a/internal/lsp/source/format_test.go +++ b/internal/lsp/source/format_test.go @@ -2,6 +2,7 @@ package source import ( "fmt" + "strings" "testing" "golang.org/x/tools/internal/lsp/diff" @@ -37,6 +38,34 @@ func TestImportPrefix(t *testing.T) { } } +func TestCRLFFile(t *testing.T) { + for i, tt := range []struct { + input, want string + }{ + { + input: `package main + +/* +Hi description +*/ +func Hi() { +} +`, + want: `package main + +/* +Hi description +*/`, + }, + } { + got := importPrefix([]byte(strings.ReplaceAll(tt.input, "\n", "\r\n"))) + want := strings.ReplaceAll(tt.want, "\n", "\r\n") + if got != want { + t.Errorf("%d: failed for %q:\n%s", i, tt.input, diffStr(want, got)) + } + } +} + func diffStr(want, got string) string { if want == got { return ""