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>

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

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)
 	}
-}
+}

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("' ")
+}

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:
 	}
-}
+}

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
+	)
+}

internal/lsp/testdata/stringlit/stringlit.go.in

@@ -1,5 +0,0 @@
-package stringlit
-
-func _() {
-	_ := "hello." //@complete(".")
-}

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