diff --git a/go/analysis/passes/deepequalerrors/deepequalerrors.go b/go/analysis/passes/deepequalerrors/deepequalerrors.go new file mode 100644 index 0000000000..067e91a246 --- /dev/null +++ b/go/analysis/passes/deepequalerrors/deepequalerrors.go @@ -0,0 +1,115 @@ +// Copyright 2019 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 deepequalerrors defines an Analyzer that checks for the use +// of reflect.DeepEqual with error values. +package deepequalerrors + +import ( + "go/ast" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + "golang.org/x/tools/go/types/typeutil" +) + +const Doc = `check for calls of reflect.DeepEqual on error values + +The deepequalerrors checker looks for calls of the form: + + reflect.DeepEqual(err1, err2) + +where err1 and err2 are errors. Using reflect.DeepEqual to compare +errors is discouraged.` + +var Analyzer = &analysis.Analyzer{ + Name: "deepequalerrors", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + inspect.Preorder(nodeFilter, func(n ast.Node) { + call := n.(*ast.CallExpr) + fn, ok := typeutil.Callee(pass.TypesInfo, call).(*types.Func) + if !ok { + return + } + if fn.FullName() == "reflect.DeepEqual" && hasError(pass, call.Args[0]) && hasError(pass, call.Args[1]) { + pass.Reportf(call.Pos(), "avoid using reflect.DeepEqual with errors") + } + }) + return nil, nil +} + +var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface) + +// hasError reports whether the type of e contains the type error. +// See containsError, below, for the meaning of "contains". +func hasError(pass *analysis.Pass, e ast.Expr) bool { + tv, ok := pass.TypesInfo.Types[e] + if !ok { // no type info, assume good + return false + } + return containsError(tv.Type) +} + +// Report whether any type that typ could store and that could be compared is the +// error type. This includes typ itself, as well as the types of struct field, slice +// and array elements, map keys and elements, and pointers. It does not include +// channel types (incomparable), arg and result types of a Signature (not stored), or +// methods of a named or interface type (not stored). +func containsError(typ types.Type) bool { + // Track types being processed, to avoid infinite recursion. + // Using types as keys here is OK because we are checking for the identical pointer, not + // type identity. See analysis/passes/printf/types.go. + inProgress := make(map[types.Type]bool) + + var check func(t types.Type) bool + check = func(t types.Type) bool { + if t == errorType { + return true + } + if inProgress[t] { + return false + } + inProgress[t] = true + switch t := t.(type) { + case *types.Pointer: + return check(t.Elem()) + case *types.Slice: + return check(t.Elem()) + case *types.Array: + return check(t.Elem()) + case *types.Map: + return check(t.Key()) || check(t.Elem()) + case *types.Struct: + for i := 0; i < t.NumFields(); i++ { + if check(t.Field(i).Type()) { + return true + } + } + case *types.Named: + return check(t.Underlying()) + + // We list the remaining valid type kinds for completeness. + case *types.Basic: + case *types.Chan: // channels store values, but they are not comparable + case *types.Signature: + case *types.Tuple: // tuples are only part of signatures + case *types.Interface: + } + return false + } + + return check(typ) +} diff --git a/go/analysis/passes/deepequalerrors/deepequalerrors_test.go b/go/analysis/passes/deepequalerrors/deepequalerrors_test.go new file mode 100644 index 0000000000..2d4faa3fbe --- /dev/null +++ b/go/analysis/passes/deepequalerrors/deepequalerrors_test.go @@ -0,0 +1,17 @@ +// Copyright 2019 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 deepequalerrors_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/deepequalerrors" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, deepequalerrors.Analyzer, "a") +} diff --git a/go/analysis/passes/deepequalerrors/testdata/src/a/a.go b/go/analysis/passes/deepequalerrors/testdata/src/a/a.go new file mode 100644 index 0000000000..f972c9bf0f --- /dev/null +++ b/go/analysis/passes/deepequalerrors/testdata/src/a/a.go @@ -0,0 +1,55 @@ +// Copyright 2019 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 deepequalerrors checker. + +package a + +import ( + "io" + "os" + "reflect" +) + +type myError int + +func (myError) Error() string { return "" } + +func bad() error { return nil } + +type s1 struct { + s2 *s2 + i int +} + +type myError2 error + +type s2 struct { + s1 *s1 + errs []*myError2 +} + +func hasError() { + var e error + var m myError2 + reflect.DeepEqual(bad(), e) // want `avoid using reflect.DeepEqual with errors` + reflect.DeepEqual(io.EOF, io.EOF) // want `avoid using reflect.DeepEqual with errors` + reflect.DeepEqual(e, &e) // want `avoid using reflect.DeepEqual with errors` + reflect.DeepEqual(e, m) // want `avoid using reflect.DeepEqual with errors` + reflect.DeepEqual(e, s1{}) // want `avoid using reflect.DeepEqual with errors` + reflect.DeepEqual(e, [1]error{}) // want `avoid using reflect.DeepEqual with errors` + reflect.DeepEqual(e, map[error]int{}) // want `avoid using reflect.DeepEqual with errors` + reflect.DeepEqual(e, map[int]error{}) // want `avoid using reflect.DeepEqual with errors` + // We catch the next not because *os.PathError implements error, but because it contains + // a field Err of type error. + reflect.DeepEqual(&os.PathError{}, io.EOF) // want `avoid using reflect.DeepEqual with errors` + +} + +func notHasError() { + reflect.ValueOf(4) // not reflect.DeepEqual + reflect.DeepEqual(3, 4) // not errors + reflect.DeepEqual(5, io.EOF) // only one error + reflect.DeepEqual(myError(1), io.EOF) // not types that implement error +}