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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
}