go/analysis/analysistest: expand testing to handle suggested fixes

This change expands go/analysis to add the ability to verify the suggested fixes returned by an analyzer.

Change-Id: Ic38e1a24342a5c24356f8b83d196da012d8e8e01
Reviewed-on: https://go-review.googlesource.com/c/tools/+/224959
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rohan Challa <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

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

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"),

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

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.
 

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 }

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

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.
 }

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"

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

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$`
+}