diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index 74043c59de..76e8ce599d 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -3,6 +3,7 @@ package analysistest import ( "fmt" + "go/format" "go/token" "go/types" "io/ioutil" @@ -18,6 +19,8 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/internal/checker" "golang.org/x/tools/go/packages" + "golang.org/x/tools/internal/lsp/diff" + "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/testenv" ) @@ -61,6 +64,75 @@ type Testing interface { Errorf(format string, args ...interface{}) } +func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result { + r := Run(t, dir, a, patterns...) + + fileEdits := make(map[*token.File][]diff.TextEdit) + fileContents := make(map[*token.File][]byte) + + // Validate edits, prepare the fileEdits map and read the file contents. + for _, act := range r { + for _, diag := range act.Diagnostics { + for _, sf := range diag.SuggestedFixes { + for _, edit := range sf.TextEdits { + // Validate the edit. + if edit.Pos > edit.End { + t.Errorf( + "diagnostic for analysis %v contains Suggested Fix with malformed edit: pos (%v) > end (%v)", + act.Pass.Analyzer.Name, edit.Pos, edit.End) + continue + } + file, endfile := act.Pass.Fset.File(edit.Pos), act.Pass.Fset.File(edit.End) + if file == nil || endfile == nil || file != endfile { + t.Errorf( + "diagnostic for analysis %v contains Suggested Fix with malformed spanning files %v and %v", + act.Pass.Analyzer.Name, file.Name(), endfile.Name()) + continue + } + if _, ok := fileContents[file]; !ok { + contents, err := ioutil.ReadFile(file.Name()) + if err != nil { + t.Errorf("error reading %s: %v", file.Name(), err) + } + fileContents[file] = contents + } + spn, err := span.NewRange(act.Pass.Fset, edit.Pos, edit.End).Span() + if err != nil { + t.Errorf("error converting edit to span %s: %v", file.Name(), err) + } + fileEdits[file] = append(fileEdits[file], diff.TextEdit{ + Span: spn, + NewText: string(edit.NewText), + }) + } + } + } + } + + for file, edits := range fileEdits { + // Get the original file contents. + orig, ok := fileContents[file] + if !ok { + t.Errorf("could not find file contents for %s", file.Name()) + continue + } + out := diff.ApplyEdits(string(orig), edits) + // Get the golden file and read the contents. + want, err := ioutil.ReadFile(file.Name() + ".golden") + if err != nil { + t.Errorf("error reading %s.golden: %v", file.Name(), err) + } + formatted, err := format.Source([]byte(out)) + if err != nil { + continue + } + if string(want) != string(formatted) { + t.Errorf("suggested fixes failed for %s, expected:\n%#v\ngot:\n%#v", file.Name(), string(want), string(formatted)) + } + } + return r +} + // Run applies an analysis to the packages denoted by the "go list" patterns. // // It loads the packages from the specified GOPATH-style project diff --git a/go/analysis/analysistest/analysistest_test.go b/go/analysis/analysistest/analysistest_test.go index f5a003c62b..18c2f1eea9 100644 --- a/go/analysis/analysistest/analysistest_test.go +++ b/go/analysis/analysistest/analysistest_test.go @@ -70,6 +70,44 @@ func main() { // OK (facts and diagnostics on same line) func println(...interface{}) { println() } // want println:"found" "call of println(...)" +`, + "a/b.go.golden": `package main // want package:"found" + +func main() { + // The expectation is ill-formed: + print() // want: "diagnostic" + print() // want foo"fact" + print() // want foo: + print() // want "\xZZ scan error" + + // A diagnostic is reported at this line, but the expectation doesn't match: + println_TEST_("hello, world") // want "wrong expectation text" + + // An unexpected diagnostic is reported at this line: + println_TEST_() // trigger an unexpected diagnostic + + // No diagnostic is reported at this line: + print() // want "unsatisfied expectation" + + // OK + println_TEST_("hello, world") // want "call of println" + + // OK /* */-form. + println_TEST_("안녕, 세계") /* want "call of println" */ + + // OK (nested comment) + println_TEST_("Γειά σου, Κόσμε") // some comment // want "call of println" + + // OK (nested comment in /**/) + println_TEST_("你好,世界") /* some comment // want "call of println" */ + + // OK (multiple expectations on same line) + println_TEST_() + println_TEST_() // want "call of println(...)" "call of println(...)" +} + +// OK (facts and diagnostics on same line) +func println(...interface{}) { println_TEST_() } // want println:"found" "call of println(...)" `} dir, cleanup, err := analysistest.WriteFiles(filemap) if err != nil { @@ -79,7 +117,7 @@ func println(...interface{}) { println() } // want println:"found" "call of prin var got []string t2 := errorfunc(func(s string) { got = append(got, s) }) // a fake *testing.T - analysistest.Run(t2, dir, findcall.Analyzer, "a") + analysistest.RunWithSuggestedFixes(t2, dir, findcall.Analyzer, "a") want := []string{ `a/b.go:5: in 'want' comment: unexpected ":"`, @@ -91,12 +129,6 @@ func println(...interface{}) { println() } // want println:"found" "call of prin `a/b.go:11: no diagnostic was reported matching "wrong expectation text"`, `a/b.go:17: no diagnostic was reported matching "unsatisfied expectation"`, } - // Go 1.13's scanner error messages uses the word invalid where Go 1.12 used illegal. Convert them - // to keep tests compatible with both. - // TODO(matloob): Remove this once Go 1.13 is released. - for i := range got { - got[i] = strings.Replace(got[i], "illegal", "invalid", -1) - } // if !reflect.DeepEqual(got, want) { t.Errorf("got:\n%s\nwant:\n%s", strings.Join(got, "\n"), diff --git a/go/analysis/passes/assign/assign_test.go b/go/analysis/passes/assign/assign_test.go index f43d6dfab1..f793e08728 100644 --- a/go/analysis/passes/assign/assign_test.go +++ b/go/analysis/passes/assign/assign_test.go @@ -13,5 +13,5 @@ import ( func Test(t *testing.T) { testdata := analysistest.TestData() - analysistest.Run(t, testdata, assign.Analyzer, "a") + analysistest.RunWithSuggestedFixes(t, testdata, assign.Analyzer, "a") } diff --git a/go/analysis/passes/assign/testdata/src/a/a.go b/go/analysis/passes/assign/testdata/src/a/a.go index aa07ebef2b..eaec634d18 100644 --- a/go/analysis/passes/assign/testdata/src/a/a.go +++ b/go/analysis/passes/assign/testdata/src/a/a.go @@ -1,4 +1,4 @@ -// Copyright 2013 The Go Authors. All rights reserved. +// Copyright 2020 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. diff --git a/go/analysis/passes/assign/testdata/src/a/a.go.golden b/go/analysis/passes/assign/testdata/src/a/a.go.golden new file mode 100644 index 0000000000..6c91d3666c --- /dev/null +++ b/go/analysis/passes/assign/testdata/src/a/a.go.golden @@ -0,0 +1,31 @@ +// Copyright 2020 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. + +// This file contains tests for the useless-assignment checker. + +package testdata + +import "math/rand" + +type ST struct { + x int + l []int +} + +func (s *ST) SetX(x int, ch chan int) { + // Accidental self-assignment; it should be "s.x = x" + // want "self-assignment of x to x" + // Another mistake + // want "self-assignment of s.x to s.x" + + // want "self-assignment of s.l.0. to s.l.0." + + // Bail on any potential side effects to avoid false positives + s.l[num()] = s.l[num()] + rng := rand.New(rand.NewSource(0)) + s.l[rng.Intn(len(s.l))] = s.l[rng.Intn(len(s.l))] + s.l[<-ch] = s.l[<-ch] +} + +func num() int { return 2 } diff --git a/go/analysis/passes/findcall/findcall.go b/go/analysis/passes/findcall/findcall.go index 948dfe6633..27b1b8400f 100644 --- a/go/analysis/passes/findcall/findcall.go +++ b/go/analysis/passes/findcall/findcall.go @@ -11,6 +11,7 @@ package findcall import ( + "fmt" "go/ast" "go/types" @@ -48,7 +49,18 @@ func run(pass *analysis.Pass) (interface{}, error) { id = fun.Sel } if id != nil && !pass.TypesInfo.Types[id].IsType() && id.Name == name { - pass.Reportf(call.Lparen, "call of %s(...)", id.Name) + pass.Report(analysis.Diagnostic{ + Pos: call.Lparen, + Message: fmt.Sprintf("call of %s(...)", id.Name), + SuggestedFixes: []analysis.SuggestedFix{{ + Message: fmt.Sprintf("Add '_TEST_'"), + TextEdits: []analysis.TextEdit{{ + Pos: call.Lparen, + End: call.Lparen, + NewText: []byte("_TEST_"), + }}, + }}, + }) } } return true diff --git a/go/analysis/passes/findcall/findcall_test.go b/go/analysis/passes/findcall/findcall_test.go index 7710d524aa..6102b9381b 100644 --- a/go/analysis/passes/findcall/findcall_test.go +++ b/go/analysis/passes/findcall/findcall_test.go @@ -62,5 +62,5 @@ func println(s string) {} // want println:"found"`, // multiple variants of a single scenario. func TestFromFileSystem(t *testing.T) { testdata := analysistest.TestData() - analysistest.Run(t, testdata, findcall.Analyzer, "a") // loads testdata/src/a/a.go. + analysistest.RunWithSuggestedFixes(t, testdata, findcall.Analyzer, "a") // loads testdata/src/a/a.go. } diff --git a/go/analysis/passes/findcall/testdata/src/a/a.go.golden b/go/analysis/passes/findcall/testdata/src/a/a.go.golden new file mode 100644 index 0000000000..0e0a568bfc --- /dev/null +++ b/go/analysis/passes/findcall/testdata/src/a/a.go.golden @@ -0,0 +1,8 @@ +package main // want package:"found" + +func main() { + println_TEST_("hi") // want "call of println" + print("hi") // not a call of println +} + +func println(s string) {} // want println:"found" diff --git a/go/analysis/passes/stringintconv/string_test.go b/go/analysis/passes/stringintconv/string_test.go index ed06332d4e..8dc4cb9714 100644 --- a/go/analysis/passes/stringintconv/string_test.go +++ b/go/analysis/passes/stringintconv/string_test.go @@ -13,5 +13,5 @@ import ( func Test(t *testing.T) { testdata := analysistest.TestData() - analysistest.Run(t, testdata, stringintconv.Analyzer, "a") + analysistest.RunWithSuggestedFixes(t, testdata, stringintconv.Analyzer, "a") } diff --git a/go/analysis/passes/stringintconv/testdata/src/a/a.go.golden b/go/analysis/passes/stringintconv/testdata/src/a/a.go.golden new file mode 100644 index 0000000000..9538a0132f --- /dev/null +++ b/go/analysis/passes/stringintconv/testdata/src/a/a.go.golden @@ -0,0 +1,36 @@ +// Copyright 2020 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. + +// This file contains tests for the stringintconv checker. + +package a + +type A string + +type B = string + +type C int + +type D = uintptr + +func StringTest() { + var ( + i int + j rune + k byte + l C + m D + n = []int{0, 1, 2} + o struct{ x int } + ) + const p = 0 + _ = string(rune(i)) // want `^conversion from int to string yields a string of one rune$` + _ = string(j) + _ = string(k) + _ = string(rune(p)) // want `^conversion from untyped int to string yields a string of one rune$` + _ = A(rune(l)) // want `^conversion from C \(int\) to A \(string\) yields a string of one rune$` + _ = B(rune(m)) // want `^conversion from uintptr to B \(string\) yields a string of one rune$` + _ = string(rune(n[1])) // want `^conversion from int to string yields a string of one rune$` + _ = string(rune(o.x)) // want `^conversion from int to string yields a string of one rune$` +}