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 <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 3f70f85..1a8fc68 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -61,16 +61,14 @@
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 @@
// 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 1677e73..54f5c64 100644
--- a/internal/lsp/testdata/bar/bar.go.in
+++ b/internal/lsp/testdata/bar/bar.go.in
@@ -38,9 +38,9 @@
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 0000000..ab895dc
--- /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 90d952b..534854c 100644
--- a/internal/lsp/testdata/baz/baz.go.in
+++ b/internal/lsp/testdata/baz/baz.go.in
@@ -18,14 +18,14 @@
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 0000000..e261cfd
--- /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 2558eb5..0000000
--- 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 bd36b82..046c9ee 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -27,7 +27,7 @@
// 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
