diff --git a/.golangci.example.yml b/.golangci.example.yml index af2ccefc1d71..78ef68ae1b05 100644 --- a/.golangci.example.yml +++ b/.golangci.example.yml @@ -196,7 +196,10 @@ linters-settings: # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run` # By default list of stable checks is used. enabled-checks: - - rangeValCopy + - nestingReduce + - unnamedresult + - ruleguard + - truncateCmp # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty disabled-checks: diff --git a/go.mod b/go.mod index 57318d59d24c..8d4c32416074 100644 --- a/go.mod +++ b/go.mod @@ -63,6 +63,7 @@ require ( github.com/pkg/errors v0.9.1 github.com/polyfloyd/go-errorlint v0.0.0-20210722154253-910bb7978349 github.com/prometheus/procfs v0.6.0 // indirect + github.com/quasilyte/go-ruleguard/dsl v0.3.10 github.com/ryancurrah/gomodguard v1.2.3 github.com/ryanrolds/sqlclosecheck v0.3.0 github.com/sanposhiho/wastedassign/v2 v2.0.6 diff --git a/go.sum b/go.sum index 38d35d7e1925..6a0338afb6bc 100644 --- a/go.sum +++ b/go.sum @@ -622,6 +622,7 @@ github.com/quasilyte/go-ruleguard v0.3.1-0.20210203134552-1b5a410e1cc8/go.mod h1 github.com/quasilyte/go-ruleguard v0.3.13 h1:O1G41cq1jUr3cJmqp7vOUT0SokqjzmS9aESWJuIDRaY= github.com/quasilyte/go-ruleguard v0.3.13/go.mod h1:Ul8wwdqR6kBVOCt2dipDBkE+T6vAV/iixkrKuRTN1oQ= github.com/quasilyte/go-ruleguard/dsl v0.3.0/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU= +github.com/quasilyte/go-ruleguard/dsl v0.3.10 h1:4tVlVVcBT+nNWoF+t/zrAMO13sHAqYotX1K12Gc8f8A= github.com/quasilyte/go-ruleguard/dsl v0.3.10/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU= github.com/quasilyte/go-ruleguard/rules v0.0.0-20201231183845-9e62ed36efe1/go.mod h1:7JTjp89EGyU1d6XfBiXihJNG37wB2VRkd125Q1u7Plc= github.com/quasilyte/go-ruleguard/rules v0.0.0-20210428214800-545e0d2e0bf7/go.mod h1:4cgAphtvu7Ftv7vOT2ZOYhC6CvBxZixcasr8qIOTA50= diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 60c44d82f149..0ad37dde6ffc 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -55,6 +55,7 @@ type Executor struct { flock *flock.Flock } +// NewExecutor creates and initializes a new command executor. func NewExecutor(version, commit, date string) *Executor { startedAt := time.Now() e := &Executor{ diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 873dab817709..bb096942fd90 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -20,6 +20,7 @@ func (e *Executor) initLinters() { e.initRunConfiguration(e.lintersCmd) } +// executeLinters runs the 'linters' CLI command, which displays the supported linters. func (e *Executor) executeLinters(_ *cobra.Command, args []string) { if len(args) != 0 { e.log.Fatalf("Usage: golangci-lint linters") diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 1fb8c7b5ec2a..23a9b064acda 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -323,6 +323,7 @@ func fixSlicesFlags(fs *pflag.FlagSet) { }) } +// runAnalysis executes the linters that have been enabled in the configuration. func (e *Executor) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) { e.cfg.Run.Args = args @@ -444,6 +445,7 @@ func (e *Executor) createPrinter() (printers.Printer, error) { return p, nil } +// executeRun executes the 'run' CLI command, which runs the linters. func (e *Executor) executeRun(_ *cobra.Command, args []string) { needTrackResources := e.cfg.Run.IsVerbose || e.cfg.Run.PrintResourcesUsage trackResourcesEndCh := make(chan struct{}) diff --git a/pkg/config/config.go b/pkg/config/config.go index 06fca64b58b2..f41705c8959b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -2,7 +2,8 @@ package config // Config encapsulates the config data specified in the golangci yaml config file. type Config struct { - Run Run + cfgDir string // The directory containing the golangci config file. + Run Run Output Output @@ -16,6 +17,11 @@ type Config struct { InternalTest bool // Option is used only for testing golangci-lint code, don't use it } +// getConfigDir returns the directory that contains golangci config file. +func (c *Config) GetConfigDir() string { + return c.cfgDir +} + func NewDefault() *Config { return &Config{ LintersSettings: defaultLintersSettings, diff --git a/pkg/config/reader.go b/pkg/config/reader.go index 6e97277daa2a..9f368341b292 100644 --- a/pkg/config/reader.go +++ b/pkg/config/reader.go @@ -71,6 +71,11 @@ func (r *FileReader) parseConfig() error { r.log.Warnf("Can't pretty print config file path: %s", err) } r.log.Infof("Used config file %s", usedConfigFile) + usedConfigDir := filepath.Dir(usedConfigFile) + if usedConfigDir, err = filepath.Abs(usedConfigDir); err != nil { + return fmt.Errorf("can't get config directory") + } + r.cfg.cfgDir = usedConfigDir if err := viper.Unmarshal(r.cfg); err != nil { return fmt.Errorf("can't unmarshal config by viper: %s", err) diff --git a/pkg/config/run.go b/pkg/config/run.go index 43a5ff2e37e1..c091ee843381 100644 --- a/pkg/config/run.go +++ b/pkg/config/run.go @@ -12,7 +12,7 @@ type Run struct { Concurrency int PrintResourcesUsage bool `mapstructure:"print-resources-usage"` - Config string + Config string // The path to the golangci config file, as specified with the --config argument. NoConfig bool Args []string diff --git a/pkg/golinters/gocritic.go b/pkg/golinters/gocritic.go index ffb44058c375..0c32a8562023 100644 --- a/pkg/golinters/gocritic.go +++ b/pkg/golinters/gocritic.go @@ -78,7 +78,10 @@ func normalizeCheckerInfoParams(info *gocriticlinter.CheckerInfo) gocriticlinter return ret } -func configureCheckerInfo(info *gocriticlinter.CheckerInfo, allParams map[string]config.GocriticCheckSettings) error { +func configureCheckerInfo( + lintCtx *linter.Context, + info *gocriticlinter.CheckerInfo, + allParams map[string]config.GocriticCheckSettings) error { params := allParams[strings.ToLower(info.Name)] if params == nil { // no config for this checker return nil @@ -88,7 +91,7 @@ func configureCheckerInfo(info *gocriticlinter.CheckerInfo, allParams map[string for k, p := range params { v, ok := infoParams[k] if ok { - v.Value = normalizeCheckerParamsValue(p) + v.Value = normalizeCheckerParamsValue(lintCtx, p) continue } @@ -117,7 +120,7 @@ func configureCheckerInfo(info *gocriticlinter.CheckerInfo, allParams map[string // then we have to convert value types into the expected value types. // Maybe in the future, this kind of conversion will be done in go-critic itself. //nolint:exhaustive // only 3 types (int, bool, and string) are supported by CheckerParam.Value -func normalizeCheckerParamsValue(p interface{}) interface{} { +func normalizeCheckerParamsValue(lintCtx *linter.Context, p interface{}) interface{} { rv := reflect.ValueOf(p) switch rv.Type().Kind() { case reflect.Int64, reflect.Int32, reflect.Int16, reflect.Int8, reflect.Int: @@ -125,7 +128,8 @@ func normalizeCheckerParamsValue(p interface{}) interface{} { case reflect.Bool: return rv.Bool() case reflect.String: - return rv.String() + // Perform variable substitution. + return strings.ReplaceAll(rv.String(), "${configDir}", lintCtx.Cfg.GetConfigDir()) default: return p } @@ -141,7 +145,7 @@ func buildEnabledCheckers(lintCtx *linter.Context, linterCtx *gocriticlinter.Con continue } - if err := configureCheckerInfo(info, allParams); err != nil { + if err := configureCheckerInfo(lintCtx, info, allParams); err != nil { return nil, err } diff --git a/test/linters_test.go b/test/linters_test.go index 355370480e91..4431a2882122 100644 --- a/test/linters_test.go +++ b/test/linters_test.go @@ -23,7 +23,7 @@ func runGoErrchk(c *exec.Cmd, defaultExpectedLinter string, files []string, t *t if err != nil { var exitErr *exec.ExitError require.ErrorAs(t, err, &exitErr) - require.Equal(t, exitcodes.IssuesFound, exitErr.ExitCode()) + require.Equal(t, exitcodes.IssuesFound, exitErr.ExitCode(), "Unexpected exit code: %s", string(output)) } fullshort := make([]string, 0, len(files)*2) diff --git a/test/ruleguard/README.md b/test/ruleguard/README.md new file mode 100644 index 000000000000..2e2441698750 --- /dev/null +++ b/test/ruleguard/README.md @@ -0,0 +1 @@ +This directory contains ruleguard files that are used in functional tests. diff --git a/test/ruleguard/dup.go b/test/ruleguard/dup.go new file mode 100644 index 000000000000..944651b6f041 --- /dev/null +++ b/test/ruleguard/dup.go @@ -0,0 +1,22 @@ +// go:build ruleguard +package ruleguard + +import "github.com/quasilyte/go-ruleguard/dsl" + +// Suppose that we want to report the duplicated left and right operands of binary operations. +// +// But if the operand has some side effects, this rule can cause false positives: +// `f() && f()` can make sense (although it's not the best piece of code). +// +// This is where *filters* come to the rescue. +func DupSubExpr(m dsl.Matcher) { + // All filters are written as a Where() argument. + // In our case, we need to assert that $x is "pure". + // It can be achieved by checking the m["x"] member Pure field. + m.Match(`$x || $x`, + `$x && $x`, + `$x | $x`, + `$x & $x`). + Where(m["x"].Pure). + Report(`suspicious identical LHS and RHS`) +} diff --git a/test/ruleguard/strings_simplify.go b/test/ruleguard/strings_simplify.go new file mode 100644 index 000000000000..2e73a50b3621 --- /dev/null +++ b/test/ruleguard/strings_simplify.go @@ -0,0 +1,18 @@ +// go:build ruleguard +package ruleguard + +import "github.com/quasilyte/go-ruleguard/dsl" + +func StringsSimplify(m dsl.Matcher) { + // Some issues have simple fixes that can be expressed as + // a replacement pattern. Rules can use Suggest() function + // to add a quickfix action for such issues. + m.Match(`strings.Replace($s, $old, $new, -1)`). + Report(`this Replace call can be simplified`). + Suggest(`strings.ReplaceAll($s, $old, $new)`) + + // Suggest() can be used without Report(). + // It'll print the suggested template to the user. + m.Match(`strings.Count($s1, $s2) == 0`). + Suggest(`!strings.Contains($s1, $s2)`) +} diff --git a/test/testdata/configs/gocritic.yml b/test/testdata/configs/gocritic.yml index 268f2fb9e9a1..5cdda3736af5 100644 --- a/test/testdata/configs/gocritic.yml +++ b/test/testdata/configs/gocritic.yml @@ -3,6 +3,17 @@ linters-settings: enabled-checks: - rangeValCopy - flagDeref + - ruleguard settings: rangevalcopy: sizethreshold: 2 + ruleguard: + debug: dupSubExpr + failOn: dsl,import + # comma-separated paths to ruleguard files. + # The ${configDir} is substituted by the directory containing the golangci-lint config file. + # Note about the directory structure for functional tests: + # The ruleguard files used in functional tests cannot be under the 'testdata' directory. + # This is because they import the 'github.com/quasilyte/go-ruleguard/dsl' package, + # which needs to be added to go.mod. The testdata directory is ignored by go mod. + rules: '${configDir}/../../ruleguard/strings_simplify.go,${configDir}/../../ruleguard/dup.go' diff --git a/test/testdata/gocritic.go b/test/testdata/gocritic.go index 5e821a4e6553..1768304d1a7f 100644 --- a/test/testdata/gocritic.go +++ b/test/testdata/gocritic.go @@ -5,6 +5,7 @@ package testdata import ( "flag" "log" + "strings" ) var _ = *flag.Bool("global1", false, "") // ERROR `flagDeref: immediate deref in \*flag.Bool\(.global1., false, ..\) is most likely an error; consider using flag\.BoolVar` @@ -29,3 +30,15 @@ func gocriticRangeValCopySize2(ss []size2) { log.Print(s) } } + +func gocriticStringSimplify() { + s := "Most of the time, travellers worry about their luggage." + s = strings.Replace(s, ",", "", -1) // ERROR "ruleguard: this Replace call can be simplified.*" + log.Print(s) +} + +func gocriticDup(x bool) { + if x && x { // ERROR "ruleguard: suspicious identical LHS and RHS.*" + log.Print("x is true") + } +}