internal/lsp/cache: allow fixing multiple syntax errors
Allow fixSrc to run multiple times on a file. For example:
func main() {
var s S
if s.<>
}
type S struct {
i int
}
To properly complete at <>, we need to perform two fixes. We must
insert a phantom underscore after "s." to deal with the dangling
selector, and then we must insert phantom "{}" for the "if" statement
to deal with the incomplete "if" block. Previously we stopped at one
fix, but now we allow for up to 10. I added a limit because I am
afraid there are cases where we could get stuck trying to apply the
same fix again and again.
Also, drive-by set isIncomplete=true when we return early in
lsp/completion.go. I have found my editor incorrectly caches this zero
result in certain cases because it thinks results are "complete".
Fixes golang/go#43471.
Change-Id: Idd34cc164d579fa12a27920dc3afb372799abf26
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289271
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/gopls/internal/regtest/completion/completion_test.go b/gopls/internal/regtest/completion/completion_test.go
index 9afa6ce..6e89c4f 100644
--- a/gopls/internal/regtest/completion/completion_test.go
+++ b/gopls/internal/regtest/completion/completion_test.go
@@ -342,5 +342,35 @@
t.Errorf("expected import of mod.com/mainmod in %q", content)
}
})
+}
+// Test that we can doctor the source code enough so the file is
+// parseable and completion works as expected.
+func TestSourceFixup(t *testing.T) {
+ const files = `
+-- go.mod --
+module mod.com
+
+go 1.12
+-- foo.go --
+package foo
+
+func _() {
+ var s S
+ if s.
+}
+
+type S struct {
+ i int
+}
+`
+
+ Run(t, files, func(t *testing.T, env *Env) {
+ env.OpenFile("foo.go")
+ completions := env.Completion("foo.go", env.RegexpSearch("foo.go", `if s\.()`))
+ diff := compareCompletionResults([]string{"i"}, completions.Items)
+ if diff != "" {
+ t.Fatal(diff)
+ }
+ })
}
diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go
index 0a2c371..edbd519 100644
--- a/internal/lsp/cache/parse.go
+++ b/internal/lsp/cache/parse.go
@@ -17,6 +17,8 @@
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/lsp/debug/tag"
+ "golang.org/x/tools/internal/lsp/diff"
+ "golang.org/x/tools/internal/lsp/diff/myers"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/memoize"
@@ -281,9 +283,26 @@
// Fix any badly parsed parts of the AST.
fixed = fixAST(ctx, file, tok, buf)
- // Fix certain syntax errors that render the file unparseable.
- newSrc := fixSrc(file, tok, buf)
- if newSrc != nil {
+ for i := 0; i < 10; i++ {
+ // Fix certain syntax errors that render the file unparseable.
+ newSrc := fixSrc(file, tok, buf)
+ if newSrc == nil {
+ break
+ }
+
+ // If we thought there was something to fix 10 times in a row,
+ // it is likely we got stuck in a loop somehow. Log out a diff
+ // of the last changes we made to aid in debugging.
+ if i == 9 {
+ edits, err := myers.ComputeEdits(fh.URI(), string(buf), string(newSrc))
+ if err != nil {
+ event.Error(ctx, "error generating fixSrc diff", err, tag.File.Of(tok.Name()))
+ } else {
+ unified := diff.ToUnified("before", "after", string(buf), edits)
+ event.Log(ctx, fmt.Sprintf("fixSrc loop - last diff:\n%v", unified), tag.File.Of(tok.Name()))
+ }
+ }
+
newFile, _ := parser.ParseFile(fset, fh.URI().Filename(), newSrc, parserMode)
if newFile != nil {
// Maintain the original parseError so we don't try formatting the doctored file.
@@ -293,12 +312,14 @@
fixed = fixAST(ctx, file, tok, buf)
}
+
}
if mode == source.ParseExported {
trimAST(file)
}
}
+
// A missing file is always an error; use the parse error or make one up if there isn't one.
if file == nil {
if parseError == nil {
diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go
index 9b55f2f..3762b7a 100644
--- a/internal/lsp/completion.go
+++ b/internal/lsp/completion.go
@@ -37,7 +37,8 @@
}
if candidates == nil {
return &protocol.CompletionList{
- Items: []protocol.CompletionItem{},
+ IsIncomplete: true,
+ Items: []protocol.CompletionItem{},
}, nil
}
// We might need to adjust the position to account for the prefix.
