From 3e93b52866ede40964c3167d35a3e1af4922d9c5 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Tue, 23 Apr 2019 22:34:23 +0000 Subject: [PATCH] internal/lsp: suppress more completions in comments and literals Completion suppression in comments wasn't working for comments in switch case statements, select case statements, and decl statements. Rather than adding those to the list of leaf ast.Node types to look for, we now always check if the position is in a comment. This fix broke some completion tests that were using re"$" since "$" matches after the comment "//" characters. We now also don't complete within any literal values. Previously we only excluded string literals. Change-Id: If02f39f79fe2cd7417e39dbac2c6f84a484391ec GitHub-Last-Rev: 7ab3f526b6752a8f74413dcd268382d359e1beba GitHub-Pull-Request: golang/tools#88 Reviewed-on: https://go-review.googlesource.com/c/tools/+/173518 Reviewed-by: Rebecca Stambler Run-TryBot: Rebecca Stambler --- internal/lsp/source/completion.go | 24 +++++++---------- internal/lsp/testdata/bar/bar.go.in | 4 +-- internal/lsp/testdata/basiclit/basiclit.go | 13 +++++++++ internal/lsp/testdata/baz/baz.go.in | 6 ++--- internal/lsp/testdata/comments/comments.go | 27 +++++++++++++++++++ .../lsp/testdata/stringlit/stringlit.go.in | 5 ---- internal/lsp/tests/tests.go | 2 +- 7 files changed, 56 insertions(+), 25 deletions(-) create mode 100644 internal/lsp/testdata/basiclit/basiclit.go create mode 100644 internal/lsp/testdata/comments/comments.go delete mode 100644 internal/lsp/testdata/stringlit/stringlit.go.in diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 3f70f850..1a8fc68f 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -61,16 +61,14 @@ func Completion(ctx context.Context, f File, pos token.Pos) (items []CompletionI return nil, "", fmt.Errorf("cannot find node enclosing position") } - // Skip completion inside comment blocks or string literals. - switch lit := path[0].(type) { - case *ast.File, *ast.BlockStmt: - if inComment(pos, file.Comments) { - return items, prefix, nil - } - case *ast.BasicLit: - if lit.Kind == token.STRING { - return items, prefix, nil - } + // Skip completion inside comments. + if inComment(pos, file.Comments) { + return items, prefix, nil + } + + // Skip completion inside any kind of literal. + if _, ok := path[0].(*ast.BasicLit); ok { + return items, prefix, nil } // Save certain facts about the query position, including the expected type @@ -283,10 +281,8 @@ func lexical(path []ast.Node, pos token.Pos, pkg *types.Package, info *types.Inf // inComment checks if given token position is inside ast.Comment node. func inComment(pos token.Pos, commentGroups []*ast.CommentGroup) bool { for _, g := range commentGroups { - for _, c := range g.List { - if c.Pos() <= pos && pos <= c.End() { - return true - } + if g.Pos() <= pos && pos <= g.End() { + return true } } return false diff --git a/internal/lsp/testdata/bar/bar.go.in b/internal/lsp/testdata/bar/bar.go.in index 1677e736..54f5c64a 100644 --- a/internal/lsp/testdata/bar/bar.go.in +++ b/internal/lsp/testdata/bar/bar.go.in @@ -38,9 +38,9 @@ func _() { Value: Valen //@complete("le", Valentine) } _ = foo.StructFoo{ - Value: //@complete(re"$", Valentine, foo, Bar, helper) + Value: //@complete(" //", Valentine, foo, Bar, helper) } _ = foo.StructFoo{ Value: //@complete(" ", Valentine, foo, Bar, helper) } -} \ No newline at end of file +} diff --git a/internal/lsp/testdata/basiclit/basiclit.go b/internal/lsp/testdata/basiclit/basiclit.go new file mode 100644 index 00000000..ab895dc0 --- /dev/null +++ b/internal/lsp/testdata/basiclit/basiclit.go @@ -0,0 +1,13 @@ +package basiclit + +func _() { + var a int // something for lexical completions + + _ = "hello." //@complete(".") + + _ = 1 //@complete(" //") + + _ = 1. //@complete(".") + + _ = 'a' //@complete("' ") +} diff --git a/internal/lsp/testdata/baz/baz.go.in b/internal/lsp/testdata/baz/baz.go.in index 90d952be..534854c3 100644 --- a/internal/lsp/testdata/baz/baz.go.in +++ b/internal/lsp/testdata/baz/baz.go.in @@ -18,14 +18,14 @@ func Baz() { func _() { bob := f.StructFoo{Value: 5} - if x := bob. //@complete(re"$", Value) + if x := bob. //@complete(" //", Value) switch true == false { case true: - if x := bob. //@complete(re"$", Value) + if x := bob. //@complete(" //", Value) case false: } if x := bob.Va //@complete("a", Value) switch true == true { default: } -} \ No newline at end of file +} diff --git a/internal/lsp/testdata/comments/comments.go b/internal/lsp/testdata/comments/comments.go new file mode 100644 index 00000000..e261cfd3 --- /dev/null +++ b/internal/lsp/testdata/comments/comments.go @@ -0,0 +1,27 @@ +package comments + +var p bool + +//@complete(re"$") + +func _() { + var a int + + switch a { + case 1: + //@complete(re"$") + _ = a + } + + var b chan int + select { + case <-b: + //@complete(re"$") + _ = b + } + + var ( + //@complete(re"$") + _ = a + ) +} diff --git a/internal/lsp/testdata/stringlit/stringlit.go.in b/internal/lsp/testdata/stringlit/stringlit.go.in deleted file mode 100644 index 2558eb57..00000000 --- a/internal/lsp/testdata/stringlit/stringlit.go.in +++ /dev/null @@ -1,5 +0,0 @@ -package stringlit - -func _() { - _ := "hello." //@complete(".") -} diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index bd36b82b..046c9ee5 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -27,7 +27,7 @@ import ( // We hardcode the expected number of test cases to ensure that all tests // are being executed. If a test is added, this number must be changed. const ( - ExpectedCompletionsCount = 75 + ExpectedCompletionsCount = 82 ExpectedDiagnosticsCount = 16 ExpectedFormatCount = 4 ExpectedDefinitionsCount = 21