diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 7de210b98ba4..2bfb5e01bbc9 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -358,14 +358,30 @@ linters-settings: # Forbid the following identifiers (list of regexp). # Default: ["^(fmt\\.Print(|f|ln)|print|println)$"] forbid: + # Builtin function: - ^print.*$ - - 'fmt\.Print.*' - # Optionally put comments at the end of the regex, surrounded by `(# )?` - # Escape any special characters. + # Optional message that gets included in error reports. + - pattern: ^fmt\.Print.*$ + msg: Do not commit print statements. + # Alternatively, put messages at the end of the regex, surrounded by `(# )?` + # Escape any special characters. Those messages get included in error reports. - 'fmt\.Print.*(# Do not commit print statements\.)?' + # Forbid spew Dump, whether it is called as function or method. + # Depends on expand_expression below. + - ^spew\.(ConfigState\.)?Dump$ + # The package name in expanded expressions might be ambiguous. + # The full import path can be used as additional criteria. + - pattern: ^v1.Dump$ + pkg: ^example.com/pkg/api/v1$ # Exclude godoc examples from forbidigo checks. # Default: true - exclude_godoc_examples: false + exclude-godoc-examples: false + # Instead of matching the literal source code, use type information to + # replace expressions with strings that contain the package name and (for + # methods and fields) the type name. This makes it possible to handle + # import renaming and forbid struct fields and methods. + # Default: false + expand-expressions: true funlen: # Checks the number of lines in a function. diff --git a/go.mod b/go.mod index 84e4478593de..a30fb5402482 100644 --- a/go.mod +++ b/go.mod @@ -149,7 +149,7 @@ require ( github.com/mattn/go-isatty v0.0.17 // indirect github.com/mattn/go-runewidth v0.0.9 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect - github.com/mitchellh/mapstructure v1.5.0 // indirect + github.com/mitchellh/mapstructure v1.5.0 github.com/nbutton23/zxcvbn-go v0.0.0-20210217022336-fa2cb2858354 // indirect github.com/olekukonko/tablewriter v0.0.5 // indirect github.com/pelletier/go-toml v1.9.5 // indirect @@ -186,6 +186,6 @@ require ( golang.org/x/text v0.6.0 // indirect google.golang.org/protobuf v1.28.0 // indirect gopkg.in/ini.v1 v1.67.0 // indirect - gopkg.in/yaml.v2 v2.4.0 // indirect + gopkg.in/yaml.v2 v2.4.0 mvdan.cc/lint v0.0.0-20170908181259-adc824a0674b // indirect ) diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 76a16b87d610..6642a5db4251 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -1,9 +1,11 @@ package config import ( + "encoding" "runtime" "github.com/pkg/errors" + "gopkg.in/yaml.v2" ) var defaultLintersSettings = LintersSettings{ @@ -307,10 +309,46 @@ type ExhaustructSettings struct { } type ForbidigoSettings struct { - Forbid []string `mapstructure:"forbid"` - ExcludeGodocExamples bool `mapstructure:"exclude-godoc-examples"` + Forbid []ForbidigoPattern `mapstructure:"forbid"` + ExcludeGodocExamples bool `mapstructure:"exclude-godoc-examples"` + ExpandExpressions bool `mapstructure:"expand-expressions"` +} + +// ForbidigoPattern corresponds to forbidigo.pattern and adds +// mapstructure support. The YAML field names must match what +// forbidigo expects. +type ForbidigoPattern struct { + // patternString gets populated when the config contains a string as + // entry in ForbidigoSettings.Forbid[] because ForbidigoPattern + // implements encoding.TextUnmarshaler and the reader uses the + // mapstructure.TextUnmarshallerHookFunc as decoder hook. + // + // If the entry is a map, then the other fields are set as usual + // by mapstructure. + patternString string + + Pattern string `yaml:"p" mapstructure:"p"` + Package string `yaml:"pkg,omitempty" mapstructure:"pkg,omitempty"` + Msg string `yaml:"msg,omitempty" mapstructure:"msg,omitempty"` +} + +func (p *ForbidigoPattern) UnmarshalText(text []byte) error { + // Validation happens when instantiating forbidigo. + p.patternString = string(text) + return nil } +// String is intentionally not called MarshalText because that led to infinite +// recursion when yaml.Marshal called MarshalText. +func (p *ForbidigoPattern) String() ([]byte, error) { + if p.patternString != "" { + return []byte(p.patternString), nil + } + return yaml.Marshal(p) +} + +var _ encoding.TextUnmarshaler = &ForbidigoPattern{} + type FunlenSettings struct { Lines int Statements int diff --git a/pkg/config/reader.go b/pkg/config/reader.go index 2dfd3c06c9b0..3e2a01967851 100644 --- a/pkg/config/reader.go +++ b/pkg/config/reader.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/mitchellh/go-homedir" + "github.com/mitchellh/mapstructure" "github.com/spf13/viper" "github.com/golangci/golangci-lint/pkg/exitcodes" @@ -83,7 +84,14 @@ func (r *FileReader) parseConfig() error { } r.cfg.cfgDir = usedConfigDir - if err := viper.Unmarshal(r.cfg); err != nil { + if err := viper.Unmarshal(r.cfg, viper.DecodeHook(mapstructure.ComposeDecodeHookFunc( + // Default hooks (https://github.com/spf13/viper/blob/518241257478c557633ab36e474dfcaeb9a3c623/viper.go#L135-L138). + mapstructure.StringToTimeDurationHookFunc(), + mapstructure.StringToSliceHookFunc(","), + + // Needed for forbidigo. + mapstructure.TextUnmarshallerHookFunc(), + ))); err != nil { return fmt.Errorf("can't unmarshal config by viper: %s", err) } diff --git a/pkg/golinters/forbidigo.go b/pkg/golinters/forbidigo.go index 95fb47e47b89..cf2f113348a2 100644 --- a/pkg/golinters/forbidigo.go +++ b/pkg/golinters/forbidigo.go @@ -10,6 +10,7 @@ import ( "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -40,6 +41,12 @@ func NewForbidigo(settings *config.ForbidigoSettings) *goanalysis.Linter { }, } + loadMode := goanalysis.LoadModeSyntax + if settings != nil && settings.ExpandExpressions { + // Need more information for the analysis. + loadMode = goanalysis.LoadModeTypesInfo + } + return goanalysis.NewLinter( forbidigoName, "Forbids identifiers", @@ -47,7 +54,7 @@ func NewForbidigo(settings *config.ForbidigoSettings) *goanalysis.Linter { nil, ).WithIssuesReporter(func(*linter.Context) []goanalysis.Issue { return resIssues - }).WithLoadMode(goanalysis.LoadModeSyntax) + }).WithLoadMode(loadMode) } func runForbidigo(pass *analysis.Pass, settings *config.ForbidigoSettings) ([]goanalysis.Issue, error) { @@ -57,14 +64,30 @@ func runForbidigo(pass *analysis.Pass, settings *config.ForbidigoSettings) ([]go forbidigo.OptionIgnorePermitDirectives(true), } - forbid, err := forbidigo.NewLinter(settings.Forbid, options...) + // Convert patterns back to strings because that is what NewLinter + // accepts. + var patterns []string + for _, pattern := range settings.Forbid { + buffer, err := pattern.String() + if err != nil { + return nil, err + } + patterns = append(patterns, string(buffer)) + } + + forbid, err := forbidigo.NewLinter(patterns, options...) if err != nil { return nil, errors.Wrapf(err, "failed to create linter %q", forbidigoName) } var issues []goanalysis.Issue for _, file := range pass.Files { - hints, err := forbid.RunWithConfig(forbidigo.RunConfig{Fset: pass.Fset}, file) + config := forbidigo.RunConfig{ + Fset: pass.Fset, + TypesInfo: pass.TypesInfo, + DebugLog: logutils.Debug(logutils.DebugKeyForbidigo), + } + hints, err := forbid.RunWithConfig(config, file) if err != nil { return nil, errors.Wrapf(err, "forbidigo linter failed on file %q", file.Name.String()) } diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 6f406f7d26e3..41cce6d923aa 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -414,10 +414,17 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithLoadForGoAnalysis(). WithURL("https://github.com/kyoh86/exportloopref"), - linter.NewConfig(golinters.NewForbidigo(forbidigoCfg)). - WithSince("v1.34.0"). - WithPresets(linter.PresetStyle). - WithURL("https://github.com/ashanbrown/forbidigo"), + func() *linter.Config { + l := linter.NewConfig(golinters.NewForbidigo(forbidigoCfg)). + WithSince("v1.34.0"). + WithPresets(linter.PresetStyle). + WithURL("https://github.com/ashanbrown/forbidigo") + if forbidigoCfg != nil && forbidigoCfg.ExpandExpressions { + // Need more information for the analysis. + l = l.WithLoadForGoAnalysis() + } + return l + }(), linter.NewConfig(golinters.NewForceTypeAssert()). WithSince("v1.38.0"). diff --git a/pkg/logutils/logutils.go b/pkg/logutils/logutils.go index 62c521eac3e0..4282193ca711 100644 --- a/pkg/logutils/logutils.go +++ b/pkg/logutils/logutils.go @@ -22,6 +22,7 @@ const ( DebugKeyExcludeRules = "exclude_rules" DebugKeyExec = "exec" DebugKeyFilenameUnadjuster = "filename_unadjuster" + DebugKeyForbidigo = "forbidigo" DebugKeyGoEnv = "goenv" DebugKeyLinter = "linter" DebugKeyLintersContext = "linters_context" diff --git a/test/testdata/configs/forbidigo_struct.yml b/test/testdata/configs/forbidigo_struct.yml new file mode 100644 index 000000000000..668c7783ef83 --- /dev/null +++ b/test/testdata/configs/forbidigo_struct.yml @@ -0,0 +1,8 @@ +linters-settings: + forbidigo: + expand-expressions: true + forbid: + - p: fmt\.Print.* + pkg: ^fmt$ + - p: time.Sleep + msg: no sleeping! diff --git a/test/testdata/forbidigo_struct_config.go b/test/testdata/forbidigo_struct_config.go new file mode 100644 index 000000000000..c39ba208cee4 --- /dev/null +++ b/test/testdata/forbidigo_struct_config.go @@ -0,0 +1,13 @@ +//golangcitest:args -Eforbidigo +//golangcitest:config_path testdata/configs/forbidigo_struct.yml +package testdata + +import ( + fmt2 "fmt" + "time" +) + +func Forbidigo() { + fmt2.Printf("too noisy!!!") // want "use of `fmt2\\.Printf` forbidden by pattern `fmt\\\\.Print\\.\\*`" + time.Sleep(time.Nanosecond) // want "no sleeping!" +}