Skip to content

Commit 1949673

Browse files
committed
passes/stdmethods: warn when an Is, As, or Unwrap has the wrong signature.
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]>
1 parent 68c6cab commit 1949673

File tree

2 files changed

+39
-0
lines changed

2 files changed

+39
-0
lines changed

go/analysis/passes/stdmethods/stdmethods.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,12 @@ var Analyzer = &analysis.Analyzer{
6161
// we let it go. But if it does have a fmt.ScanState, then the
6262
// rest has to match.
6363
var canonicalMethods = map[string]struct{ args, results []string }{
64+
"As": {[]string{"interface{}"}, []string{"bool"}}, // errors.As
6465
// "Flush": {{}, {"error"}}, // http.Flusher and jpeg.writer conflict
6566
"Format": {[]string{"=fmt.State", "rune"}, []string{}}, // fmt.Formatter
6667
"GobDecode": {[]string{"[]byte"}, []string{"error"}}, // gob.GobDecoder
6768
"GobEncode": {[]string{}, []string{"[]byte", "error"}}, // gob.GobEncoder
69+
"Is": {[]string{"error"}, []string{"bool"}}, // errors.Is
6870
"MarshalJSON": {[]string{}, []string{"[]byte", "error"}}, // json.Marshaler
6971
"MarshalXML": {[]string{"*xml.Encoder", "xml.StartElement"}, []string{"error"}}, // xml.Marshaler
7072
"ReadByte": {[]string{}, []string{"byte", "error"}}, // io.ByteReader
@@ -76,6 +78,7 @@ var canonicalMethods = map[string]struct{ args, results []string }{
7678
"UnmarshalXML": {[]string{"*xml.Decoder", "xml.StartElement"}, []string{"error"}}, // xml.Unmarshaler
7779
"UnreadByte": {[]string{}, []string{"error"}},
7880
"UnreadRune": {[]string{}, []string{"error"}},
81+
"Unwrap": {[]string{}, []string{"error"}}, // errors.Unwrap
7982
"WriteByte": {[]string{"byte"}, []string{"error"}}, // jpeg.writer (matching bufio.Writer)
8083
"WriteTo": {[]string{"=io.Writer"}, []string{"int64", "error"}}, // io.WriterTo
8184
}
@@ -123,6 +126,14 @@ func canonicalMethod(pass *analysis.Pass, id *ast.Ident) {
123126
return
124127
}
125128

129+
// Special case: Is, As and Unwrap only apply when type
130+
// implements error.
131+
if id.Name == "Is" || id.Name == "As" || id.Name == "Unwrap" {
132+
if recv := sign.Recv(); recv == nil || !implementsError(recv.Type()) {
133+
return
134+
}
135+
}
136+
126137
// Do the =s (if any) all match?
127138
if !matchParams(pass, expect.args, args, "=") || !matchParams(pass, expect.results, results, "=") {
128139
return
@@ -185,3 +196,9 @@ func matchParamType(expect string, actual types.Type) bool {
185196
// Overkill but easy.
186197
return typeString(actual) == expect
187198
}
199+
200+
var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)
201+
202+
func implementsError(actual types.Type) bool {
203+
return types.Implements(actual, errorType)
204+
}

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,25 @@ func (T) WriteTo(w io.Writer, more, args int) {} // ok - clearly not io.WriterTo
3636
type I interface {
3737
ReadByte() byte // want `should have signature ReadByte\(\) \(byte, error\)`
3838
}
39+
40+
type V int // V does not implement error.
41+
42+
func (V) As() T { return 0 } // ok - V is not an error
43+
func (V) Is() bool { return false } // ok - V is not an error
44+
func (V) Unwrap() int { return 0 } // ok - V is not an error
45+
46+
type E int
47+
48+
func (E) Error() string { return "" } // E implements error.
49+
50+
func (E) As() {} // want `method As\(\) should have signature As\(interface{}\) bool`
51+
func (E) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
52+
func (E) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`
53+
54+
type F int
55+
56+
func (F) Error() string { return "" } // Both F and *F implement error.
57+
58+
func (*F) As() {} // want `method As\(\) should have signature As\(interface{}\) bool`
59+
func (*F) Is() {} // want `method Is\(\) should have signature Is\(error\) bool`
60+
func (*F) Unwrap() {} // want `method Unwrap\(\) should have signature Unwrap\(\) error`

0 commit comments

Comments
 (0)