go/analysis/passes/stdmethods: split check out of vet

Also, rename to stdmethods and add more tests.

Change-Id: I09b65899dc02a8062f3ec1d909c2eae45472e236
Reviewed-on: https://go-review.googlesource.com/c/140761
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>

go/analysis/passes/stdmethods/stdmethods.go

@@ -1,30 +1,47 @@
-// +build ignore
-
 // Copyright 2010 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 the code to check canonical methods.
+package stdmethods
-
-package main
 
 import (
+	"bytes"
 	"fmt"
 	"go/ast"
 	"go/printer"
+	"go/token"
+	"go/types"
 	"strings"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
 )
 
-func init() {
+var Analyzer = &analysis.Analyzer{
-	register("methods",
+	Name: "stdmethods",
-		"check that canonically named methods are canonically defined",
+	Doc: `check signature of methods of well-known interfaces
-		checkCanonicalMethod,
-		funcDecl, interfaceType)
-}
 
-type MethodSig struct {
+Sometimes a type may be intended to satisfy an interface but may fail to
-	args    []string
+do so because of a mistake in its method signature.
-	results []string
+For example, the result of this WriteTo method should be (int64, error),
+not error, to satisfy io.WriterTo:
+
+	type myWriterTo struct{...}
+        func (myWriterTo) WriteTo(w io.Writer) error { ... }
+
+This check ensures that each method whose name matches one of several
+well-known interface methods from the standard library has the correct
+signature for that interface.
+
+Checked method names include:
+	Format GobEncode GobDecode MarshalJSON MarshalXML
+	Peek ReadByte ReadFrom ReadRune Scan Seek
+	UnmarshalJSON UnreadByte UnreadRune WriteByte
+	WriteTo
+`,
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
 }
 
 // canonicalMethods lists the input and output types for Go methods
@@ -43,7 +60,7 @@ type MethodSig struct {
 // method doesn't have a fmt.ScanState as its first argument,
 // we let it go. But if it does have a fmt.ScanState, then the
 // rest has to match.
-var canonicalMethods = map[string]MethodSig{
+var canonicalMethods = map[string]struct{ args, results []string }{
 	// "Flush": {{}, {"error"}}, // http.Flusher and jpeg.writer conflict
 	"Format":        {[]string{"=fmt.State", "rune"}, []string{}},                      // fmt.Formatter
 	"GobDecode":     {[]string{"[]byte"}, []string{"error"}},                           // gob.GobDecoder
@@ -63,22 +80,31 @@ var canonicalMethods = map[string]MethodSig{
 	"WriteTo":       {[]string{"=io.Writer"}, []string{"int64", "error"}}, // io.WriterTo
 }
 
-func checkCanonicalMethod(f *File, node ast.Node) {
+func run(pass *analysis.Pass) (interface{}, error) {
-	switch n := node.(type) {
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-	case *ast.FuncDecl:
+
-		if n.Recv != nil {
+	nodeFilter := []ast.Node{
-			canonicalMethod(f, n.Name, n.Type)
+		(*ast.FuncDecl)(nil),
-		}
+		(*ast.InterfaceType)(nil),
-	case *ast.InterfaceType:
+	}
-		for _, field := range n.Methods.List {
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
-			for _, id := range field.Names {
+		switch n := n.(type) {
-				canonicalMethod(f, id, field.Type.(*ast.FuncType))
+		case *ast.FuncDecl:
+			if n.Recv != nil {
+				canonicalMethod(pass, n.Name, n.Type)
+			}
+		case *ast.InterfaceType:
+			for _, field := range n.Methods.List {
+				for _, id := range field.Names {
+					canonicalMethod(pass, id, field.Type.(*ast.FuncType))
+				}
 			}
 		}
-	}
+	})
+	return nil, nil
 }
 
-func canonicalMethod(f *File, id *ast.Ident, t *ast.FuncType) {
+func canonicalMethod(pass *analysis.Pass, id *ast.Ident, t *ast.FuncType) {
 	// Expected input/output.
 	expect, ok := canonicalMethods[id.Name]
 	if !ok {
@@ -93,12 +119,12 @@ func canonicalMethod(f *File, id *ast.Ident, t *ast.FuncType) {
 	}
 
 	// Do the =s (if any) all match?
-	if !f.matchParams(expect.args, args, "=") || !f.matchParams(expect.results, results, "=") {
+	if !matchParams(pass, expect.args, args, "=") || !matchParams(pass, expect.results, results, "=") {
 		return
 	}
 
 	// Everything must match.
-	if !f.matchParams(expect.args, args, "") || !f.matchParams(expect.results, results, "") {
+	if !matchParams(pass, expect.args, args, "") || !matchParams(pass, expect.results, results, "") {
 		expectFmt := id.Name + "(" + argjoin(expect.args) + ")"
 		if len(expect.results) == 1 {
 			expectFmt += " " + argjoin(expect.results)
@@ -106,15 +132,15 @@ func canonicalMethod(f *File, id *ast.Ident, t *ast.FuncType) {
 			expectFmt += " (" + argjoin(expect.results) + ")"
 		}
 
-		f.b.Reset()
+		var buf bytes.Buffer
-		if err := printer.Fprint(&f.b, f.fset, t); err != nil {
+		if err := printer.Fprint(&buf, pass.Fset, t); err != nil {
-			fmt.Fprintf(&f.b, "<%s>", err)
+			fmt.Fprintf(&buf, "<%s>", err)
 		}
-		actual := f.b.String()
+		actual := buf.String()
 		actual = strings.TrimPrefix(actual, "func")
 		actual = id.Name + actual
 
-		f.Badf(id.Pos(), "method %s should have signature %s", actual, expectFmt)
+		pass.Reportf(id.Pos(), "method %s should have signature %s", actual, expectFmt)
 	}
 }
 
@@ -148,7 +174,7 @@ func typeFlatten(l []*ast.Field) []ast.Expr {
 }
 
 // Does each type in expect with the given prefix match the corresponding type in actual?
-func (f *File) matchParams(expect []string, actual []ast.Expr, prefix string) bool {
+func matchParams(pass *analysis.Pass, expect []string, actual []ast.Expr, prefix string) bool {
 	for i, x := range expect {
 		if !strings.HasPrefix(x, prefix) {
 			continue
@@ -156,7 +182,7 @@ func (f *File) matchParams(expect []string, actual []ast.Expr, prefix string) bo
 		if i >= len(actual) {
 			return false
 		}
-		if !f.matchParamType(x, actual[i]) {
+		if !matchParamType(pass.Fset, pass.Pkg, x, actual[i]) {
 			return false
 		}
 	}
@@ -167,15 +193,15 @@ func (f *File) matchParams(expect []string, actual []ast.Expr, prefix string) bo
 }
 
 // Does this one type match?
-func (f *File) matchParamType(expect string, actual ast.Expr) bool {
+func matchParamType(fset *token.FileSet, pkg *types.Package, expect string, actual ast.Expr) bool {
 	expect = strings.TrimPrefix(expect, "=")
 	// Strip package name if we're in that package.
-	if n := len(f.file.Name.Name); len(expect) > n && expect[:n] == f.file.Name.Name && expect[n] == '.' {
+	if n := len(pkg.Name()); len(expect) > n && expect[:n] == pkg.Name() && expect[n] == '.' {
 		expect = expect[n+1:]
 	}
 
 	// Overkill but easy.
-	f.b.Reset()
+	var buf bytes.Buffer
-	printer.Fprint(&f.b, f.fset, actual)
+	printer.Fprint(&buf, fset, actual)
-	return f.b.String() == expect
+	return buf.String() == expect
 }

go/analysis/passes/stdmethods/stdmethods_test.go

@@ -0,0 +1,13 @@
+package stdmethods_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/analysis/passes/stdmethods"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, stdmethods.Analyzer, "a")
+}

go/analysis/passes/stdmethods/testdata/src/a/a.go

@@ -0,0 +1,23 @@
+// Copyright 2010 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 a
+
+import "fmt"
+
+type T int
+
+func (T) Scan(x fmt.ScanState, c byte) {} // want "should have signature Scan"
+
+func (T) Format(fmt.State, byte) {} // want `should have signature Format\(fmt.State, rune\)`
+
+type U int
+
+func (U) Format(byte) {} // no error: first parameter must be fmt.State to trigger check
+
+func (U) GobDecode() {} // want `should have signature GobDecode\(\[\]byte\) error`
+
+type I interface {
+	ReadByte() byte // want "should have signature ReadByte"
+}

go/analysis/passes/vet/testdata/method.go

@@ -1,22 +0,0 @@
-// Copyright 2010 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 canonical method checker.
-
-// This file contains the code to check canonical methods.
-
-package testdata
-
-import (
-	"fmt"
-)
-
-type MethodTest int
-
-func (t *MethodTest) Scan(x fmt.ScanState, c byte) { // ERROR "should have signature Scan"
-}
-
-type MethodTestInterface interface {
-	ReadByte() byte // ERROR "should have signature ReadByte"
-}