From b5c4dc140fc712404af432356335ab58376664de Mon Sep 17 00:00:00 2001 From: Dimitri Tcaciuc Date: Sun, 28 Mar 2021 14:45:12 -0700 Subject: [PATCH 1/5] Provide go/analysis analyzer instance --- README.md | 8 ++++ errcheck/analyzer.go | 74 ++++++++++++++++++++++++++++++++++ errcheck/errcheck.go | 85 +++++++++++---------------------------- errcheck/excludes.go | 79 ++++++++++++++++++++++++++++++++++++ errcheck/excludes_test.go | 39 ++++++++++++++++++ main.go | 32 +-------------- main_test.go | 34 ---------------- 7 files changed, 225 insertions(+), 126 deletions(-) create mode 100644 errcheck/analyzer.go create mode 100644 errcheck/excludes.go create mode 100644 errcheck/excludes_test.go diff --git a/README.md b/README.md index 484556b..f1de3a6 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,14 @@ takes no arguments. The `-blank` flag enables checking for assignments of errors to the blank identifier. It takes no arguments. +### go/analysis + +The package provides `Analyzer` instance that can be used with +[go/analysis](https://pkg.go.dev/golang.org/x/tools/go/analysis) API. + +Currently supported flags are `blank`, `assert`, `exclude`, and `excludeonly`. +Just as the API itself, the analyzer is exprimental and may change in the +future. ## Excluding functions diff --git a/errcheck/analyzer.go b/errcheck/analyzer.go new file mode 100644 index 0000000..f6f70d6 --- /dev/null +++ b/errcheck/analyzer.go @@ -0,0 +1,74 @@ +package errcheck + +import ( + "fmt" + "go/ast" + "go/token" + "regexp" + + "golang.org/x/tools/go/analysis" +) + +var Analyzer = &analysis.Analyzer{ + Name: "errcheck", + Doc: "check for unchecked errors", + Run: runAnalyzer, +} + +var ( + argBlank bool + argAsserts bool + argExcludeFile string + argExcludeOnly bool +) + +func init() { + Analyzer.Flags.BoolVar(&argBlank, "blank", false, "if true, check for errors assigned to blank identifier") + Analyzer.Flags.BoolVar(&argAsserts, "assert", false, "if true, check for ignored type assertion results") + Analyzer.Flags.StringVar(&argExcludeFile, "exclude", "", "Path to a file containing a list of functions to exclude from checking") + Analyzer.Flags.BoolVar(&argExcludeOnly, "excludeonly", false, "Use only excludes from exclude file") +} + +func runAnalyzer(pass *analysis.Pass) (interface{}, error) { + + exclude := map[string]bool{} + if !argExcludeOnly { + for _, name := range DefaultExcludedSymbols { + exclude[name] = true + } + } + if argExcludeFile != "" { + excludes, err := ReadExcludes(argExcludeFile) + if err != nil { + return nil, fmt.Errorf("Could not read exclude file: %v\n", err) + } + for _, name := range excludes { + exclude[name] = true + } + } + + for _, f := range pass.Files { + v := &visitor{ + typesInfo: pass.TypesInfo, + fset: pass.Fset, + blank: argBlank, + asserts: argAsserts, + exclude: exclude, + ignore: map[string]*regexp.Regexp{}, // deprecated & not used + lines: make(map[string][]string), + errors: nil, + } + + ast.Walk(v, f) + + for _, err := range v.errors { + pass.Report(analysis.Diagnostic{ + Pos: token.Pos(int(f.Pos()) + err.Pos.Offset), + Message: fmt.Sprintf("unchecked error returned by %s", err.FuncName), + }) + } + + } + + return nil, nil +} diff --git a/errcheck/errcheck.go b/errcheck/errcheck.go index 724e3e8..24aa664 100644 --- a/errcheck/errcheck.go +++ b/errcheck/errcheck.go @@ -25,45 +25,6 @@ func init() { var ( // ErrNoGoFiles is returned when CheckPackage is run on a package with no Go source files ErrNoGoFiles = errors.New("package contains no go source files") - - // DefaultExcludedSymbols is a list of symbol names that are usually excluded from checks by default. - // - // Note, that they still need to be explicitly copied to Checker.Exclusions.Symbols - DefaultExcludedSymbols = []string{ - // bytes - "(*bytes.Buffer).Write", - "(*bytes.Buffer).WriteByte", - "(*bytes.Buffer).WriteRune", - "(*bytes.Buffer).WriteString", - - // fmt - "fmt.Errorf", - "fmt.Print", - "fmt.Printf", - "fmt.Println", - "fmt.Fprint(*bytes.Buffer)", - "fmt.Fprintf(*bytes.Buffer)", - "fmt.Fprintln(*bytes.Buffer)", - "fmt.Fprint(*strings.Builder)", - "fmt.Fprintf(*strings.Builder)", - "fmt.Fprintln(*strings.Builder)", - "fmt.Fprint(os.Stderr)", - "fmt.Fprintf(os.Stderr)", - "fmt.Fprintln(os.Stderr)", - - // math/rand - "math/rand.Read", - "(*math/rand.Rand).Read", - - // strings - "(*strings.Builder).Write", - "(*strings.Builder).WriteByte", - "(*strings.Builder).WriteRune", - "(*strings.Builder).WriteString", - - // hash - "(hash.Hash).Write", - } ) // UncheckedError indicates the position of an unchecked error return. @@ -257,16 +218,17 @@ func (c *Checker) CheckPackage(pkg *packages.Package) Result { } v := &visitor{ - pkg: pkg, - ignore: ignore, - blank: !c.Exclusions.BlankAssignments, - asserts: !c.Exclusions.TypeAssertions, - lines: make(map[string][]string), - exclude: excludedSymbols, - errors: []UncheckedError{}, + typesInfo: pkg.TypesInfo, + fset: pkg.Fset, + ignore: ignore, + blank: !c.Exclusions.BlankAssignments, + asserts: !c.Exclusions.TypeAssertions, + lines: make(map[string][]string), + exclude: excludedSymbols, + errors: []UncheckedError{}, } - for _, astFile := range v.pkg.Syntax { + for _, astFile := range pkg.Syntax { if c.shouldSkipFile(astFile) { continue } @@ -277,12 +239,13 @@ func (c *Checker) CheckPackage(pkg *packages.Package) Result { // visitor implements the errcheck algorithm type visitor struct { - pkg *packages.Package - ignore map[string]*regexp.Regexp - blank bool - asserts bool - lines map[string][]string - exclude map[string]bool + typesInfo *types.Info + fset *token.FileSet + ignore map[string]*regexp.Regexp + blank bool + asserts bool + lines map[string][]string + exclude map[string]bool errors []UncheckedError } @@ -302,7 +265,7 @@ func (v *visitor) selectorAndFunc(call *ast.CallExpr) (*ast.SelectorExpr, *types return nil, nil, false } - fn, ok := v.pkg.TypesInfo.ObjectOf(sel.Sel).(*types.Func) + fn, ok := v.typesInfo.ObjectOf(sel.Sel).(*types.Func) if !ok { // Shouldn't happen, but be paranoid return nil, nil, false @@ -389,7 +352,7 @@ func (v *visitor) namesForExcludeCheck(call *ast.CallExpr) []string { // This will be missing for functions without a receiver (like fmt.Printf), // so just fall back to the the function's fullName in that case. - selection, ok := v.pkg.TypesInfo.Selections[sel] + selection, ok := v.typesInfo.Selections[sel] if !ok { return []string{name} } @@ -416,14 +379,14 @@ func (v *visitor) namesForExcludeCheck(call *ast.CallExpr) []string { func (v *visitor) argName(expr ast.Expr) string { // Special-case literal "os.Stdout" and "os.Stderr" if sel, ok := expr.(*ast.SelectorExpr); ok { - if obj := v.pkg.TypesInfo.ObjectOf(sel.Sel); obj != nil { + if obj := v.typesInfo.ObjectOf(sel.Sel); obj != nil { vr, ok := obj.(*types.Var) if ok && vr.Pkg() != nil && vr.Pkg().Name() == "os" && (vr.Name() == "Stderr" || vr.Name() == "Stdout") { return "os." + vr.Name() } } } - t := v.pkg.TypesInfo.TypeOf(expr) + t := v.typesInfo.TypeOf(expr) if t == nil { return "" } @@ -474,7 +437,7 @@ func (v *visitor) ignoreCall(call *ast.CallExpr) bool { return true } - if obj := v.pkg.TypesInfo.Uses[id]; obj != nil { + if obj := v.typesInfo.Uses[id]; obj != nil { if pkg := obj.Pkg(); pkg != nil { if re, ok := v.ignore[nonVendoredPkgPath(pkg.Path())]; ok { return re.MatchString(id.Name) @@ -500,7 +463,7 @@ func nonVendoredPkgPath(pkgPath string) string { // len(s) == number of return types of call // s[i] == true iff return type at position i from left is an error type func (v *visitor) errorsByArg(call *ast.CallExpr) []bool { - switch t := v.pkg.TypesInfo.Types[call].Type.(type) { + switch t := v.typesInfo.Types[call].Type.(type) { case *types.Named: // Single return return []bool{isErrorType(t)} @@ -542,7 +505,7 @@ func (v *visitor) callReturnsError(call *ast.CallExpr) bool { // isRecover returns true if the given CallExpr is a call to the built-in recover() function. func (v *visitor) isRecover(call *ast.CallExpr) bool { if fun, ok := call.Fun.(*ast.Ident); ok { - if _, ok := v.pkg.TypesInfo.Uses[fun].(*types.Builtin); ok { + if _, ok := v.typesInfo.Uses[fun].(*types.Builtin); ok { return fun.Name == "recover" } } @@ -550,7 +513,7 @@ func (v *visitor) isRecover(call *ast.CallExpr) bool { } func (v *visitor) addErrorAtPosition(position token.Pos, call *ast.CallExpr) { - pos := v.pkg.Fset.Position(position) + pos := v.fset.Position(position) lines, ok := v.lines[pos.Filename] if !ok { lines = readfile(pos.Filename) diff --git a/errcheck/excludes.go b/errcheck/excludes.go new file mode 100644 index 0000000..f499e2d --- /dev/null +++ b/errcheck/excludes.go @@ -0,0 +1,79 @@ +package errcheck + +import ( + "bufio" + "bytes" + "io/ioutil" + "strings" +) + +var ( + // DefaultExcludedSymbols is a list of symbol names that are usually excluded from checks by default. + // + // Note, that they still need to be explicitly copied to Checker.Exclusions.Symbols + DefaultExcludedSymbols = []string{ + // bytes + "(*bytes.Buffer).Write", + "(*bytes.Buffer).WriteByte", + "(*bytes.Buffer).WriteRune", + "(*bytes.Buffer).WriteString", + + // fmt + "fmt.Errorf", + "fmt.Print", + "fmt.Printf", + "fmt.Println", + "fmt.Fprint(*bytes.Buffer)", + "fmt.Fprintf(*bytes.Buffer)", + "fmt.Fprintln(*bytes.Buffer)", + "fmt.Fprint(*strings.Builder)", + "fmt.Fprintf(*strings.Builder)", + "fmt.Fprintln(*strings.Builder)", + "fmt.Fprint(os.Stderr)", + "fmt.Fprintf(os.Stderr)", + "fmt.Fprintln(os.Stderr)", + + // math/rand + "math/rand.Read", + "(*math/rand.Rand).Read", + + // strings + "(*strings.Builder).Write", + "(*strings.Builder).WriteByte", + "(*strings.Builder).WriteRune", + "(*strings.Builder).WriteString", + + // hash + "(hash.Hash).Write", + } +) + +// ReadExcludes reads an excludes file, a newline delimited file that lists +// patterns for which to allow unchecked errors. +// +// Lines that start with two forward slashes are considered comments and are ignored. +// +func ReadExcludes(path string) ([]string, error) { + var excludes []string + + buf, err := ioutil.ReadFile(path) + if err != nil { + return nil, err + } + + scanner := bufio.NewScanner(bytes.NewReader(buf)) + + for scanner.Scan() { + name := scanner.Text() + // Skip comments and empty lines. + if strings.HasPrefix(name, "//") || name == "" { + continue + } + excludes = append(excludes, name) + } + if err := scanner.Err(); err != nil { + return nil, err + } + + return excludes, nil +} diff --git a/errcheck/excludes_test.go b/errcheck/excludes_test.go new file mode 100644 index 0000000..0535dc2 --- /dev/null +++ b/errcheck/excludes_test.go @@ -0,0 +1,39 @@ +package errcheck + +import ( + "reflect" + "testing" +) + +func TestReadExcludes(t *testing.T) { + expectedExcludes := []string{ + "hello()", + "world()", + } + t.Logf("expectedExcludes: %#v", expectedExcludes) + excludes, err := ReadExcludes("testdata/excludes.txt") + if err != nil { + t.Fatal(err) + } + t.Logf("excludes: %#v", excludes) + if !reflect.DeepEqual(expectedExcludes, excludes) { + t.Fatal("excludes did not match expectedExcludes") + } +} + +func TestReadEmptyExcludes(t *testing.T) { + excludes, err := ReadExcludes("testdata/empty_excludes.txt") + if err != nil { + t.Fatal(err) + } + if len(excludes) != 0 { + t.Fatalf("expected empty excludes, got %#v", excludes) + } +} + +func TestReadExcludesMissingFile(t *testing.T) { + _, err := ReadExcludes("testdata/missing_file") + if err == nil { + t.Fatal("expected non-nil err, got nil") + } +} diff --git a/main.go b/main.go index 641aa54..4cb2ed6 100644 --- a/main.go +++ b/main.go @@ -1,11 +1,8 @@ package main import ( - "bufio" - "bytes" "flag" "fmt" - "io/ioutil" "os" "path/filepath" "regexp" @@ -215,7 +212,7 @@ func parseFlags(checker *errcheck.Checker, args []string) ([]string, int) { } if excludeFile != "" { - excludes, err := readExcludes(excludeFile) + excludes, err := errcheck.ReadExcludes(excludeFile) if err != nil { fmt.Fprintf(os.Stderr, "Could not read exclude file: %v\n", err) return nil, exitFatalError @@ -240,33 +237,6 @@ func parseFlags(checker *errcheck.Checker, args []string) ([]string, int) { return paths, exitCodeOk } -// readExcludes reads an excludes file, a newline delimited file that lists -// patterns for which to allow unchecked errors. -func readExcludes(path string) ([]string, error) { - var excludes []string - - buf, err := ioutil.ReadFile(path) - if err != nil { - return nil, err - } - - scanner := bufio.NewScanner(bytes.NewReader(buf)) - - for scanner.Scan() { - name := scanner.Text() - // Skip comments and empty lines. - if strings.HasPrefix(name, "//") || name == "" { - continue - } - excludes = append(excludes, name) - } - if err := scanner.Err(); err != nil { - return nil, err - } - - return excludes, nil -} - func main() { os.Exit(mainCmd(os.Args)) } diff --git a/main_test.go b/main_test.go index 60bf7d2..7d77a04 100644 --- a/main_test.go +++ b/main_test.go @@ -4,7 +4,6 @@ import ( "bytes" "io" "os" - "reflect" "regexp" "strings" "testing" @@ -250,36 +249,3 @@ func TestParseFlags(t *testing.T) { } } } - -func TestReadExcludes(t *testing.T) { - expectedExcludes := []string{ - "hello()", - "world()", - } - t.Logf("expectedExcludes: %#v", expectedExcludes) - excludes, err := readExcludes("testdata/excludes.txt") - if err != nil { - t.Fatal(err) - } - t.Logf("excludes: %#v", excludes) - if !reflect.DeepEqual(expectedExcludes, excludes) { - t.Fatal("excludes did not match expectedExcludes") - } -} - -func TestReadEmptyExcludes(t *testing.T) { - excludes, err := readExcludes("testdata/empty_excludes.txt") - if err != nil { - t.Fatal(err) - } - if len(excludes) != 0 { - t.Fatalf("expected empty excludes, got %#v", excludes) - } -} - -func TestReadExcludesMissingFile(t *testing.T) { - _, err := readExcludes("testdata/missing_file") - if err == nil { - t.Fatal("expected non-nil err, got nil") - } -} From 83b278b45f06bef383c04402e68cbdb7ef8cb579 Mon Sep 17 00:00:00 2001 From: Dimitri Tcaciuc Date: Sun, 28 Mar 2021 14:57:02 -0700 Subject: [PATCH 2/5] Fix tests --- {testdata => errcheck/testdata}/empty_excludes.txt | 0 {testdata => errcheck/testdata}/excludes.txt | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename {testdata => errcheck/testdata}/empty_excludes.txt (100%) rename {testdata => errcheck/testdata}/excludes.txt (100%) diff --git a/testdata/empty_excludes.txt b/errcheck/testdata/empty_excludes.txt similarity index 100% rename from testdata/empty_excludes.txt rename to errcheck/testdata/empty_excludes.txt diff --git a/testdata/excludes.txt b/errcheck/testdata/excludes.txt similarity index 100% rename from testdata/excludes.txt rename to errcheck/testdata/excludes.txt From 7154fcc88eba96bc0c6355466eaab378cb6698ba Mon Sep 17 00:00:00 2001 From: Dimitri Tcaciuc Date: Sun, 28 Mar 2021 15:07:02 -0700 Subject: [PATCH 3/5] analyzer: Return result --- errcheck/analyzer.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/errcheck/analyzer.go b/errcheck/analyzer.go index f6f70d6..996d1f6 100644 --- a/errcheck/analyzer.go +++ b/errcheck/analyzer.go @@ -4,15 +4,17 @@ import ( "fmt" "go/ast" "go/token" + "reflect" "regexp" "golang.org/x/tools/go/analysis" ) var Analyzer = &analysis.Analyzer{ - Name: "errcheck", - Doc: "check for unchecked errors", - Run: runAnalyzer, + Name: "errcheck", + Doc: "check for unchecked errors", + Run: runAnalyzer, + ResultType: reflect.TypeOf(Result{}), } var ( @@ -47,6 +49,7 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) { } } + var allErrors []UncheckedError for _, f := range pass.Files { v := &visitor{ typesInfo: pass.TypesInfo, @@ -68,7 +71,8 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) { }) } + allErrors = append(allErrors, v.errors...) } - return nil, nil + return Result{UncheckedErrors: allErrors}, nil } From 9b8014a6525369fbb9a42ce6d74675b30d188c6a Mon Sep 17 00:00:00 2001 From: Dimitri Tcaciuc Date: Sun, 28 Mar 2021 17:08:16 -0700 Subject: [PATCH 4/5] Add analyzer tests --- errcheck/analyzer.go | 6 +- errcheck/errcheck.go | 3 + errcheck/testdata/custom_exclude.txt | 1 + errcheck/testdata/src/a/main.go | 155 +++++++++++++++++++ errcheck/testdata/src/a/t.go | 17 ++ errcheck/testdata/src/assert/main.go | 6 + errcheck/testdata/src/blank/main.go | 9 ++ errcheck/testdata/src/custom_exclude/main.go | 14 ++ 8 files changed, 210 insertions(+), 1 deletion(-) create mode 100644 errcheck/testdata/custom_exclude.txt create mode 100644 errcheck/testdata/src/a/main.go create mode 100644 errcheck/testdata/src/a/t.go create mode 100644 errcheck/testdata/src/assert/main.go create mode 100644 errcheck/testdata/src/blank/main.go create mode 100644 errcheck/testdata/src/custom_exclude/main.go diff --git a/errcheck/analyzer.go b/errcheck/analyzer.go index 996d1f6..46dd592 100644 --- a/errcheck/analyzer.go +++ b/errcheck/analyzer.go @@ -64,10 +64,14 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) { ast.Walk(v, f) + // TODO have visitor report all facts and then sort out + // the ones we need to report based on flags. This can + // likely simplify the visitor code. + for _, err := range v.errors { pass.Report(analysis.Diagnostic{ Pos: token.Pos(int(f.Pos()) + err.Pos.Offset), - Message: fmt.Sprintf("unchecked error returned by %s", err.FuncName), + Message: "unchecked error", }) } diff --git a/errcheck/errcheck.go b/errcheck/errcheck.go index 24aa664..5440c01 100644 --- a/errcheck/errcheck.go +++ b/errcheck/errcheck.go @@ -512,6 +512,9 @@ func (v *visitor) isRecover(call *ast.CallExpr) bool { return false } +// TODO collect token.Pos and then convert them to UncheckedErrors +// after visitor is done running. This will allow to integrate more cleanly +// with analyzer so that we don't have to convert Position back to Pos. func (v *visitor) addErrorAtPosition(position token.Pos, call *ast.CallExpr) { pos := v.fset.Position(position) lines, ok := v.lines[pos.Filename] diff --git a/errcheck/testdata/custom_exclude.txt b/errcheck/testdata/custom_exclude.txt new file mode 100644 index 0000000..a16307a --- /dev/null +++ b/errcheck/testdata/custom_exclude.txt @@ -0,0 +1 @@ +(custom_exclude.ErrorMakerInterfaceWrapper).MakeNilError diff --git a/errcheck/testdata/src/a/main.go b/errcheck/testdata/src/a/main.go new file mode 100644 index 0000000..50034d9 --- /dev/null +++ b/errcheck/testdata/src/a/main.go @@ -0,0 +1,155 @@ +package a + +import ( + "bytes" + "crypto/sha256" + "fmt" + "io/ioutil" + "math/rand" + mrand "math/rand" +) + +func a() error { + fmt.Println("this function returns an error") // ok, excluded + return nil +} + +func b() (int, error) { + fmt.Println("this function returns an int and an error") // ok, excluded + return 0, nil +} + +func c() int { + fmt.Println("this function returns an int") // ok, excluded + return 7 +} + +func rec() { + defer func() { + recover() // want "unchecked error" + _ = recover() // ok, assigned to blank + }() + defer recover() // want "unchecked error" +} + +type MyError string + +func (e MyError) Error() string { + return string(e) +} + +func customError() error { + return MyError("an error occurred") +} + +func customConcreteError() MyError { + return MyError("an error occurred") +} + +func customConcreteErrorTuple() (int, MyError) { + return 0, MyError("an error occurred") +} + +type MyPointerError string + +func (e *MyPointerError) Error() string { + return string(*e) +} + +func customPointerError() *MyPointerError { + e := MyPointerError("an error occurred") + return &e +} + +func customPointerErrorTuple() (int, *MyPointerError) { + e := MyPointerError("an error occurred") + return 0, &e +} + +type ErrorMakerInterface interface { + MakeNilError() error +} +type ErrorMakerInterfaceWrapper interface { + ErrorMakerInterface +} + +func main() { + // Single error return + _ = a() // ok, assigned to blank + a() // want "unchecked error" + + // Return another value and an error + _, _ = b() // ok, assigned to blank + b() // want "unchecked error" + + // Return a custom error type + _ = customError() // ok, assigned to blank + customError() // want "unchecked error" + + // Return a custom concrete error type + _ = customConcreteError() // ok, assigned to blank + customConcreteError() // want "unchecked error" + _, _ = customConcreteErrorTuple() // ok, assigned to blank + customConcreteErrorTuple() // want "unchecked error" + + // Return a custom pointer error type + _ = customPointerError() // ok, assigned to blank + customPointerError() // want "unchecked error" + _, _ = customPointerErrorTuple() // ok, assigned to blank + customPointerErrorTuple() // want "unchecked error" + + // Method with a single error return + x := t{} + _ = x.a() // ok, assigned to blank + x.a() // want "unchecked error" + + // Method call on a struct member + y := u{x} + _ = y.t.a() // ok, assigned to blank + y.t.a() // want "unchecked error" + + m1 := map[string]func() error{"a": a} + _ = m1["a"]() // ok, assigned to blank + m1["a"]() // want "unchecked error" + + // Additional cases for assigning errors to blank identifier + z, _ := b() // ok, assigned to blank + _, w := a(), 5 // ok, assigned to blank + + // Assign non error to blank identifier + _ = c() + + _ = z + w // Avoid complaints about unused variables + + // Type assertions + var i interface{} + s1 := i.(string) // ok, would fail with -assert + s1 = i.(string) // ok, would fail with -assert + s2, _ := i.(string) // ok, would fail with -blank + s2, _ = i.(string) // ok, would fail with -blank + s3, ok := i.(string) + s3, ok = i.(string) + switch s4 := i.(type) { + case string: + _ = s4 + } + _, _, _, _ = s1, s2, s3, ok + + // Goroutine + go a() // want "unchecked error" + defer a() // want "unchecked error" + + // IO errors excluded by default + b1 := bytes.Buffer{} + b2 := &bytes.Buffer{} + b1.Write(nil) + b2.Write(nil) + rand.Read(nil) + mrand.Read(nil) + sha256.New().Write([]byte{}) + + ioutil.ReadFile("main.go") // want "unchecked error" + + var emiw ErrorMakerInterfaceWrapper + emiw.MakeNilError() // want "unchecked error" +} diff --git a/errcheck/testdata/src/a/t.go b/errcheck/testdata/src/a/t.go new file mode 100644 index 0000000..2e1a6e9 --- /dev/null +++ b/errcheck/testdata/src/a/t.go @@ -0,0 +1,17 @@ +// This file exists so that we can check that multi-file packages work +package a + +import "fmt" + +type t struct{} + +func (x t) a() error { + fmt.Println("this method returns an error") // EXCLUDED +//line myfile.txt:100 + fmt.Println("this method also returns an error") // EXCLUDED + return nil +} + +type u struct { + t t +} diff --git a/errcheck/testdata/src/assert/main.go b/errcheck/testdata/src/assert/main.go new file mode 100644 index 0000000..77feb6d --- /dev/null +++ b/errcheck/testdata/src/assert/main.go @@ -0,0 +1,6 @@ +package assert + +func main() { + var i interface{} + _ = i.(string) // want "unchecked error" +} diff --git a/errcheck/testdata/src/blank/main.go b/errcheck/testdata/src/blank/main.go new file mode 100644 index 0000000..eaf3281 --- /dev/null +++ b/errcheck/testdata/src/blank/main.go @@ -0,0 +1,9 @@ +package blank + +func a() error { + return nil +} + +func main() { + _ = a() // want "unchecked error" +} diff --git a/errcheck/testdata/src/custom_exclude/main.go b/errcheck/testdata/src/custom_exclude/main.go new file mode 100644 index 0000000..1ae20e1 --- /dev/null +++ b/errcheck/testdata/src/custom_exclude/main.go @@ -0,0 +1,14 @@ +package custom_excludes + +// Test custom excludes +type ErrorMakerInterface interface { + MakeNilError() error +} +type ErrorMakerInterfaceWrapper interface { + ErrorMakerInterface +} + +func main() { + var emiw ErrorMakerInterfaceWrapper + emiw.MakeNilError() // ok, custom exclude +} From 35d2ca6940dde1e480fa7a5445f20d258cb55c29 Mon Sep 17 00:00:00 2001 From: Dimitri Tcaciuc Date: Sun, 28 Mar 2021 17:17:47 -0700 Subject: [PATCH 5/5] Remove a TODO --- errcheck/analyzer.go | 4 ---- errcheck/errcheck.go | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/errcheck/analyzer.go b/errcheck/analyzer.go index 46dd592..8b444c7 100644 --- a/errcheck/analyzer.go +++ b/errcheck/analyzer.go @@ -64,10 +64,6 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) { ast.Walk(v, f) - // TODO have visitor report all facts and then sort out - // the ones we need to report based on flags. This can - // likely simplify the visitor code. - for _, err := range v.errors { pass.Report(analysis.Diagnostic{ Pos: token.Pos(int(f.Pos()) + err.Pos.Offset), diff --git a/errcheck/errcheck.go b/errcheck/errcheck.go index 5440c01..b3a67d4 100644 --- a/errcheck/errcheck.go +++ b/errcheck/errcheck.go @@ -512,7 +512,7 @@ func (v *visitor) isRecover(call *ast.CallExpr) bool { return false } -// TODO collect token.Pos and then convert them to UncheckedErrors +// TODO (dtcaciuc) collect token.Pos and then convert them to UncheckedErrors // after visitor is done running. This will allow to integrate more cleanly // with analyzer so that we don't have to convert Position back to Pos. func (v *visitor) addErrorAtPosition(position token.Pos, call *ast.CallExpr) {