internal/lsp/cache: fix incorrect detection of useless code
Our logic to detect whether suggested fixes delete unnecessary code did
not take into account suggested fixes that have a command.
Fixes golang/go#48938
Change-Id: I70d2bebbf7059236525117cc1f454ef726162a96
Reviewed-on: https://go-review.googlesource.com/c/tools/+/355769
Trust: Robert Findley <rfindley@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
diff --git a/gopls/internal/regtest/diagnostics/undeclared_test.go b/gopls/internal/regtest/diagnostics/undeclared_test.go
new file mode 100644
index 0000000..79f7d42
--- /dev/null
+++ b/gopls/internal/regtest/diagnostics/undeclared_test.go
@@ -0,0 +1,67 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package diagnostics
+
+import (
+ "testing"
+
+ "golang.org/x/tools/internal/lsp/protocol"
+ . "golang.org/x/tools/internal/lsp/regtest"
+)
+
+func TestUndeclaredDiagnostics(t *testing.T) {
+ src := `
+-- go.mod --
+module mod.com
+
+go 1.12
+-- a/a.go --
+package a
+
+func _() int {
+ return x
+}
+-- b/b.go --
+package b
+
+func _() int {
+ var y int
+ y = y
+ return y
+}
+`
+ Run(t, src, func(t *testing.T, env *Env) {
+ isUnnecessary := func(diag protocol.Diagnostic) bool {
+ for _, tag := range diag.Tags {
+ if tag == protocol.Unnecessary {
+ return true
+ }
+ }
+ return false
+ }
+
+ // 'x' is undeclared, but still necessary.
+ env.OpenFile("a/a.go")
+ env.Await(env.DiagnosticAtRegexp("a/a.go", "x"))
+ diags := env.DiagnosticsFor("a/a.go")
+ if got := len(diags.Diagnostics); got != 1 {
+ t.Errorf("len(Diagnostics) = %d, want 1", got)
+ }
+ if diag := diags.Diagnostics[0]; isUnnecessary(diag) {
+ t.Errorf("%v tagged unnecessary, want necessary", diag)
+ }
+
+ // 'y = y' is pointless, and should be detected as unnecessary.
+ env.OpenFile("b/b.go")
+ env.Await(env.DiagnosticAtRegexp("b/b.go", "y = y"))
+ diags = env.DiagnosticsFor("b/b.go")
+ if got := len(diags.Diagnostics); got != 1 {
+ t.Errorf("len(Diagnostics) = %d, want 1", got)
+ }
+ if diag := diags.Diagnostics[0]; !isUnnecessary(diag) {
+ t.Errorf("%v tagged necessary, want unnecessary", diag)
+ }
+ })
+}
diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go
index 3f58d67..6790938 100644
--- a/internal/lsp/cache/errors.go
+++ b/internal/lsp/cache/errors.go
@@ -233,6 +233,9 @@
// onlyDeletions returns true if all of the suggested fixes are deletions.
func onlyDeletions(fixes []source.SuggestedFix) bool {
for _, fix := range fixes {
+ if fix.Command != nil {
+ return false
+ }
for _, edits := range fix.Edits {
for _, edit := range edits {
if edit.NewText != "" {
