internal/lsp: don't deep complete struct field names

When it is certain we are completing a struct field name, we don't
want deep completions. The only possible completions are the remaining
field names.

I also silenced the log spam in tests by disabling the go/packages
logger and the lsp logger.

Fixes golang/go#33614

Change-Id: Icec8d92112b1674fa7a6a21145ab710d054919b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190097
Reviewed-by: Rebecca Stambler <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>

internal/lsp/lsp_test.go

@@ -20,6 +20,7 @@ import (
 	"golang.org/x/tools/internal/lsp/diff"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
+	"golang.org/x/tools/internal/lsp/telemetry/log"
 	"golang.org/x/tools/internal/lsp/tests"
 	"golang.org/x/tools/internal/span"
 )
@@ -41,6 +42,8 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 	data := tests.Load(t, exporter, "testdata")
 	defer data.Exported.Cleanup()
 
+	log.AddLogger(log.NullLogger)
+
 	cache := cache.New()
 	session := cache.NewSession(ctx)
 	view := session.NewView(ctx, viewName, span.FileURI(data.Config.Dir))

internal/lsp/source/deep_completion.go

@@ -56,6 +56,12 @@ func (c *completer) deepSearch(obj types.Object) {
 		return
 	}
 
+	// If we are definitely completing a struct field name, deep completions
+	// don't make sense.
+	if c.wantStructFieldCompletions() && c.enclosingCompositeLiteral.inKey {
+		return
+	}
+
 	// Don't search into type names.
 	if isTypeName(obj) {
 		return

internal/lsp/source/source_test.go

@@ -18,6 +18,7 @@ import (
 	"golang.org/x/tools/internal/lsp/cache"
 	"golang.org/x/tools/internal/lsp/diff"
 	"golang.org/x/tools/internal/lsp/source"
+	"golang.org/x/tools/internal/lsp/telemetry/log"
 	"golang.org/x/tools/internal/lsp/tests"
 	"golang.org/x/tools/internal/span"
 )
@@ -37,6 +38,8 @@ func testSource(t *testing.T, exporter packagestest.Exporter) {
 	data := tests.Load(t, exporter, "../testdata")
 	defer data.Exported.Cleanup()
 
+	log.AddLogger(log.NullLogger)
+
 	cache := cache.New()
 	session := cache.NewSession(ctx)
 	r := &runner{

internal/lsp/telemetry/log/log.go

@@ -90,3 +90,9 @@ func Stderr(ctx context.Context, at time.Time, tags tag.List) bool {
 	fmt.Fprintf(os.Stderr, "%v\n", ToEntry(ctx, at, tags))
 	return true
 }
+
+// NullLogger is a logger that throws away log messages and reports
+// success so that the fallback stderr logging does not happen.
+var NullLogger = func(context.Context, time.Time, tag.List) bool {
+	return true
+}

internal/lsp/testdata/deepcomplete/deep_complete.go

@@ -59,3 +59,14 @@ func _() {
 	a.deepEmbedC     //@item(deepEmbedC, "a.deepEmbedC", "deepEmbedC", "field")
 	wantsC(a)        //@complete(")", deepEmbedC, deepEmbedA, deepEmbedB)
 }
+
+func _() {
+	type nested struct {
+		a int
+		n *nested //@item(deepNestedField, "n", "*nested", "field")
+	}
+
+	nested{
+		a: 123, //@complete(" //", deepNestedField)
+	}
+}

internal/lsp/tests/tests.go

@@ -26,7 +26,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       = 144
+	ExpectedCompletionsCount       = 145
 	ExpectedCompletionSnippetCount = 15
 	ExpectedDiagnosticsCount       = 21
 	ExpectedFormatCount            = 6
@@ -195,12 +195,12 @@ func Load(t testing.TB, exporter packagestest.Exporter, dir string) *Data {
 		filename := data.Exported.File(testModule, fragment)
 		data.fragments[filename] = fragment
 	}
-	data.Exported.Config.Logf = t.Logf
+	data.Exported.Config.Logf = nil
 
 	// Merge the exported.Config with the view.Config.
 	data.Config = *data.Exported.Config
 	data.Config.Fset = token.NewFileSet()
-	data.Config.Logf = t.Logf
+	data.Config.Logf = nil
 	data.Config.Context = Context(nil)
 	data.Config.ParseFile = func(fset *token.FileSet, filename string, src []byte) (*ast.File, error) {
 		panic("ParseFile should not be called")