Skip to content

Commit

Permalink
passes/stdmethods: warn when an Is, As, or Unwrap has the wrong signa…
Browse files Browse the repository at this point in the history
…ture.

Extend stdmethods to warn when a type implementing error has an Is, As, or Unwrap method with a different signature than the one expected by the errors package.

For golang/go#40356

Change-Id: Ia196bb83a685340d6dab216b87c8a4aa2846f785
Reviewed-on: https://go-review.googlesource.com/c/tools/+/312829
Run-TryBot: Tim King <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
Trust: Bryan C. Mills <[email protected]>
  • Loading branch information
timothy-king committed May 5, 2021
1 parent 68c6cab commit 1949673
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
17 changes: 17 additions & 0 deletions go/analysis/passes/stdmethods/stdmethods.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,12 @@ var Analyzer = &analysis.Analyzer{
// we let it go. But if it does have a fmt.ScanState, then the
// rest has to match.
var canonicalMethods = map[string]struct{ args, results []string }{
"As": {[]string{"interface{}"}, []string{"bool"}}, // errors.As
// "Flush": {{}, {"error"}}, // http.Flusher and jpeg.writer conflict
"Format": {[]string{"=fmt.State", "rune"}, []string{}}, // fmt.Formatter
"GobDecode": {[]string{"[]byte"}, []string{"error"}}, // gob.GobDecoder
"GobEncode": {[]string{}, []string{"[]byte", "error"}}, // gob.GobEncoder
"Is": {[]string{"error"}, []string{"bool"}}, // errors.Is
"MarshalJSON": {[]string{}, []string{"[]byte", "error"}}, // json.Marshaler
"MarshalXML": {[]string{"*xml.Encoder", "xml.StartElement"}, []string{"error"}}, // xml.Marshaler
"ReadByte": {[]string{}, []string{"byte", "error"}}, // io.ByteReader
Expand All @@ -76,6 +78,7 @@ var canonicalMethods = map[string]struct{ args, results []string }{
"UnmarshalXML": {[]string{"*xml.Decoder", "xml.StartElement"}, []string{"error"}}, // xml.Unmarshaler
"UnreadByte": {[]string{}, []string{"error"}},
"UnreadRune": {[]string{}, []string{"error"}},
"Unwrap": {[]string{}, []string{"error"}}, // errors.Unwrap
"WriteByte": {[]string{"byte"}, []string{"error"}}, // jpeg.writer (matching bufio.Writer)
"WriteTo": {[]string{"=io.Writer"}, []string{"int64", "error"}}, // io.WriterTo
}
Expand Down Expand Up @@ -123,6 +126,14 @@ func canonicalMethod(pass *analysis.Pass, id *ast.Ident) {
return
}

// Special case: Is, As and Unwrap only apply when type
// implements error.
if id.Name == "Is" || id.Name == "As" || id.Name == "Unwrap" {
if recv := sign.Recv(); recv == nil || !implementsError(recv.Type()) {
return
}
}

// Do the =s (if any) all match?
if !matchParams(pass, expect.args, args, "=") || !matchParams(pass, expect.results, results, "=") {
return
Expand Down Expand Up @@ -185,3 +196,9 @@ func matchParamType(expect string, actual types.Type) bool {
// Overkill but easy.
return typeString(actual) == expect
}

var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)

func implementsError(actual types.Type) bool {
return types.Implements(actual, errorType)
}
22 changes: 22 additions & 0 deletions go/analysis/passes/stdmethods/testdata/src/a/a.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,25 @@ func (T) WriteTo(w io.Writer, more, args int) {} // ok - clearly not io.WriterTo
type I interface {
ReadByte() byte // want `should have signature ReadByte\(\) \(byte, error\)`
}

type V int // V does not implement error.

func (V) As() T { return 0 } // ok - V is not an error
func (V) Is() bool { return false } // ok - V is not an error
func (V) Unwrap() int { return 0 } // ok - V is not an error

type E int

func (E) Error() string { return "" } // E implements error.

func (E) As() {} // want `method As\(\) should have signature As\(interface{}\) bool`
func (E) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
func (E) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`

type F int

func (F) Error() string { return "" } // Both F and *F implement error.

func (*F) As() {} // want `method As\(\) should have signature As\(interface{}\) bool`
func (*F) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
func (*F) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`

0 comments on commit 1949673

Please sign in to comment.