From 716a04c65256530b22e5e1947388abfc244d4645 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Fri, 23 Apr 2021 15:10:27 -0700 Subject: [PATCH] lsp/completion: fix postfix completions preceding assignments In cases like: foo.<> bar = 123 We weren't detecting that the selector preceding <> was a statement. The above is parsed as "foo.<>bar = 123", so it looks like <> is contained in an AssignStmt. This matters because we give different postfix completions depending on whether it is valid to replace the surrounding selector with a statement or not. Updates golang/go#45718. Change-Id: I8f74505b2c8c7060f1e94433904ff0a987d0cc57 Reviewed-on: https://go-review.googlesource.com/c/tools/+/313269 Run-TryBot: Muir Manders gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Robert Findley Trust: Rebecca Stambler --- .../lsp/source/completion/postfix_snippets.go | 17 +++++++++++++++-- internal/lsp/testdata/snippets/postfix.go | 15 +++++++++++++++ internal/lsp/testdata/summary.txt.golden | 2 +- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/internal/lsp/source/completion/postfix_snippets.go b/internal/lsp/source/completion/postfix_snippets.go index 2c3c6e9fa5..4c5cb0ef1f 100644 --- a/internal/lsp/source/completion/postfix_snippets.go +++ b/internal/lsp/source/completion/postfix_snippets.go @@ -303,11 +303,25 @@ func (c *completer) addPostfixSnippetCandidates(ctx context.Context, sel *ast.Se return } + tokFile := c.snapshot.FileSet().File(c.pos) + // Only replace sel with a statement if sel is already a statement. var stmtOK bool for i, n := range c.path { if n == sel && i < len(c.path)-1 { - _, stmtOK = c.path[i+1].(*ast.ExprStmt) + switch p := c.path[i+1].(type) { + case *ast.ExprStmt: + stmtOK = true + case *ast.AssignStmt: + // In cases like: + // + // foo.<> + // bar = 123 + // + // detect that "foo." makes up the entire statement since the + // apparent selector spans lines. + stmtOK = tokFile.Line(c.pos) < tokFile.Line(p.TokPos) + } break } } @@ -328,7 +342,6 @@ func (c *completer) addPostfixSnippetCandidates(ctx context.Context, sel *ast.Se // // and adjust afterDot so that we don't mistakenly delete the // newline thinking "bar" is part of our selector. - tokFile := c.snapshot.FileSet().File(c.pos) if startLine := tokFile.Line(sel.Pos()); startLine != tokFile.Line(afterDot) { if tokFile.Line(c.pos) != startLine { return diff --git a/internal/lsp/testdata/snippets/postfix.go b/internal/lsp/testdata/snippets/postfix.go index 29b419225f..d29694e835 100644 --- a/internal/lsp/testdata/snippets/postfix.go +++ b/internal/lsp/testdata/snippets/postfix.go @@ -25,3 +25,18 @@ func _() { var blah func() []int blah().append //@complete(" //") } + +func _() { + /* append! */ //@item(postfixAppend, "append!", "append and re-assign slice", "snippet") + /* last! */ //@item(postfixLast, "last!", "s[len(s)-1]", "snippet") + /* print! */ //@item(postfixPrint, "print!", "print to stdout", "snippet") + /* range! */ //@item(postfixRange, "range!", "range over slice", "snippet") + /* reverse! */ //@item(postfixReverse, "reverse!", "reverse slice", "snippet") + /* sort! */ //@item(postfixSort, "sort!", "sort.Slice()", "snippet") + /* var! */ //@item(postfixVar, "var!", "assign to variable", "snippet") + + var foo []int + foo. //@complete(" //", postfixAppend, postfixCopy, postfixLast, postfixPrint, postfixRange, postfixReverse, postfixSort, postfixVar) + + foo = nil +} diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index 8c4c72a4c7..6db15ec330 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -1,7 +1,7 @@ -- summary -- CallHierarchyCount = 2 CodeLensCount = 5 -CompletionsCount = 262 +CompletionsCount = 263 CompletionSnippetCount = 95 UnimportedCompletionsCount = 5 DeepCompletionsCount = 5