Skip to content

Commit

Permalink
gopls/internal/lsp: tolerate new 'imported and not used' error message
Browse files Browse the repository at this point in the history
Tolerate the new form of the "... imported but not used" error message,
to allow landing this change in go/types.

Along the way, improve the test output when comparing diagnostics, and
formatting results.

For golang/go#54845

Change-Id: I998d539f82e0f70c85bdb4c40858be5e01dd08b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/435355
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
  • Loading branch information
findleyr authored and gopherbot committed Sep 27, 2022
1 parent eb45e98 commit 10e9d3c
Show file tree
Hide file tree
Showing 18 changed files with 113 additions and 106 deletions.
1 change: 1 addition & 0 deletions gopls/internal/lsp/analysis/undeclaredname/undeclared.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ var Analyzer = &analysis.Analyzer{
RunDespiteErrors: true,
}

// The prefix for this error message changed in Go 1.20.
var undeclaredNamePrefixes = []string{"undeclared name: ", "undefined: "}

func run(pass *analysis.Pass) (interface{}, error) {
Expand Down
34 changes: 17 additions & 17 deletions gopls/internal/lsp/analysis/unusedvariable/testdata/src/assign/a.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,55 +14,55 @@ type A struct {
}

func singleAssignment() {
v := "s" // want `v declared but not used`
v := "s" // want `v declared (and|but) not used`

s := []int{ // want `s declared but not used`
s := []int{ // want `s declared (and|but) not used`
1,
2,
}

a := func(s string) bool { // want `a declared but not used`
a := func(s string) bool { // want `a declared (and|but) not used`
return false
}

if 1 == 1 {
s := "v" // want `s declared but not used`
s := "v" // want `s declared (and|but) not used`
}

panic("I should survive")
}

func noOtherStmtsInBlock() {
v := "s" // want `v declared but not used`
v := "s" // want `v declared (and|but) not used`
}

func partOfMultiAssignment() {
f, err := os.Open("file") // want `f declared but not used`
f, err := os.Open("file") // want `f declared (and|but) not used`
panic(err)
}

func sideEffects(cBool chan bool, cInt chan int) {
b := <-c // want `b declared but not used`
s := fmt.Sprint("") // want `s declared but not used`
a := A{ // want `a declared but not used`
b := <-c // want `b declared (and|but) not used`
s := fmt.Sprint("") // want `s declared (and|but) not used`
a := A{ // want `a declared (and|but) not used`
b: func() int {
return 1
}(),
}
c := A{<-cInt} // want `c declared but not used`
d := fInt() + <-cInt // want `d declared but not used`
e := fBool() && <-cBool // want `e declared but not used`
f := map[int]int{ // want `f declared but not used`
c := A{<-cInt} // want `c declared (and|but) not used`
d := fInt() + <-cInt // want `d declared (and|but) not used`
e := fBool() && <-cBool // want `e declared (and|but) not used`
f := map[int]int{ // want `f declared (and|but) not used`
fInt(): <-cInt,
}
g := []int{<-cInt} // want `g declared but not used`
h := func(s string) {} // want `h declared but not used`
i := func(s string) {}() // want `i declared but not used`
g := []int{<-cInt} // want `g declared (and|but) not used`
h := func(s string) {} // want `h declared (and|but) not used`
i := func(s string) {}() // want `i declared (and|but) not used`
}

func commentAbove() {
// v is a variable
v := "s" // want `v declared but not used`
v := "s" // want `v declared (and|but) not used`
}

func fBool() bool {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,26 @@ func noOtherStmtsInBlock() {
}

func partOfMultiAssignment() {
_, err := os.Open("file") // want `f declared but not used`
_, err := os.Open("file") // want `f declared (and|but) not used`
panic(err)
}

func sideEffects(cBool chan bool, cInt chan int) {
<-c // want `b declared but not used`
fmt.Sprint("") // want `s declared but not used`
A{ // want `a declared but not used`
<-c // want `b declared (and|but) not used`
fmt.Sprint("") // want `s declared (and|but) not used`
A{ // want `a declared (and|but) not used`
b: func() int {
return 1
}(),
}
A{<-cInt} // want `c declared but not used`
fInt() + <-cInt // want `d declared but not used`
fBool() && <-cBool // want `e declared but not used`
map[int]int{ // want `f declared but not used`
A{<-cInt} // want `c declared (and|but) not used`
fInt() + <-cInt // want `d declared (and|but) not used`
fBool() && <-cBool // want `e declared (and|but) not used`
map[int]int{ // want `f declared (and|but) not used`
fInt(): <-cInt,
}
[]int{<-cInt} // want `g declared but not used`
func(s string) {}() // want `i declared but not used`
[]int{<-cInt} // want `g declared (and|but) not used`
func(s string) {}() // want `i declared (and|but) not used`
}

func commentAbove() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,25 @@
package decl

func a() {
var b, c bool // want `b declared but not used`
var b, c bool // want `b declared (and|but) not used`
panic(c)

if 1 == 1 {
var s string // want `s declared but not used`
var s string // want `s declared (and|but) not used`
}
}

func b() {
// b is a variable
var b bool // want `b declared but not used`
var b bool // want `b declared (and|but) not used`
}

func c() {
var (
d string

// some comment for c
c bool // want `c declared but not used`
c bool // want `c declared (and|but) not used`
)

panic(d)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
package decl

func a() {
var c bool // want `b declared but not used`
var c bool // want `b declared (and|but) not used`
panic(c)

if 1 == 1 {
Expand Down
15 changes: 9 additions & 6 deletions gopls/internal/lsp/analysis/unusedvariable/unusedvariable.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,18 @@ var Analyzer = &analysis.Analyzer{

type fixesForError map[types.Error][]analysis.SuggestedFix

const unusedVariableSuffix = " declared but not used"
// The suffix for this error message changed in Go 1.20.
var unusedVariableSuffixes = []string{" declared and not used", " declared but not used"}

func run(pass *analysis.Pass) (interface{}, error) {
for _, typeErr := range analysisinternal.GetTypeErrors(pass) {
if strings.HasSuffix(typeErr.Msg, unusedVariableSuffix) {
varName := strings.TrimSuffix(typeErr.Msg, unusedVariableSuffix)
err := runForError(pass, typeErr, varName)
if err != nil {
return nil, err
for _, suffix := range unusedVariableSuffixes {
if strings.HasSuffix(typeErr.Msg, suffix) {
varName := strings.TrimSuffix(typeErr.Msg, suffix)
err := runForError(pass, typeErr, varName)
if err != nil {
return nil, err
}
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions gopls/internal/lsp/cmd/test/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,5 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnost
diag.Severity = 0
}

if diff := tests.DiffDiagnostics(uri, want, got); diff != "" {
t.Error(diff)
}
tests.CompareDiagnostics(t, uri, want, got)
}
8 changes: 3 additions & 5 deletions gopls/internal/lsp/lsp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,7 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnost
v := r.server.session.View(r.data.Config.Dir)
r.collectDiagnostics(v)
got := append([]*source.Diagnostic(nil), r.diagnostics[uri]...) // copy
if diff := tests.DiffDiagnostics(uri, want, got); diff != "" {
t.Error(diff)
}
tests.CompareDiagnostics(t, uri, want, got)
}

func (r *runner) FoldingRanges(t *testing.T, spn span.Span) {
Expand Down Expand Up @@ -416,8 +414,8 @@ func (r *runner) Format(t *testing.T, spn span.Span) {
t.Error(err)
}
got := diff.ApplyEdits(string(m.Content), sedits)
if gofmted != got {
t.Errorf("format failed for %s, expected:\n%v\ngot:\n%v", filename, gofmted, got)
if diff := compare.Text(gofmted, got); diff != "" {
t.Errorf("format failed for %s (-want +got):\n%s", filename, diff)
}
}

Expand Down
4 changes: 1 addition & 3 deletions gopls/internal/lsp/source/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnost
if err != nil {
t.Fatal(err)
}
if diff := tests.DiffDiagnostics(fileID.URI, want, got); diff != "" {
t.Error(diff)
}
tests.CompareDiagnostics(t, fileID.URI, want, got)
}

func (r *runner) Completion(t *testing.T, src span.Span, test tests.Completion, items tests.CompletionItems) {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/testdata/bad/bad0.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ func stuff() { //@item(stuff, "stuff", "func()", "func")
x := "heeeeyyyy"
random2(x) //@diag("x", "compiler", "cannot use x \\(variable of type string\\) as int value in argument to random2", "error")
random2(1) //@complete("dom", random, random2, random3)
y := 3 //@diag("y", "compiler", "y declared but not used", "error")
y := 3 //@diag("y", "compiler", "y declared (and|but) not used", "error")
}

type bob struct { //@item(bob, "bob", "struct{...}", "struct")
Expand Down
16 changes: 8 additions & 8 deletions gopls/internal/lsp/testdata/bad/bad1.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ func random() int { //@item(random, "random", "func() int", "func")
}

func random2(y int) int { //@item(random2, "random2", "func(y int) int", "func"),item(bad_y_param, "y", "int", "var")
x := 6 //@item(x, "x", "int", "var"),diag("x", "compiler", "x declared but not used", "error")
var q blah //@item(q, "q", "blah", "var"),diag("q", "compiler", "q declared but not used", "error"),diag("blah", "compiler", "(undeclared name|undefined): blah", "error")
var t **blob //@item(t, "t", "**blob", "var"),diag("t", "compiler", "t declared but not used", "error"),diag("blob", "compiler", "(undeclared name|undefined): blob", "error")
x := 6 //@item(x, "x", "int", "var"),diag("x", "compiler", "x declared (and|but) not used", "error")
var q blah //@item(q, "q", "blah", "var"),diag("q", "compiler", "q declared (and|but) not used", "error"),diag("blah", "compiler", "(undeclared name|undefined): blah", "error")
var t **blob //@item(t, "t", "**blob", "var"),diag("t", "compiler", "t declared (and|but) not used", "error"),diag("blob", "compiler", "(undeclared name|undefined): blob", "error")
//@complete("", q, t, x, bad_y_param, global_a, bob, random, random2, random3, stateFunc, stuff)

return y
Expand All @@ -25,10 +25,10 @@ func random2(y int) int { //@item(random2, "random2", "func(y int) int", "func")
func random3(y ...int) { //@item(random3, "random3", "func(y ...int)", "func"),item(y_variadic_param, "y", "[]int", "var")
//@complete("", y_variadic_param, global_a, bob, random, random2, random3, stateFunc, stuff)

var ch chan (favType1) //@item(ch, "ch", "chan (favType1)", "var"),diag("ch", "compiler", "ch declared but not used", "error"),diag("favType1", "compiler", "(undeclared name|undefined): favType1", "error")
var m map[keyType]int //@item(m, "m", "map[keyType]int", "var"),diag("m", "compiler", "m declared but not used", "error"),diag("keyType", "compiler", "(undeclared name|undefined): keyType", "error")
var arr []favType2 //@item(arr, "arr", "[]favType2", "var"),diag("arr", "compiler", "arr declared but not used", "error"),diag("favType2", "compiler", "(undeclared name|undefined): favType2", "error")
var fn1 func() badResult //@item(fn1, "fn1", "func() badResult", "var"),diag("fn1", "compiler", "fn1 declared but not used", "error"),diag("badResult", "compiler", "(undeclared name|undefined): badResult", "error")
var fn2 func(badParam) //@item(fn2, "fn2", "func(badParam)", "var"),diag("fn2", "compiler", "fn2 declared but not used", "error"),diag("badParam", "compiler", "(undeclared name|undefined): badParam", "error")
var ch chan (favType1) //@item(ch, "ch", "chan (favType1)", "var"),diag("ch", "compiler", "ch declared (and|but) not used", "error"),diag("favType1", "compiler", "(undeclared name|undefined): favType1", "error")
var m map[keyType]int //@item(m, "m", "map[keyType]int", "var"),diag("m", "compiler", "m declared (and|but) not used", "error"),diag("keyType", "compiler", "(undeclared name|undefined): keyType", "error")
var arr []favType2 //@item(arr, "arr", "[]favType2", "var"),diag("arr", "compiler", "arr declared (and|but) not used", "error"),diag("favType2", "compiler", "(undeclared name|undefined): favType2", "error")
var fn1 func() badResult //@item(fn1, "fn1", "func() badResult", "var"),diag("fn1", "compiler", "fn1 declared (and|but) not used", "error"),diag("badResult", "compiler", "(undeclared name|undefined): badResult", "error")
var fn2 func(badParam) //@item(fn2, "fn2", "func(badParam)", "var"),diag("fn2", "compiler", "fn2 declared (and|but) not used", "error"),diag("badParam", "compiler", "(undeclared name|undefined): badParam", "error")
//@complete("", arr, ch, fn1, fn2, m, y_variadic_param, global_a, bob, random, random2, random3, stateFunc, stuff)
}
2 changes: 1 addition & 1 deletion gopls/internal/lsp/testdata/format/bad_format.go.golden
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

func hello() {

var x int //@diag("x", "compiler", "x declared but not used", "error")
var x int //@diag("x", "compiler", "x declared (and|but) not used", "error")
}

func hi() {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/testdata/format/bad_format.go.in
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ func hello() {



var x int //@diag("x", "compiler", "x declared but not used", "error")
var x int //@diag("x", "compiler", "x declared (and|but) not used", "error")
}

func hi() {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/lsp/testdata/generated/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion gopls/internal/lsp/testdata/generated/generator.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package generated

func _() {
var x int //@diag("x", "compiler", "x declared but not used", "error")
var x int //@diag("x", "compiler", "x declared (and|but) not used", "error")
}
2 changes: 1 addition & 1 deletion gopls/internal/lsp/testdata/testy/testy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

func TestSomething(t *testing.T) { //@item(TestSomething, "TestSomething(t *testing.T)", "", "func")
var x int //@mark(testyX, "x"),diag("x", "compiler", "x declared but not used", "error"),refs("x", testyX)
var x int //@mark(testyX, "x"),diag("x", "compiler", "x declared (and|but) not used", "error"),refs("x", testyX)
a() //@mark(testyA, "a")
}

Expand Down
Loading

0 comments on commit 10e9d3c

Please sign in to comment.