From 2307df92cd1b8a1eaedb8503349c0a1603959079 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Fri, 9 Jul 2021 21:05:17 -0400 Subject: [PATCH 1/3] Allow errcheck exclude config without extra file One of the challenges in standardizing on a golangci-lint configuration is that if we want to configure `errcheck`, it requires two separate files to configure: the golangci config file, plus a separate errcheck excludes file. In particular, copying golangci-lint configs across projects often fails, as a reference to an exclude file at a particular file will not resolve. While this config does not add any new functionality, it allows the entirety of the errcheck configuration to be done inside the golangci-lint config file itself. --- .golangci.example.yml | 8 ++++++++ pkg/commands/run.go | 3 +++ pkg/config/linters_settings.go | 9 +++++---- pkg/golinters/errcheck.go | 2 ++ test/testdata/errcheck_exclude_functions.go | 19 +++++++++++++++++++ 5 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 test/testdata/errcheck_exclude_functions.go diff --git a/.golangci.example.yml b/.golangci.example.yml index 350029eae1fd..900bed14f597 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -117,6 +117,14 @@ linters-settings: # see https://github.com/kisielk/errcheck#excluding-functions for details exclude: /path/to/file.txt + # list of functions to exclude from checking, where each entry is a single + # function to exclude. + # see https://github.com/kisielk/errcheck#excluding-functions for details + exclude-functions: + - io/ioutil.ReadFile + - io.Copy(*bytes.Buffer) + - io.Copy(os.Stdout) + errorlint: # Check whether fmt.Errorf uses the %w verb for formatting errors. See the readme for caveats errorf: true diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 271fffe94f1a..ead0f474c681 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -133,6 +133,9 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is fs.StringVar(&lsc.Errcheck.Exclude, "errcheck.exclude", "", "Path to a file containing a list of functions to exclude from checking") hideFlag("errcheck.exclude") + fs.StringSliceVar(&lsc.Errcheck.ExcludeFunctions, "errcheck.exclude-functions", nil, + "List of functions to exclude from checking") + hideFlag("errcheck.exclude-functions") fs.StringVar(&lsc.Errcheck.Ignore, "errcheck.ignore", "fmt:.*", `Comma-separated list of pairs of the form pkg:regex. The regex is used to ignore names within pkg`) hideFlag("errcheck.ignore") diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 0a4d0a5b9821..1f35c6fd4672 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -161,10 +161,11 @@ type DuplSettings struct { } type ErrcheckSettings struct { - CheckTypeAssertions bool `mapstructure:"check-type-assertions"` - CheckAssignToBlank bool `mapstructure:"check-blank"` - Ignore string `mapstructure:"ignore"` - Exclude string `mapstructure:"exclude"` + CheckTypeAssertions bool `mapstructure:"check-type-assertions"` + CheckAssignToBlank bool `mapstructure:"check-blank"` + Ignore string `mapstructure:"ignore"` + Exclude string `mapstructure:"exclude"` + ExcludeFunctions []string `mapstructure:"exclude-functions"` } type ErrorLintSettings struct { diff --git a/pkg/golinters/errcheck.go b/pkg/golinters/errcheck.go index 9aac7326a0ca..2d9a4fc4cd05 100644 --- a/pkg/golinters/errcheck.go +++ b/pkg/golinters/errcheck.go @@ -152,6 +152,8 @@ func getChecker(errCfg *config.ErrcheckSettings) (*errcheck.Checker, error) { checker.Exclusions.Symbols = append(checker.Exclusions.Symbols, exclude...) } + checker.Exclusions.Symbols = append(checker.Exclusions.Symbols, errCfg.ExcludeFunctions...) + return &checker, nil } diff --git a/test/testdata/errcheck_exclude_functions.go b/test/testdata/errcheck_exclude_functions.go new file mode 100644 index 000000000000..82fcf93da6b0 --- /dev/null +++ b/test/testdata/errcheck_exclude_functions.go @@ -0,0 +1,19 @@ +//args: -Eerrcheck +//config: linters-settings.errcheck.check-blank=true +//config: linters-settings.errcheck.exclude-functions=io/ioutil.ReadFile,io/ioutil.ReadDir +package testdata + +import ( + "io/ioutil" +) + +func TestErrcheckExclude() []byte { + ret, _ := ioutil.ReadFile("f.txt") + _, _ = ioutil.ReadDir("dir") + return ret +} + +func TestErrcheckNoExclude() []byte { + ret, _ := ioutil.ReadAll(nil) // ERROR "Error return value of `ioutil.ReadAll` is not checked" + return ret +} From b3a5924929d1f8b5ac76692c53a705cfea43a66c Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Fri, 9 Jul 2021 21:56:27 -0400 Subject: [PATCH 2/3] Respond to review comments --- pkg/commands/run.go | 3 --- test/testdata/errcheck/exclude_functions.yml | 6 ++++++ test/testdata/errcheck_exclude_functions.go | 9 ++++----- 3 files changed, 10 insertions(+), 8 deletions(-) create mode 100644 test/testdata/errcheck/exclude_functions.yml diff --git a/pkg/commands/run.go b/pkg/commands/run.go index ead0f474c681..271fffe94f1a 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -133,9 +133,6 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is fs.StringVar(&lsc.Errcheck.Exclude, "errcheck.exclude", "", "Path to a file containing a list of functions to exclude from checking") hideFlag("errcheck.exclude") - fs.StringSliceVar(&lsc.Errcheck.ExcludeFunctions, "errcheck.exclude-functions", nil, - "List of functions to exclude from checking") - hideFlag("errcheck.exclude-functions") fs.StringVar(&lsc.Errcheck.Ignore, "errcheck.ignore", "fmt:.*", `Comma-separated list of pairs of the form pkg:regex. The regex is used to ignore names within pkg`) hideFlag("errcheck.ignore") diff --git a/test/testdata/errcheck/exclude_functions.yml b/test/testdata/errcheck/exclude_functions.yml new file mode 100644 index 000000000000..fcf07927a6d1 --- /dev/null +++ b/test/testdata/errcheck/exclude_functions.yml @@ -0,0 +1,6 @@ +linters-settings: + errcheck: + check-blank: true + exclude-functions: + - io/ioutil.ReadFile + - io/ioutil.ReadDir diff --git a/test/testdata/errcheck_exclude_functions.go b/test/testdata/errcheck_exclude_functions.go index 82fcf93da6b0..9ad8dda6cd97 100644 --- a/test/testdata/errcheck_exclude_functions.go +++ b/test/testdata/errcheck_exclude_functions.go @@ -1,19 +1,18 @@ //args: -Eerrcheck -//config: linters-settings.errcheck.check-blank=true -//config: linters-settings.errcheck.exclude-functions=io/ioutil.ReadFile,io/ioutil.ReadDir +//config_path: testdata/errcheck/exclude_functions.yml package testdata import ( "io/ioutil" ) -func TestErrcheckExclude() []byte { +func TestErrcheckExcludeFunctions() []byte { ret, _ := ioutil.ReadFile("f.txt") - _, _ = ioutil.ReadDir("dir") + ioutil.ReadDir("dir") return ret } -func TestErrcheckNoExclude() []byte { +func TestErrcheckNoExcludeFunctions() []byte { ret, _ := ioutil.ReadAll(nil) // ERROR "Error return value of `ioutil.ReadAll` is not checked" return ret } From 42e7381d8589e38462ac1807e3d79ced97ef64fa Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 10 Jul 2021 04:18:21 +0200 Subject: [PATCH 3/3] review: depecate errcheck.exclude --- .golangci.example.yml | 4 ++-- .golangci.yml | 6 ++++++ pkg/config/linters_settings.go | 4 +++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.golangci.example.yml b/.golangci.example.yml index 900bed14f597..7cdb607310d4 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -113,12 +113,12 @@ linters-settings: # see https://github.com/kisielk/errcheck#the-deprecated-method for details ignore: fmt:.*,io/ioutil:^Read.* + # [deprecated] use exclude-functions instead. # path to a file containing a list of functions to exclude from checking # see https://github.com/kisielk/errcheck#excluding-functions for details exclude: /path/to/file.txt - # list of functions to exclude from checking, where each entry is a single - # function to exclude. + # list of functions to exclude from checking, where each entry is a single function to exclude. # see https://github.com/kisielk/errcheck#excluding-functions for details exclude-functions: - io/ioutil.ReadFile diff --git a/.golangci.yml b/.golangci.yml index 12768450a5c8..bc6efa238693 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -122,6 +122,12 @@ issues: - path: _test\.go linters: - gomnd + + - path: pkg/golinters/errcheck.go + text: "SA1019: errCfg.Exclude is deprecated: use ExcludeFunctions instead" + - path: pkg/commands/run.go + text: "SA1019: lsc.Errcheck.Exclude is deprecated: use ExcludeFunctions instead" + # TODO must be removed after the release of the next version (v1.41.0) - path: pkg/commands/run.go linters: diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 1f35c6fd4672..62fc59d6663e 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -164,8 +164,10 @@ type ErrcheckSettings struct { CheckTypeAssertions bool `mapstructure:"check-type-assertions"` CheckAssignToBlank bool `mapstructure:"check-blank"` Ignore string `mapstructure:"ignore"` - Exclude string `mapstructure:"exclude"` ExcludeFunctions []string `mapstructure:"exclude-functions"` + + // Deprecated: use ExcludeFunctions instead + Exclude string `mapstructure:"exclude"` } type ErrorLintSettings struct {