Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion pkg/commands/executor.go
Expand Up @@ -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
}

Expand Down
92 changes: 62 additions & 30 deletions pkg/golinters/errcheck.go
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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

Expand All @@ -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
}
4 changes: 3 additions & 1 deletion scripts/expand_website_templates/main.go
Expand Up @@ -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 " +
Expand Down
4 changes: 2 additions & 2 deletions test/run_test.go
Expand Up @@ -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"),
},
Expand All @@ -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"),
},
}
Expand Down
2 changes: 1 addition & 1 deletion test/testdata/errcheck.go
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions test/testdata/errcheck_ignore.go
Expand Up @@ -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
}

Expand Down
7 changes: 7 additions & 0 deletions test/testdata/errcheck_ignore_default.go
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions test/testdata/errcheck_type_assertions.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
}