Use errcheck from main repo instead of golangci-lint fork (#1319)

Co-authored-by: Roman Leventov <[email protected]>
Co-authored-by: Fernandez Ludovic <[email protected]>

Author: 3 people

go.mod

@@ -23,7 +23,6 @@ require (
 	github.com/gofrs/flock v0.8.0
 	github.com/golangci/check v0.0.0-20180506172741-cfe4005ccda2
 	github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a
-	github.com/golangci/errcheck v0.0.0-20181223084120-ef45e06d44b6
 	github.com/golangci/go-misc v0.0.0-20180628070357-927a3d87b613
 	github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a
 	github.com/golangci/ineffassign v0.0.0-20190609212857-42439a7714cc
@@ -35,6 +34,7 @@ require (
 	github.com/jgautheron/goconst v0.0.0-20201117150253-ccae5bf973f3
 	github.com/jingyugao/rowserrcheck v0.0.0-20210130005344-c6a0c12dd98d
 	github.com/jirfag/go-printf-func-name v0.0.0-20191110105641-45db9963cdd3
+	github.com/kisielk/errcheck v1.6.0
 	github.com/kulti/thelper v0.4.0
 	github.com/kunwardeep/paralleltest v1.0.2
 	github.com/kyoh86/exportloopref v0.1.8

go.sum

@@ -205,7 +205,9 @@ func computeConfigSalt(cfg *config.Config) ([]byte, error) {
 	configData.WriteString("\nbuild-tags=%s" + strings.Join(cfg.Run.BuildTags, ","))
 
 	h := sha256.New()
-	h.Write(configData.Bytes()) //nolint:errcheck
+	if _, err := h.Write(configData.Bytes()); err != nil {
+		return nil, err
+	}
 	return h.Sum(nil), nil
 }
 

pkg/commands/executor.go

@@ -10,9 +10,10 @@ import (
 	"strings"
 	"sync"
 
-	errcheck "github.com/golangci/errcheck/golangci"
+	"github.com/kisielk/errcheck/errcheck"
 	"github.com/pkg/errors"
 	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/packages"
 
 	"github.com/golangci/golangci-lint/pkg/config"
 	"github.com/golangci/golangci-lint/pkg/fsutils"
@@ -23,51 +24,70 @@ import (
 
 func NewErrcheck() *goanalysis.Linter {
 	const linterName = "errcheck"
+
 	var mu sync.Mutex
 	var res []goanalysis.Issue
+
 	analyzer := &analysis.Analyzer{
 		Name: linterName,
 		Doc:  goanalysis.TheOnlyanalyzerDoc,
 	}
+
 	return goanalysis.NewLinter(
 		linterName,
 		"Errcheck is a program for checking for unchecked errors "+
 			"in go programs. These unchecked errors can be critical bugs in some cases",
 		[]*analysis.Analyzer{analyzer},
 		nil,
 	).WithContextSetter(func(lintCtx *linter.Context) {
+		// copied from errcheck
+		checker, err := getChecker(&lintCtx.Settings().Errcheck)
+		if err != nil {
+			lintCtx.Log.Errorf("failed to get checker: %v", err)
+			return
+		}
+
+		checker.Tags = lintCtx.Cfg.Run.BuildTags
+
 		analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
-			prog := goanalysis.MakeFakeLoaderProgram(pass)
-			errCfg, err := genConfig(&lintCtx.Settings().Errcheck)
-			if err != nil {
-				return nil, err
-			}
-			errcheckIssues, err := errcheck.RunWithConfig(prog, errCfg)
-			if err != nil {
-				return nil, err
+			pkg := &packages.Package{
+				Fset:      pass.Fset,
+				Syntax:    pass.Files,
+				Types:     pass.Pkg,
+				TypesInfo: pass.TypesInfo,
 			}
 
-			if len(errcheckIssues) == 0 {
+			errcheckIssues := checker.CheckPackage(pkg).Unique()
+			if len(errcheckIssues.UncheckedErrors) == 0 {
 				return nil, nil
 			}
 
-			issues := make([]goanalysis.Issue, 0, len(errcheckIssues))
-			for _, i := range errcheckIssues {
+			issues := make([]goanalysis.Issue, len(errcheckIssues.UncheckedErrors))
+			for i, err := range errcheckIssues.UncheckedErrors {
 				var text string
-				if i.FuncName != "" {
-					text = fmt.Sprintf("Error return value of %s is not checked", formatCode(i.FuncName, lintCtx.Cfg))
+				if err.FuncName != "" {
+					text = fmt.Sprintf(
+						"Error return value of %s is not checked",
+						formatCode(err.SelectorName, lintCtx.Cfg),
+					)
 				} else {
 					text = "Error return value is not checked"
 				}
-				issues = append(issues, goanalysis.NewIssue(&result.Issue{
-					FromLinter: linterName,
-					Text:       text,
-					Pos:        i.Pos,
-				}, pass))
+
+				issues[i] = goanalysis.NewIssue(
+					&result.Issue{
+						FromLinter: linterName,
+						Text:       text,
+						Pos:        err.Pos,
+					},
+					pass,
+				)
 			}
+
 			mu.Lock()
 			res = append(res, issues...)
 			mu.Unlock()
+
 			return nil, nil
 		}
 	}).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue {
@@ -104,27 +124,35 @@ func parseIgnoreConfig(s string) (map[string]*regexp.Regexp, error) {
 	return cfg, nil
 }
 
-func genConfig(errCfg *config.ErrcheckSettings) (*errcheck.Config, error) {
+func getChecker(errCfg *config.ErrcheckSettings) (*errcheck.Checker, error) {
 	ignoreConfig, err := parseIgnoreConfig(errCfg.Ignore)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to parse 'ignore' directive")
 	}
 
-	c := &errcheck.Config{
-		Ignore:  ignoreConfig,
-		Blank:   errCfg.CheckAssignToBlank,
-		Asserts: errCfg.CheckTypeAssertions,
+	checker := errcheck.Checker{
+		Exclusions: errcheck.Exclusions{
+			BlankAssignments:       !errCfg.CheckAssignToBlank,
+			TypeAssertions:         !errCfg.CheckTypeAssertions,
+			SymbolRegexpsByPackage: map[string]*regexp.Regexp{},
+			Symbols:                append([]string{}, errcheck.DefaultExcludedSymbols...),
+		},
+	}
+
+	for pkg, re := range ignoreConfig {
+		checker.Exclusions.SymbolRegexpsByPackage[pkg] = re
 	}
 
 	if errCfg.Exclude != "" {
 		exclude, err := readExcludeFile(errCfg.Exclude)
 		if err != nil {
 			return nil, err
 		}
-		c.Exclude = exclude
+
+		checker.Exclusions.Symbols = append(checker.Exclusions.Symbols, exclude...)
 	}
 
-	return c, nil
+	return &checker, nil
 }
 
 func getFirstPathArg() string {
@@ -192,7 +220,7 @@ func setupConfigFileSearch(name string) []string {
 	return configSearchPaths
 }
 
-func readExcludeFile(name string) (map[string]bool, error) {
+func readExcludeFile(name string) ([]string, error) {
 	var err error
 	var fh *os.File
 
@@ -205,13 +233,17 @@ func readExcludeFile(name string) (map[string]bool, error) {
 	if fh == nil {
 		return nil, errors.Wrapf(err, "failed reading exclude file: %s", name)
 	}
+
 	scanner := bufio.NewScanner(fh)
-	exclude := make(map[string]bool)
+
+	var excludes []string
 	for scanner.Scan() {
-		exclude[scanner.Text()] = true
+		excludes = append(excludes, scanner.Text())
 	}
+
 	if err := scanner.Err(); err != nil {
 		return nil, errors.Wrapf(err, "failed scanning file: %s", name)
 	}
-	return exclude, nil
+
+	return excludes, nil
 }

pkg/golinters/errcheck.go

@@ -55,7 +55,9 @@ func updateStateFile(replacements map[string]string) error {
 	}
 
 	h := sha256.New()
-	h.Write(replBytes) //nolint:errcheck
+	if _, err := h.Write(replBytes); err != nil {
+		return err
+	}
 
 	var contentBuf bytes.Buffer
 	contentBuf.WriteString("This file stores hash of website templates to trigger " +

scripts/expand_website_templates/main.go

@@ -135,7 +135,7 @@ func TestSortedResults(t *testing.T) {
 			"--sort-results=false",
 			strings.Join([]string{
 				"testdata/sort_results/main.go:12:5: `db` is unused (deadcode)",
-				"testdata/sort_results/main.go:15:13: Error return value of `returnError` is not checked (errcheck)",
+				"testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)",
 				"testdata/sort_results/main.go:8:6: func `returnError` is unused (unused)",
 			}, "\n"),
 		},
@@ -144,7 +144,7 @@ func TestSortedResults(t *testing.T) {
 			strings.Join([]string{
 				"testdata/sort_results/main.go:8:6: func `returnError` is unused (unused)",
 				"testdata/sort_results/main.go:12:5: `db` is unused (deadcode)",
-				"testdata/sort_results/main.go:15:13: Error return value of `returnError` is not checked (errcheck)",
+				"testdata/sort_results/main.go:15:13: Error return value is not checked (errcheck)",
 			}, "\n"),
 		},
 	}

test/run_test.go

@@ -12,7 +12,7 @@ func RetErr() error {
 }
 
 func MissedErrorCheck() {
-	RetErr() // ERROR "Error return value of `RetErr` is not checked"
+	RetErr() // ERROR "Error return value is not checked"
 }
 
 func IgnoreCloseMissingErrHandling() error {

test/testdata/errcheck.go

@@ -12,8 +12,8 @@ func TestErrcheckIgnoreOs() {
 	_, _ = os.Open("f.txt")
 }
 
-func TestErrcheckNoIgnoreFmt(s string) int {
-	n, _ := fmt.Println(s) // ERROR "Error return value of `fmt.Println` is not checked"
+func TestErrcheckIgnoreFmt(s string) int {
+	n, _ := fmt.Println(s)
 	return n
 }
 

test/testdata/errcheck_ignore.go

@@ -3,10 +3,17 @@
 package testdata
 
 import (
+	"crypto/sha256"
 	"fmt"
 	"os"
 )
 
+func TestErrcheckIgnoreHashWriteByDefault() []byte {
+	h := sha256.New()
+	h.Write([]byte("food"))
+	return h.Sum(nil)
+}
+
 func TestErrcheckIgnoreFmtByDefault(s string) int {
 	n, _ := fmt.Println(s)
 	return n

test/testdata/errcheck_ignore_default.go

@@ -0,0 +1,7 @@
+//args: -Eerrcheck
+//config: linters-settings.errcheck.check-type-assertions=true
+package testdata
+
+func ErrorTypeAssertion(filter map[string]interface{}) bool {
+	return filter["messages_sent.messageid"].(map[string]interface{})["$ne"] != nil
+}