diff --git a/internal/errcheck/embedded_walker.go b/errcheck/embedded_walker.go similarity index 100% rename from internal/errcheck/embedded_walker.go rename to errcheck/embedded_walker.go diff --git a/internal/errcheck/embedded_walker_test.go b/errcheck/embedded_walker_test.go similarity index 100% rename from internal/errcheck/embedded_walker_test.go rename to errcheck/embedded_walker_test.go diff --git a/internal/errcheck/errcheck.go b/errcheck/errcheck.go similarity index 70% rename from internal/errcheck/errcheck.go rename to errcheck/errcheck.go index ddf9b11..74bfaf0 100644 --- a/internal/errcheck/errcheck.go +++ b/errcheck/errcheck.go @@ -1,6 +1,4 @@ // Package errcheck is the library used to implement the errcheck command-line tool. -// -// Note: The API of this package has not been finalized and may change at any point. package errcheck import ( @@ -11,12 +9,9 @@ import ( "go/token" "go/types" "os" - "os/exec" "regexp" - "runtime" "sort" "strings" - "sync" "golang.org/x/tools/go/packages" ) @@ -30,9 +25,11 @@ 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") - // DefaultExcludes is a list of symbols that are excluded from checks by default. - // Note that they still need to be explicitly added to checker with AddExcludes(). - DefaultExcludes = []string{ + + // 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", @@ -76,38 +73,22 @@ type UncheckedError struct { FuncName string } -// UncheckedErrors is returned from the CheckPackage function if the package contains -// any unchecked errors. -// Errors should be appended using the Append method, which is safe to use concurrently. -type UncheckedErrors struct { - mu sync.Mutex - - // Errors is a list of all the unchecked errors in the package. +// Result is returned from the CheckPackage function, and holds all the errors +// that were found to be unchecked in a package. +// +// Aggregation can be done using the Append method for users that want to +// combine results from multiple packages. +type Result struct { + // UncheckedErrors is a list of all the unchecked errors in the package. // Printing an error reports its position within the file and the contents of the line. - Errors []UncheckedError -} - -func (e *UncheckedErrors) Append(errors ...UncheckedError) { - e.mu.Lock() - defer e.mu.Unlock() - e.Errors = append(e.Errors, errors...) + UncheckedErrors []UncheckedError } -func (e *UncheckedErrors) Error() string { - return fmt.Sprintf("%d unchecked errors", len(e.Errors)) -} - -// Len is the number of elements in the collection. -func (e *UncheckedErrors) Len() int { return len(e.Errors) } - -// Swap swaps the elements with indexes i and j. -func (e *UncheckedErrors) Swap(i, j int) { e.Errors[i], e.Errors[j] = e.Errors[j], e.Errors[i] } - -type byName struct{ *UncheckedErrors } +type byName []UncheckedError // Less reports whether the element with index i should sort before the element with index j. -func (e byName) Less(i, j int) bool { - ei, ej := e.Errors[i], e.Errors[j] +func (b byName) Less(i, j int) bool { + ei, ej := b[i], b[j] pi, pj := ei.Pos, ej.Pos @@ -124,48 +105,88 @@ func (e byName) Less(i, j int) bool { return ei.Line < ej.Line } -type Checker struct { - // ignore is a map of package names to regular expressions. Identifiers from a package are - // checked against its regular expressions and if any of the expressions match the call - // is not checked. - Ignore map[string]*regexp.Regexp - - // If blank is true then assignments to the blank identifier are also considered to be - // ignored errors. - Blank bool - - // If asserts is true then ignored type assertion results are also checked - Asserts bool - - // build tags - Tags []string - - Verbose bool - - // If true, checking of _test.go files is disabled - WithoutTests bool - - // If true, checking of files with generated code is disabled - WithoutGeneratedCode bool +func (b byName) Swap(i, j int) { + b[i], b[j] = b[j], b[i] +} - exclude map[string]bool +func (b byName) Len() int { + return len(b) } -func NewChecker() *Checker { - return &Checker{exclude: map[string]bool{}} +// Append appends errors to e. Append does not do any duplicate checking. +func (r *Result) Append(other Result) { + r.UncheckedErrors = append(r.UncheckedErrors, other.UncheckedErrors...) } -func (c *Checker) AddExcludes(excludes []string) { - for _, k := range excludes { - c.logf("Excluding %v", k) - c.exclude[k] = true +// Returns the unique errors that have been accumulated. Duplicates may occur +// when a file containing an unchecked error belongs to > 1 package. +// +// The method receiver remains unmodified after the call to Unique. +func (r Result) Unique() Result { + result := make([]UncheckedError, len(r.UncheckedErrors)) + copy(result, r.UncheckedErrors) + sort.Sort((byName)(result)) + uniq := result[:0] // compact in-place + for i, err := range result { + if i == 0 || err != result[i-1] { + uniq = append(uniq, err) + } } + return Result{UncheckedErrors: uniq} } -func (c *Checker) logf(msg string, args ...interface{}) { - if c.Verbose { - fmt.Fprintf(os.Stderr, msg+"\n", args...) - } +// Exclusions define symbols and language elements that will be not checked +type Exclusions struct { + + // Packages lists paths of excluded packages. + Packages []string + + // SymbolRegexpsByPackage maps individual package paths to regular + // expressions that match symbols to be excluded. + // + // Packages whose paths appear both here and in Packages list will + // be excluded entirely. + // + // This is a legacy input that will be deprecated in errcheck version 2 and + // should not be used. + SymbolRegexpsByPackage map[string]*regexp.Regexp + + // Symbols lists patterns that exclude individual package symbols. + // + // For example: + // + // "fmt.Errorf" // function + // "fmt.Fprintf(os.Stderr)" // function with set argument value + // "(hash.Hash).Write" // method + // + Symbols []string + + // TestFiles excludes _test.go files. + TestFiles bool + + // GeneratedFiles excludes generated source files. + // + // Source file is assumed to be generated if its contents + // match the following regular expression: + // + // ^// Code generated .* DO NOT EDIT\\.$ + // + GeneratedFiles bool + + // BlankAssignments ignores assignments to blank identifier. + BlankAssignments bool + + // TypeAssertions ignores unchecked type assertions. + TypeAssertions bool +} + +// Checker checks that you checked errors. +type Checker struct { + // Exclusions defines code packages, symbols, and other elements that will not be checked. + Exclusions Exclusions + + // Tags are a list of build tags to use. + Tags []string } // loadPackages is used for testing. @@ -173,19 +194,22 @@ var loadPackages = func(cfg *packages.Config, paths ...string) ([]*packages.Pack return packages.Load(cfg, paths...) } -func (c *Checker) load(paths ...string) ([]*packages.Package, error) { +// LoadPackages loads all the packages in all the paths provided. It uses the +// exclusions and build tags provided to by the user when loading the packages. +func (c *Checker) LoadPackages(paths ...string) ([]*packages.Package, error) { cfg := &packages.Config{ Mode: packages.LoadAllSyntax, - Tests: !c.WithoutTests, + Tests: !c.Exclusions.TestFiles, BuildFlags: []string{fmtTags(c.Tags)}, } return loadPackages(cfg, paths...) } var generatedCodeRegexp = regexp.MustCompile("^// Code generated .* DO NOT EDIT\\.$") +var dotStar = regexp.MustCompile(".*") func (c *Checker) shouldSkipFile(file *ast.File) bool { - if !c.WithoutGeneratedCode { + if !c.Exclusions.GeneratedFiles { return false } @@ -200,94 +224,57 @@ func (c *Checker) shouldSkipFile(file *ast.File) bool { return false } -// CheckPackages checks packages for errors. -func (c *Checker) CheckPackages(paths ...string) error { - pkgs, err := c.load(paths...) - if err != nil { - return err - } - // Check for errors in the initial packages. - work := make(chan *packages.Package, len(pkgs)) - for _, pkg := range pkgs { - if len(pkg.Errors) > 0 { - return fmt.Errorf("errors while loading package %s: %v", pkg.ID, pkg.Errors) - } - work <- pkg - } - close(work) - - gomod, err := exec.Command("go", "env", "GOMOD").Output() - go111module := (err == nil) && strings.TrimSpace(string(gomod)) != "" - ignore := c.Ignore - if go111module { - ignore = make(map[string]*regexp.Regexp) - for pkg, re := range c.Ignore { - if nonVendoredPkg, ok := nonVendoredPkgPath(pkg); ok { - ignore[nonVendoredPkg] = re - } else { - ignore[pkg] = re - } +// CheckPackage checks packages for errors that have not been checked. +// +// It will exclude specific errors from analysis if the user has configured +// exclusions. +func (c *Checker) CheckPackage(pkg *packages.Package) Result { + excludedSymbols := map[string]bool{} + for _, sym := range c.Exclusions.Symbols { + excludedSymbols[sym] = true + } + + ignore := map[string]*regexp.Regexp{} + // Apply SymbolRegexpsByPackage first so that if the same path appears in + // Packages, a more narrow regexp will be superceded by dotStar below. + if regexps := c.Exclusions.SymbolRegexpsByPackage; regexps != nil { + for pkg, re := range regexps { + // TODO warn if previous entry overwritten? + ignore[nonVendoredPkgPath(pkg)] = re } } + for _, pkg := range c.Exclusions.Packages { + // TODO warn if previous entry overwritten? + ignore[nonVendoredPkgPath(pkg)] = dotStar + } - var wg sync.WaitGroup - u := &UncheckedErrors{} - for i := 0; i < runtime.NumCPU(); i++ { - wg.Add(1) - - go func() { - defer wg.Done() - for pkg := range work { - c.logf("Checking %s", pkg.Types.Path()) - - v := &visitor{ - pkg: pkg, - ignore: ignore, - blank: c.Blank, - asserts: c.Asserts, - lines: make(map[string][]string), - exclude: c.exclude, - go111module: go111module, - errors: []UncheckedError{}, - } + v := &visitor{ + pkg: pkg, + 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 { - if c.shouldSkipFile(astFile) { - continue - } - ast.Walk(v, astFile) - } - u.Append(v.errors...) - } - }() - } - - wg.Wait() - if u.Len() > 0 { - // Sort unchecked errors and remove duplicates. Duplicates may occur when a file - // containing an unchecked error belongs to > 1 package. - sort.Sort(byName{u}) - uniq := u.Errors[:0] // compact in-place - for i, err := range u.Errors { - if i == 0 || err != u.Errors[i-1] { - uniq = append(uniq, err) - } + for _, astFile := range v.pkg.Syntax { + if c.shouldSkipFile(astFile) { + continue } - u.Errors = uniq - return u + ast.Walk(v, astFile) } - return nil + return Result{UncheckedErrors: v.errors} } // 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 - go111module bool + pkg *packages.Package + ignore map[string]*regexp.Regexp + blank bool + asserts bool + lines map[string][]string + exclude map[string]bool errors []UncheckedError } @@ -450,34 +437,24 @@ func (v *visitor) ignoreCall(call *ast.CallExpr) bool { if obj := v.pkg.TypesInfo.Uses[id]; obj != nil { if pkg := obj.Pkg(); pkg != nil { - if re, ok := v.ignore[pkg.Path()]; ok { + if re, ok := v.ignore[nonVendoredPkgPath(pkg.Path())]; ok { return re.MatchString(id.Name) } - - // if current package being considered is vendored, check to see if it should be ignored based - // on the unvendored path. - if !v.go111module { - if nonVendoredPkg, ok := nonVendoredPkgPath(pkg.Path()); ok { - if re, ok := v.ignore[nonVendoredPkg]; ok { - return re.MatchString(id.Name) - } - } - } } } return false } -// nonVendoredPkgPath returns the unvendored version of the provided package path (or returns the provided path if it -// does not represent a vendored path). The second return value is true if the provided package was vendored, false -// otherwise. -func nonVendoredPkgPath(pkgPath string) (string, bool) { +// nonVendoredPkgPath returns the unvendored version of the provided package +// path (or returns the provided path if it does not represent a vendored +// path). +func nonVendoredPkgPath(pkgPath string) string { lastVendorIndex := strings.LastIndex(pkgPath, "/vendor/") if lastVendorIndex == -1 { - return pkgPath, false + return pkgPath } - return pkgPath[lastVendorIndex+len("/vendor/"):], true + return pkgPath[lastVendorIndex+len("/vendor/"):] } // errorsByArg returns a slice s such that diff --git a/internal/errcheck/errcheck_test.go b/errcheck/errcheck_test.go similarity index 70% rename from internal/errcheck/errcheck_test.go rename to errcheck/errcheck_test.go index f3343ad..472c023 100644 --- a/internal/errcheck/errcheck_test.go +++ b/errcheck/errcheck_test.go @@ -6,6 +6,7 @@ import ( "os" "path" "regexp" + "strings" "testing" "golang.org/x/tools/go/packages" @@ -165,35 +166,40 @@ package custom }, } - for i, currCase := range cases { - checker := NewChecker() - checker.Tags = currCase.tags + for _, test := range cases { + testName := strings.Join(test.tags, ",") + t.Run(testName, func(t *testing.T) { + var checker Checker + checker.Tags = test.tags - loadPackages = func(cfg *packages.Config, paths ...string) ([]*packages.Package, error) { - cfg.Env = append(os.Environ(), - "GOPATH="+tmpGopath) - cfg.Dir = testBuildTagsDir - pkgs, err := packages.Load(cfg, paths...) - return pkgs, err - } - err := checker.CheckPackages("github.com/testbuildtags") - - if currCase.numExpectedErrs == 0 { + loadPackages = func(cfg *packages.Config, paths ...string) ([]*packages.Package, error) { + cfg.Env = append(os.Environ(), + "GOPATH="+tmpGopath) + cfg.Dir = testBuildTagsDir + pkgs, err := packages.Load(cfg, paths...) + return pkgs, err + } + packages, err := checker.LoadPackages("github.com/testbuildtags") if err != nil { - t.Errorf("Case %d: expected no errors, but got: %v", i, err) + t.Fatal(err) } - continue - } - uerr, ok := err.(*UncheckedErrors) - if !ok { - t.Errorf("Case %d: wrong error type returned: %v", i, err) - continue - } + uerr := &Result{} + for _, pkg := range packages { + uerr.Append(checker.CheckPackage(pkg)) + } + *uerr = uerr.Unique() + if test.numExpectedErrs == 0 { + if len(uerr.UncheckedErrors) != 0 { + t.Errorf("expected no errors, but got: %v", uerr) + } + return + } - if currCase.numExpectedErrs != len(uerr.Errors) { - t.Errorf("Case %d:\nExpected: %d errors\nActual: %d errors", i, currCase.numExpectedErrs, len(uerr.Errors)) - } + if test.numExpectedErrs != len(uerr.UncheckedErrors) { + t.Errorf("expected: %d errors\nactual: %d errors", test.numExpectedErrs, len(uerr.UncheckedErrors)) + } + }) } } @@ -271,35 +277,39 @@ require github.com/testlog v0.0.0 }, } - for i, currCase := range cases { - checker := NewChecker() - checker.Ignore = currCase.ignore - loadPackages = func(cfg *packages.Config, paths ...string) ([]*packages.Package, error) { - cfg.Env = append(os.Environ(), - "GOPATH="+tmpGopath, - "GOFLAGS=-mod=vendor") - cfg.Dir = testVendorDir - pkgs, err := packages.Load(cfg, paths...) - return pkgs, err - } - err := checker.CheckPackages("github.com/testvendor") - - if currCase.numExpectedErrs == 0 { + for i, test := range cases { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + var checker Checker + checker.Exclusions.SymbolRegexpsByPackage = test.ignore + loadPackages = func(cfg *packages.Config, paths ...string) ([]*packages.Package, error) { + cfg.Env = append(os.Environ(), + "GOPATH="+tmpGopath, + "GOFLAGS=-mod=vendor") + cfg.Dir = testVendorDir + pkgs, err := packages.Load(cfg, paths...) + return pkgs, err + } + packages, err := checker.LoadPackages("github.com/testvendor") if err != nil { - t.Errorf("Case %d: expected no errors, but got: %v", i, err) + t.Fatal(err) } - continue - } + uerr := &Result{} + for _, pkg := range packages { + uerr.Append(checker.CheckPackage(pkg)) + } + *uerr = uerr.Unique() - uerr, ok := err.(*UncheckedErrors) - if !ok { - t.Errorf("Case %d: wrong error type returned: %v", i, err) - continue - } + if test.numExpectedErrs == 0 { + if len(uerr.UncheckedErrors) != 0 { + t.Errorf("expected no errors, but got: %v", uerr) + } + return + } - if currCase.numExpectedErrs != len(uerr.Errors) { - t.Errorf("Case %d:\nExpected: %d errors\nActual: %d errors", i, currCase.numExpectedErrs, len(uerr.Errors)) - } + if test.numExpectedErrs != len(uerr.UncheckedErrors) { + t.Errorf("expected: %d errors\nactual: %d errors", test.numExpectedErrs, len(uerr.UncheckedErrors)) + } + }) } } @@ -367,35 +377,39 @@ require github.com/testlog v0.0.0 }, } - for i, currCase := range cases { - checker := NewChecker() - checker.WithoutGeneratedCode = currCase.withoutGeneratedCode - loadPackages = func(cfg *packages.Config, paths ...string) ([]*packages.Package, error) { - cfg.Env = append(os.Environ(), - "GOPATH="+tmpGopath, - "GOFLAGS=-mod=vendor") - cfg.Dir = testVendorDir - pkgs, err := packages.Load(cfg, paths...) - return pkgs, err - } - err := checker.CheckPackages(path.Join("github.com/testvendor")) - - if currCase.numExpectedErrs == 0 { + for i, test := range cases { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + var checker Checker + checker.Exclusions.GeneratedFiles = test.withoutGeneratedCode + loadPackages = func(cfg *packages.Config, paths ...string) ([]*packages.Package, error) { + cfg.Env = append(os.Environ(), + "GOPATH="+tmpGopath, + "GOFLAGS=-mod=vendor") + cfg.Dir = testVendorDir + pkgs, err := packages.Load(cfg, paths...) + return pkgs, err + } + packages, err := checker.LoadPackages("github.com/testvendor") if err != nil { - t.Errorf("Case %d: expected no errors, but got: %v", i, err) + t.Fatal(err) } - continue - } + uerr := Result{} + for _, pkg := range packages { + uerr.Append(checker.CheckPackage(pkg)) + } + uerr = uerr.Unique() - uerr, ok := err.(*UncheckedErrors) - if !ok { - t.Errorf("Case %d: wrong error type returned: %v", i, err) - continue - } + if test.numExpectedErrs == 0 { + if len(uerr.UncheckedErrors) != 0 { + t.Errorf("expected no errors, but got: %v", uerr) + } + return + } - if currCase.numExpectedErrs != len(uerr.Errors) { - t.Errorf("Case %d:\nExpected: %d errors\nActual: %d errors", i, currCase.numExpectedErrs, len(uerr.Errors)) - } + if test.numExpectedErrs != len(uerr.UncheckedErrors) { + t.Errorf("expected: %d errors\nactual: %d errors", test.numExpectedErrs, len(uerr.UncheckedErrors)) + } + }) } } @@ -404,19 +418,18 @@ func test(t *testing.T, f flags) { asserts bool = f&CheckAsserts != 0 blank bool = f&CheckBlank != 0 ) - checker := NewChecker() - checker.Asserts = asserts - checker.Blank = blank - checker.AddExcludes(DefaultExcludes) - checker.AddExcludes([]string{ + var checker Checker + checker.Exclusions.TypeAssertions = !asserts + checker.Exclusions.BlankAssignments = !blank + checker.Exclusions.Symbols = append(checker.Exclusions.Symbols, DefaultExcludedSymbols...) + checker.Exclusions.Symbols = append(checker.Exclusions.Symbols, fmt.Sprintf("(%s.ErrorMakerInterface).MakeNilError", testPackage), - }) - err := checker.CheckPackages(testPackage) - uerr, ok := err.(*UncheckedErrors) - if !ok { - t.Fatalf("wrong error type returned: %v", err) + ) + packages, err := checker.LoadPackages(testPackage) + if err != nil { + t.Fatal(err) } - + uerr := Result{} numErrors := len(uncheckedMarkers) if blank { numErrors += len(blankMarkers) @@ -425,11 +438,18 @@ func test(t *testing.T, f flags) { numErrors += len(assertMarkers) } - if len(uerr.Errors) != numErrors { - t.Errorf("got %d errors, want %d", len(uerr.Errors), numErrors) + for _, pkg := range packages { + err := checker.CheckPackage(pkg) + uerr.Append(err) + } + + uerr = uerr.Unique() + + if len(uerr.UncheckedErrors) != numErrors { + t.Errorf("got %d errors, want %d", len(uerr.UncheckedErrors), numErrors) unchecked_loop: for k := range uncheckedMarkers { - for _, e := range uerr.Errors { + for _, e := range uerr.UncheckedErrors { if newMarker(e) == k { continue unchecked_loop } @@ -439,7 +459,7 @@ func test(t *testing.T, f flags) { if blank { blank_loop: for k := range blankMarkers { - for _, e := range uerr.Errors { + for _, e := range uerr.UncheckedErrors { if newMarker(e) == k { continue blank_loop } @@ -450,7 +470,7 @@ func test(t *testing.T, f flags) { if asserts { assert_loop: for k := range assertMarkers { - for _, e := range uerr.Errors { + for _, e := range uerr.UncheckedErrors { if newMarker(e) == k { continue assert_loop } @@ -460,7 +480,7 @@ func test(t *testing.T, f flags) { } } - for i, err := range uerr.Errors { + for i, err := range uerr.UncheckedErrors { m := marker{err.Pos.Filename, err.Pos.Line} if !uncheckedMarkers[m] && !blankMarkers[m] && !assertMarkers[m] { t.Errorf("%d: unexpected error: %v", i, err) diff --git a/internal/errcheck/tags.go b/errcheck/tags.go similarity index 100% rename from internal/errcheck/tags.go rename to errcheck/tags.go diff --git a/internal/errcheck/tags_compat.go b/errcheck/tags_compat.go similarity index 100% rename from internal/errcheck/tags_compat.go rename to errcheck/tags_compat.go diff --git a/main.go b/main.go index a094f99..6369927 100644 --- a/main.go +++ b/main.go @@ -9,9 +9,12 @@ import ( "os" "path/filepath" "regexp" + "runtime" "strings" + "sync" - "github.com/kisielk/errcheck/internal/errcheck" + "github.com/kisielk/errcheck/errcheck" + "golang.org/x/tools/go/packages" ) const ( @@ -20,10 +23,14 @@ const ( exitFatalError ) -var abspath bool - type ignoreFlag map[string]*regexp.Regexp +// global flags +var ( + abspath bool + verbose bool +) + func (f ignoreFlag) String() string { pairs := make([]string, 0, len(f)) for pkg, re := range f { @@ -80,14 +87,12 @@ func (f *tagsFlag) Set(s string) error { return nil } -var dotStar = regexp.MustCompile(".*") - -func reportUncheckedErrors(e *errcheck.UncheckedErrors, verbose bool) { +func reportResult(e errcheck.Result) { wd, err := os.Getwd() if err != nil { wd = "" } - for _, uncheckedError := range e.Errors { + for _, uncheckedError := range e.UncheckedErrors { pos := uncheckedError.Pos.String() if !abspath { newPos, err := filepath.Rel(wd, pos) @@ -104,34 +109,82 @@ func reportUncheckedErrors(e *errcheck.UncheckedErrors, verbose bool) { } } +func logf(msg string, args ...interface{}) { + if verbose { + fmt.Fprintf(os.Stderr, msg+"\n", args...) + } +} + func mainCmd(args []string) int { - checker := errcheck.NewChecker() - paths, err := parseFlags(checker, args) - if err != exitCodeOk { - return err + var checker errcheck.Checker + paths, rc := parseFlags(&checker, args) + if rc != exitCodeOk { + return rc } - if err := checker.CheckPackages(paths...); err != nil { - if e, ok := err.(*errcheck.UncheckedErrors); ok { - reportUncheckedErrors(e, checker.Verbose) - return exitUncheckedError - } else if err == errcheck.ErrNoGoFiles { + result, err := checkPaths(&checker, paths...) + if err != nil { + if err == errcheck.ErrNoGoFiles { fmt.Fprintln(os.Stderr, err) return exitCodeOk } fmt.Fprintf(os.Stderr, "error: failed to check packages: %s\n", err) return exitFatalError } + if len(result.UncheckedErrors) > 0 { + reportResult(result) + return exitUncheckedError + } return exitCodeOk } +func checkPaths(c *errcheck.Checker, paths ...string) (errcheck.Result, error) { + pkgs, err := c.LoadPackages(paths...) + if err != nil { + return errcheck.Result{}, err + } + // Check for errors in the initial packages. + work := make(chan *packages.Package, len(pkgs)) + for _, pkg := range pkgs { + if len(pkg.Errors) > 0 { + return errcheck.Result{}, fmt.Errorf("errors while loading package %s: %v", pkg.ID, pkg.Errors) + } + work <- pkg + } + close(work) + + var wg sync.WaitGroup + result := &errcheck.Result{} + mu := &sync.Mutex{} + for i := 0; i < runtime.NumCPU(); i++ { + wg.Add(1) + + go func() { + defer wg.Done() + for pkg := range work { + logf("checking %s", pkg.Types.Path()) + r := c.CheckPackage(pkg) + mu.Lock() + result.Append(r) + mu.Unlock() + } + }() + } + + wg.Wait() + return result.Unique(), nil +} + func parseFlags(checker *errcheck.Checker, args []string) ([]string, int) { flags := flag.NewFlagSet(args[0], flag.ContinueOnError) - flags.BoolVar(&checker.Blank, "blank", false, "if true, check for errors assigned to blank identifier") - flags.BoolVar(&checker.Asserts, "asserts", false, "if true, check for ignored type assertion results") - flags.BoolVar(&checker.WithoutTests, "ignoretests", false, "if true, checking of _test.go files is disabled") - flags.BoolVar(&checker.WithoutGeneratedCode, "ignoregenerated", false, "if true, checking of files with generated code is disabled") - flags.BoolVar(&checker.Verbose, "verbose", false, "produce more verbose logging") + + var checkAsserts, checkBlanks bool + + flags.BoolVar(&checkBlanks, "blank", false, "if true, check for errors assigned to blank identifier") + flags.BoolVar(&checkAsserts, "asserts", false, "if true, check for ignored type assertion results") + flags.BoolVar(&checker.Exclusions.TestFiles, "ignoretests", false, "if true, checking of _test.go files is disabled") + flags.BoolVar(&checker.Exclusions.GeneratedFiles, "ignoregenerated", false, "if true, checking of files with generated code is disabled") + flags.BoolVar(&verbose, "verbose", false, "produce more verbose logging") flags.BoolVar(&abspath, "abspath", false, "print absolute paths to files") @@ -152,8 +205,11 @@ func parseFlags(checker *errcheck.Checker, args []string) ([]string, int) { return nil, exitFatalError } + checker.Exclusions.BlankAssignments = !checkBlanks + checker.Exclusions.TypeAssertions = !checkAsserts + if !excludeOnly { - checker.AddExcludes(errcheck.DefaultExcludes) + checker.Exclusions.Symbols = append(checker.Exclusions.Symbols, errcheck.DefaultExcludedSymbols...) } if excludeFile != "" { @@ -162,21 +218,23 @@ func parseFlags(checker *errcheck.Checker, args []string) ([]string, int) { fmt.Fprintf(os.Stderr, "Could not read exclude file: %v\n", err) return nil, exitFatalError } - checker.AddExcludes(excludes) + checker.Exclusions.Symbols = append(checker.Exclusions.Symbols, excludes...) } checker.Tags = tags for _, pkg := range strings.Split(*ignorePkg, ",") { if pkg != "" { - ignore[pkg] = dotStar + checker.Exclusions.Packages = append(checker.Exclusions.Packages, pkg) } } - checker.Ignore = ignore + + checker.Exclusions.SymbolRegexpsByPackage = ignore paths := flags.Args() if len(paths) == 0 { paths = []string{"."} } + return paths, exitCodeOk } diff --git a/main_test.go b/main_test.go index a903af2..60bf7d2 100644 --- a/main_test.go +++ b/main_test.go @@ -9,9 +9,11 @@ import ( "strings" "testing" - "github.com/kisielk/errcheck/internal/errcheck" + "github.com/kisielk/errcheck/errcheck" ) +var dotStar = regexp.MustCompile(".*") + func TestMain(t *testing.T) { saveStderr := os.Stderr saveStdout := os.Stdout @@ -64,10 +66,11 @@ func TestMain(t *testing.T) { type parseTestCase struct { args []string paths []string - ignore map[string]string - tags []string - blank bool - asserts bool + ignore map[string]string // Exclusions.SymbolRegexpsByPackage + pkgs []string // Exclusions.Packages + tags []string // Tags + blank bool // !BlankAssignments + asserts bool // !TypeAssertions error int } @@ -77,6 +80,7 @@ func TestParseFlags(t *testing.T) { args: []string{"errcheck"}, paths: []string{"."}, ignore: map[string]string{}, + pkgs: []string{}, tags: []string{}, blank: false, asserts: false, @@ -86,6 +90,7 @@ func TestParseFlags(t *testing.T) { args: []string{"errcheck", "-blank", "-asserts"}, paths: []string{"."}, ignore: map[string]string{}, + pkgs: []string{}, tags: []string{}, blank: true, asserts: true, @@ -95,6 +100,7 @@ func TestParseFlags(t *testing.T) { args: []string{"errcheck", "foo", "bar"}, paths: []string{"foo", "bar"}, ignore: map[string]string{}, + pkgs: []string{}, tags: []string{}, blank: false, asserts: false, @@ -104,6 +110,7 @@ func TestParseFlags(t *testing.T) { args: []string{"errcheck", "-ignore", "fmt:.*,encoding/binary:.*"}, paths: []string{"."}, ignore: map[string]string{"fmt": ".*", "encoding/binary": dotStar.String()}, + pkgs: []string{}, tags: []string{}, blank: false, asserts: false, @@ -113,6 +120,7 @@ func TestParseFlags(t *testing.T) { args: []string{"errcheck", "-ignore", "fmt:[FS]?[Pp]rint*"}, paths: []string{"."}, ignore: map[string]string{"fmt": "[FS]?[Pp]rint*"}, + pkgs: []string{}, tags: []string{}, blank: false, asserts: false, @@ -122,6 +130,7 @@ func TestParseFlags(t *testing.T) { args: []string{"errcheck", "-ignore", "[rR]ead|[wW]rite"}, paths: []string{"."}, ignore: map[string]string{"": "[rR]ead|[wW]rite"}, + pkgs: []string{}, tags: []string{}, blank: false, asserts: false, @@ -130,7 +139,8 @@ func TestParseFlags(t *testing.T) { parseTestCase{ args: []string{"errcheck", "-ignorepkg", "testing"}, paths: []string{"."}, - ignore: map[string]string{"testing": dotStar.String()}, + ignore: map[string]string{}, + pkgs: []string{"testing"}, tags: []string{}, blank: false, asserts: false, @@ -139,7 +149,8 @@ func TestParseFlags(t *testing.T) { parseTestCase{ args: []string{"errcheck", "-ignorepkg", "testing,foo"}, paths: []string{"."}, - ignore: map[string]string{"testing": dotStar.String(), "foo": dotStar.String()}, + ignore: map[string]string{}, + pkgs: []string{"testing", "foo"}, tags: []string{}, blank: false, asserts: false, @@ -149,6 +160,7 @@ func TestParseFlags(t *testing.T) { args: []string{"errcheck", "-tags", "foo"}, paths: []string{"."}, ignore: map[string]string{}, + pkgs: []string{}, tags: []string{"foo"}, blank: false, asserts: false, @@ -158,6 +170,7 @@ func TestParseFlags(t *testing.T) { args: []string{"errcheck", "-tags", "foo bar !baz"}, paths: []string{"."}, ignore: map[string]string{}, + pkgs: []string{}, tags: []string{"foo", "bar", "!baz"}, blank: false, asserts: false, @@ -167,6 +180,7 @@ func TestParseFlags(t *testing.T) { args: []string{"errcheck", "-tags", "foo,bar,!baz"}, paths: []string{"."}, ignore: map[string]string{}, + pkgs: []string{}, tags: []string{"foo", "bar", "!baz"}, blank: false, asserts: false, @@ -176,6 +190,7 @@ func TestParseFlags(t *testing.T) { args: []string{"errcheck", "-tags", "foo bar !baz"}, paths: []string{"."}, ignore: map[string]string{}, + pkgs: []string{}, tags: []string{"foo", "bar", "!baz"}, blank: false, asserts: false, @@ -196,7 +211,7 @@ func TestParseFlags(t *testing.T) { } ignoresEqual := func(a map[string]*regexp.Regexp, b map[string]string) bool { - if len(a) != len(b) { + if (a == nil && b != nil) || (a != nil && b == nil) || (len(a) != len(b)) { return false } for k, v := range a { @@ -208,24 +223,27 @@ func TestParseFlags(t *testing.T) { } for _, c := range cases { - checker := errcheck.NewChecker() - p, e := parseFlags(checker, c.args) + var checker errcheck.Checker + p, e := parseFlags(&checker, c.args) argsStr := strings.Join(c.args, " ") if !slicesEqual(p, c.paths) { t.Errorf("%q: path got %q want %q", argsStr, p, c.paths) } - if ign := checker.Ignore; !ignoresEqual(ign, c.ignore) { + if ign := checker.Exclusions.SymbolRegexpsByPackage; !ignoresEqual(ign, c.ignore) { t.Errorf("%q: ignore got %q want %q", argsStr, ign, c.ignore) } + if pkgs := checker.Exclusions.Packages; !slicesEqual(pkgs, c.pkgs) { + t.Errorf("%q: packages got %v want %v", argsStr, pkgs, c.pkgs) + } if tags := checker.Tags; !slicesEqual(tags, c.tags) { t.Errorf("%q: tags got %v want %v", argsStr, tags, c.tags) } - if b := checker.Blank; b != c.blank { - t.Errorf("%q: blank got %v want %v", argsStr, b, c.blank) + if b := checker.Exclusions.BlankAssignments; b != !c.blank { + t.Errorf("%q: BlankAssignments got %v want %v", argsStr, b, !c.blank) } - if a := checker.Asserts; a != c.asserts { - t.Errorf("%q: asserts got %v want %v", argsStr, a, c.asserts) + if a := checker.Exclusions.TypeAssertions; a != !c.asserts { + t.Errorf("%q: TypeAssertions got %v want %v", argsStr, a, !c.asserts) } if e != c.error { t.Errorf("%q: error got %q want %q", argsStr, e, c.error)