diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index a9f06c715243..f8e6cfd28d3b 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -128,6 +128,9 @@ jobs: - run: ./golangci-lint + - run: ./golangci-lint fmt + - run: ./golangci-lint fmt --diff + - run: ./golangci-lint cache - run: ./golangci-lint cache status - run: ./golangci-lint cache clean diff --git a/cmd/golangci-lint/main.go b/cmd/golangci-lint/main.go index bf235bf17fca..019a0748d2cb 100644 --- a/cmd/golangci-lint/main.go +++ b/cmd/golangci-lint/main.go @@ -64,11 +64,10 @@ func createBuildInfo() commands.BuildInfo { } } - revision = cmp.Or(revision, "unknown") - modified = cmp.Or(modified, "?") info.Date = cmp.Or(info.Date, "(unknown)") - info.Commit = fmt.Sprintf("(%s, modified: %s, mod sum: %q)", revision, modified, buildInfo.Main.Sum) + info.Commit = fmt.Sprintf("(%s, modified: %s, mod sum: %q)", + cmp.Or(revision, "unknown"), cmp.Or(modified, "?"), buildInfo.Main.Sum) return info } diff --git a/pkg/commands/flagsets.go b/pkg/commands/flagsets.go index d514f127186b..67a774315eb9 100644 --- a/pkg/commands/flagsets.go +++ b/pkg/commands/flagsets.go @@ -38,6 +38,11 @@ func setupLintersFlagSet(v *viper.Viper, fs *pflag.FlagSet) { color.GreenString("Override linters configuration section to only run the specific linter(s)")) // Flags only. } +func setupFormattersFlagSet(v *viper.Viper, fs *pflag.FlagSet) { + internal.AddFlagAndBindP(v, fs, fs.StringSliceP, "enable", "E", "formatters.enable", nil, + color.GreenString("Enable specific formatter")) +} + func setupRunFlagSet(v *viper.Viper, fs *pflag.FlagSet) { internal.AddFlagAndBindP(v, fs, fs.IntP, "concurrency", "j", "run.concurrency", getDefaultConcurrency(), color.GreenString("Number of CPUs to use (Default: number of logical CPUs)")) @@ -102,7 +107,7 @@ func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { internal.AddFlagAndBind(v, fs, fs.Bool, "exclude-dirs-use-default", "issues.exclude-dirs-use-default", true, formatList("Use or not use default excluded directories:", processors.StdExcludeDirRegexps)) - internal.AddFlagAndBind(v, fs, fs.String, "exclude-generated", "issues.exclude-generated", processors.AutogeneratedModeLax, + internal.AddFlagAndBind(v, fs, fs.String, "exclude-generated", "issues.exclude-generated", config.GeneratedModeLax, color.GreenString("Mode of the generated files analysis")) const newDesc = "Show only new issues: if there are unstaged changes or untracked files, only those changes " + diff --git a/pkg/commands/fmt.go b/pkg/commands/fmt.go new file mode 100644 index 000000000000..cb7472847f25 --- /dev/null +++ b/pkg/commands/fmt.go @@ -0,0 +1,152 @@ +package commands + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/fatih/color" + "github.com/spf13/cobra" + "github.com/spf13/viper" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/goformat" + "github.com/golangci/golangci-lint/pkg/goformatters" + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/result/processors" +) + +type fmtOptions struct { + config.LoaderOptions + + diff bool // Flag only. +} + +type fmtCommand struct { + viper *viper.Viper + cmd *cobra.Command + + opts fmtOptions + + cfg *config.Config + + buildInfo BuildInfo + + runner *goformat.Runner + + log logutils.Log + debugf logutils.DebugFunc +} + +func newFmtCommand(logger logutils.Log, info BuildInfo) *fmtCommand { + c := &fmtCommand{ + viper: viper.New(), + log: logger, + debugf: logutils.Debug(logutils.DebugKeyExec), + cfg: config.NewDefault(), + buildInfo: info, + } + + fmtCmd := &cobra.Command{ + Use: "fmt", + Short: "Format Go source files", + RunE: c.execute, + PreRunE: c.preRunE, + PersistentPreRunE: c.persistentPreRunE, + PersistentPostRun: c.persistentPostRun, + SilenceUsage: true, + } + + fmtCmd.SetOut(logutils.StdOut) // use custom output to properly color it in Windows terminals + fmtCmd.SetErr(logutils.StdErr) + + fs := fmtCmd.Flags() + fs.SortFlags = false // sort them as they are defined here + + setupConfigFileFlagSet(fs, &c.opts.LoaderOptions) + + setupFormattersFlagSet(c.viper, fs) + + fs.BoolVarP(&c.opts.diff, "diff", "d", false, color.GreenString("Display diffs instead of rewriting files")) + + c.cmd = fmtCmd + + return c +} + +func (c *fmtCommand) persistentPreRunE(cmd *cobra.Command, args []string) error { + c.log.Infof("%s", c.buildInfo.String()) + + loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, cmd.Flags(), c.opts.LoaderOptions, c.cfg, args) + + err := loader.Load(config.LoadOptions{CheckDeprecation: true, Validation: true}) + if err != nil { + return fmt.Errorf("can't load config: %w", err) + } + + return nil +} + +func (c *fmtCommand) preRunE(_ *cobra.Command, _ []string) error { + metaFormatter, err := goformatters.NewMetaFormatter(c.log, &c.cfg.Formatters, &c.cfg.Run) + if err != nil { + return fmt.Errorf("failed to create meta-formatter: %w", err) + } + + matcher := processors.NewGeneratedFileMatcher(c.cfg.Formatters.Exclusions.Generated) + + opts, err := goformat.NewRunnerOptions(c.cfg, c.opts.diff) + if err != nil { + return fmt.Errorf("build walk options: %w", err) + } + + c.runner = goformat.NewRunner(c.log, metaFormatter, matcher, opts) + + return nil +} + +func (c *fmtCommand) execute(_ *cobra.Command, args []string) error { + paths, err := cleanArgs(args) + if err != nil { + return fmt.Errorf("failed to clean arguments: %w", err) + } + + c.log.Infof("Formatting Go files...") + + err = c.runner.Run(paths) + if err != nil { + return fmt.Errorf("failed to process files: %w", err) + } + + return nil +} + +func (c *fmtCommand) persistentPostRun(_ *cobra.Command, _ []string) { + if c.runner.ExitCode() != 0 { + os.Exit(c.runner.ExitCode()) + } +} + +func cleanArgs(args []string) ([]string, error) { + if len(args) == 0 { + abs, err := filepath.Abs(".") + if err != nil { + return nil, err + } + + return []string{abs}, nil + } + + var expanded []string + for _, arg := range args { + abs, err := filepath.Abs(strings.ReplaceAll(arg, "...", "")) + if err != nil { + return nil, err + } + + expanded = append(expanded, abs) + } + + return expanded, nil +} diff --git a/pkg/commands/root.go b/pkg/commands/root.go index cbb838aac2bc..82b224446163 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -60,6 +60,7 @@ func newRootCommand(info BuildInfo) *rootCommand { rootCmd.AddCommand( newLintersCommand(log).cmd, newRunCommand(log, info).cmd, + newFmtCommand(log, info).cmd, newCacheCommand().cmd, newConfigCommand(log, info).cmd, newVersionCommand(info).cmd, diff --git a/pkg/config/config.go b/pkg/config/config.go index 4ddd896170d9..10327d5f31b1 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -29,6 +29,8 @@ type Config struct { Issues Issues `mapstructure:"issues"` Severity Severity `mapstructure:"severity"` + Formatters Formatters `mapstructure:"formatters"` + InternalCmdTest bool // Option is used only for testing golangci-lint command, don't use it InternalTest bool // Option is used only for testing golangci-lint code, don't use it } @@ -64,6 +66,9 @@ func (c *Config) Validate() error { func NewDefault() *Config { return &Config{ LintersSettings: defaultLintersSettings, + Formatters: Formatters{ + Settings: defaultFormatterSettings, + }, } } diff --git a/pkg/config/formatters.go b/pkg/config/formatters.go new file mode 100644 index 000000000000..054ea54d6d05 --- /dev/null +++ b/pkg/config/formatters.go @@ -0,0 +1,12 @@ +package config + +type Formatters struct { + Enable []string `mapstructure:"enable"` + Settings FormatterSettings `mapstructure:"settings"` + Exclusions FormatterExclusions `mapstructure:"exclusions"` +} + +type FormatterExclusions struct { + Generated string `mapstructure:"generated"` + Paths []string `mapstructure:"paths"` +} diff --git a/pkg/config/formatters_settings.go b/pkg/config/formatters_settings.go new file mode 100644 index 000000000000..373eb0dba90c --- /dev/null +++ b/pkg/config/formatters_settings.go @@ -0,0 +1,52 @@ +package config + +var defaultFormatterSettings = FormatterSettings{ + GoFmt: GoFmtSettings{ + Simplify: true, + }, + Gci: GciSettings{ + Sections: []string{"standard", "default"}, + SkipGenerated: true, + }, +} + +type FormatterSettings struct { + Gci GciSettings `mapstructure:"gci"` + GoFmt GoFmtSettings `mapstructure:"gofmt"` + GoFumpt GoFumptSettings `mapstructure:"gofumpt"` + GoImports GoImportsSettings `mapstructure:"goimports"` +} + +type GciSettings struct { + Sections []string `mapstructure:"sections"` + NoInlineComments bool `mapstructure:"no-inline-comments"` + NoPrefixComments bool `mapstructure:"no-prefix-comments"` + SkipGenerated bool `mapstructure:"skip-generated"` + CustomOrder bool `mapstructure:"custom-order"` + NoLexOrder bool `mapstructure:"no-lex-order"` + + // Deprecated: use Sections instead. + LocalPrefixes string `mapstructure:"local-prefixes"` +} + +type GoFmtSettings struct { + Simplify bool + RewriteRules []GoFmtRewriteRule `mapstructure:"rewrite-rules"` +} + +type GoFmtRewriteRule struct { + Pattern string + Replacement string +} + +type GoFumptSettings struct { + ModulePath string `mapstructure:"module-path"` + ExtraRules bool `mapstructure:"extra-rules"` + + // Deprecated: use the global `run.go` instead. + LangVersion string `mapstructure:"lang-version"` +} + +type GoImportsSettings struct { + LocalPrefixes string `mapstructure:"local-prefixes"` +} diff --git a/pkg/config/linters_exclusions.go b/pkg/config/linters_exclusions.go index 3bed6dfc1535..a07c5dc25ebf 100644 --- a/pkg/config/linters_exclusions.go +++ b/pkg/config/linters_exclusions.go @@ -5,6 +5,12 @@ import ( "slices" ) +const ( + GeneratedModeLax = "lax" + GeneratedModeStrict = "strict" + GeneratedModeDisable = "disable" +) + const ( ExclusionPresetComments = "comments" ExclusionPresetStdErrorHandling = "stdErrorHandling" diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 94650a66de9b..575580538ad5 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -10,6 +10,8 @@ import ( ) var defaultLintersSettings = LintersSettings{ + FormatterSettings: defaultFormatterSettings, + Asasalint: AsasalintSettings{ UseBuiltinExclusions: true, }, @@ -43,10 +45,6 @@ var defaultLintersSettings = LintersSettings{ Forbidigo: ForbidigoSettings{ ExcludeGodocExamples: true, }, - Gci: GciSettings{ - Sections: []string{"standard", "default"}, - SkipGenerated: true, - }, GoChecksumType: GoChecksumTypeSettings{ DefaultSignifiesExhaustive: true, }, @@ -74,14 +72,6 @@ var defaultLintersSettings = LintersSettings{ Scope: "declarations", Period: true, }, - Gofmt: GoFmtSettings{ - Simplify: true, - }, - Gofumpt: GofumptSettings{ - LangVersion: "", - ModulePath: "", - ExtraRules: false, - }, Gosec: GoSecSettings{ Concurrency: runtime.NumCPU(), }, @@ -214,6 +204,8 @@ var defaultLintersSettings = LintersSettings{ } type LintersSettings struct { + FormatterSettings `mapstructure:",squash"` + Asasalint AsasalintSettings BiDiChk BiDiChkSettings CopyLoopVar CopyLoopVarSettings @@ -231,7 +223,6 @@ type LintersSettings struct { Fatcontext FatcontextSettings Forbidigo ForbidigoSettings Funlen FunlenSettings - Gci GciSettings GinkgoLinter GinkgoLinterSettings Gocognit GocognitSettings GoChecksumType GoChecksumTypeSettings @@ -240,10 +231,7 @@ type LintersSettings struct { Gocyclo GoCycloSettings Godot GodotSettings Godox GodoxSettings - Gofmt GoFmtSettings - Gofumpt GofumptSettings Goheader GoHeaderSettings - Goimports GoImportsSettings GoModDirectives GoModDirectivesSettings Gomodguard GoModGuardSettings Gosec GoSecSettings @@ -488,18 +476,6 @@ type FunlenSettings struct { IgnoreComments bool `mapstructure:"ignore-comments"` } -type GciSettings struct { - Sections []string `mapstructure:"sections"` - NoInlineComments bool `mapstructure:"no-inline-comments"` - NoPrefixComments bool `mapstructure:"no-prefix-comments"` - SkipGenerated bool `mapstructure:"skip-generated"` - CustomOrder bool `mapstructure:"custom-order"` - NoLexOrder bool `mapstructure:"no-lex-order"` - - // Deprecated: use Sections instead. - LocalPrefixes string `mapstructure:"local-prefixes"` -} - type GinkgoLinterSettings struct { SuppressLenAssertion bool `mapstructure:"suppress-len-assertion"` SuppressNilAssertion bool `mapstructure:"suppress-nil-assertion"` @@ -567,34 +543,12 @@ type GodoxSettings struct { Keywords []string } -type GoFmtSettings struct { - Simplify bool - RewriteRules []GoFmtRewriteRule `mapstructure:"rewrite-rules"` -} - -type GoFmtRewriteRule struct { - Pattern string - Replacement string -} - -type GofumptSettings struct { - ModulePath string `mapstructure:"module-path"` - ExtraRules bool `mapstructure:"extra-rules"` - - // Deprecated: use the global `run.go` instead. - LangVersion string `mapstructure:"lang-version"` -} - type GoHeaderSettings struct { Values map[string]map[string]string `mapstructure:"values"` Template string `mapstructure:"template"` TemplatePath string `mapstructure:"template-path"` } -type GoImportsSettings struct { - LocalPrefixes string `mapstructure:"local-prefixes"` -} - type GoModDirectivesSettings struct { ReplaceAllowList []string `mapstructure:"replace-allow-list"` ReplaceLocal bool `mapstructure:"replace-local"` diff --git a/pkg/config/loader.go b/pkg/config/loader.go index dc9ceeadd1fd..7079d90c1990 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -70,7 +70,7 @@ func (l *Loader) Load(opts LoadOptions) error { if l.cfg.Linters.LinterExclusions.Generated == "" { // `l.cfg.Issues.ExcludeGenerated` is always non-empty because of the flag default value. - l.cfg.Linters.LinterExclusions.Generated = cmp.Or(l.cfg.Issues.ExcludeGenerated, "strict") + l.cfg.Linters.LinterExclusions.Generated = cmp.Or(l.cfg.Issues.ExcludeGenerated, GeneratedModeStrict) } // Compatibility layer with v1. @@ -88,11 +88,15 @@ func (l *Loader) Load(opts LoadOptions) error { l.cfg.Linters.LinterExclusions.Rules = append(l.cfg.Linters.LinterExclusions.Rules, l.cfg.Issues.ExcludeRules...) } + l.handleFormatters() + if opts.CheckDeprecation { err = l.handleDeprecation() if err != nil { return err } + + l.handleFormatterDeprecations() } l.handleGoVersion() @@ -319,7 +323,8 @@ func (l *Loader) handleGoVersion() { l.cfg.LintersSettings.ParallelTest.Go = l.cfg.Run.Go - l.cfg.LintersSettings.Gofumpt.LangVersion = cmp.Or(l.cfg.LintersSettings.Gofumpt.LangVersion, l.cfg.Run.Go) + l.cfg.LintersSettings.GoFumpt.LangVersion = cmp.Or(l.cfg.LintersSettings.GoFumpt.LangVersion, l.cfg.Run.Go) + l.cfg.Formatters.Settings.GoFumpt.LangVersion = cmp.Or(l.cfg.Formatters.Settings.GoFumpt.LangVersion, l.cfg.Run.Go) trimmedGoVersion := goutil.TrimGoVersion(l.cfg.Run.Go) @@ -433,7 +438,7 @@ func (l *Loader) handleLinterOptionDeprecations() { } // Deprecated since v1.47.0 - if l.cfg.LintersSettings.Gofumpt.LangVersion != "" { + if l.cfg.LintersSettings.GoFumpt.LangVersion != "" { l.log.Warnf("The configuration option `linters.gofumpt.lang-version` is deprecated, please use global `run.go`.") } @@ -495,6 +500,59 @@ func (l *Loader) handleEnableOnlyOption() error { return nil } +func (l *Loader) handleFormatters() { + l.handleFormatterOverrides() + l.handleFormatterExclusions() +} + +// Overrides linter settings with formatter settings if the formatter is enabled. +func (l *Loader) handleFormatterOverrides() { + if slices.Contains(l.cfg.Formatters.Enable, "gofmt") { + l.cfg.LintersSettings.GoFmt = l.cfg.Formatters.Settings.GoFmt + } + + if slices.Contains(l.cfg.Formatters.Enable, "gofumpt") { + l.cfg.LintersSettings.GoFumpt = l.cfg.Formatters.Settings.GoFumpt + } + + if slices.Contains(l.cfg.Formatters.Enable, "goimports") { + l.cfg.LintersSettings.GoImports = l.cfg.Formatters.Settings.GoImports + } + + if slices.Contains(l.cfg.Formatters.Enable, "gci") { + l.cfg.LintersSettings.Gci = l.cfg.Formatters.Settings.Gci + } +} + +// Add formatter exclusions to linters exclusions. +func (l *Loader) handleFormatterExclusions() { + if len(l.cfg.Formatters.Enable) == 0 { + return + } + + for _, path := range l.cfg.Formatters.Exclusions.Paths { + l.cfg.Linters.LinterExclusions.Rules = append(l.cfg.Linters.LinterExclusions.Rules, ExcludeRule{ + BaseRule: BaseRule{ + Linters: l.cfg.Formatters.Enable, + Path: path, + }, + }) + } +} +func (l *Loader) handleFormatterDeprecations() { + // Deprecated since v1.44.0. + if l.cfg.Formatters.Settings.Gci.LocalPrefixes != "" { + l.log.Warnf("The configuration option `formatters.settings.gci.local-prefixes` is deprecated, " + + "please use `prefix()` inside `formatters.settings.gci.sections`.") + } + + // Deprecated since v1.47.0 + if l.cfg.Formatters.Settings.GoFumpt.LangVersion != "" { + l.log.Warnf("The configuration option `formatters.settings.gofumpt.lang-version` is deprecated, " + + "please use global `run.go`.") + } +} + func customDecoderHook() viper.DecoderConfigOption { return viper.DecodeHook(mapstructure.ComposeDecodeHookFunc( // Default hooks (https://github.com/spf13/viper/blob/518241257478c557633ab36e474dfcaeb9a3c623/viper.go#L135-L138). diff --git a/pkg/goformat/runner.go b/pkg/goformat/runner.go new file mode 100644 index 000000000000..9b2c1b41b0cb --- /dev/null +++ b/pkg/goformat/runner.go @@ -0,0 +1,207 @@ +package goformat + +import ( + "bytes" + "context" + "fmt" + "io" + "io/fs" + "log" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/rogpeppe/go-internal/diff" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/goformatters" + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/result/processors" +) + +type Runner struct { + log logutils.Log + + metaFormatter *goformatters.MetaFormatter + matcher *processors.GeneratedFileMatcher + + opts RunnerOptions + + exitCode int +} + +func NewRunner(log logutils.Log, + metaFormatter *goformatters.MetaFormatter, matcher *processors.GeneratedFileMatcher, + opts RunnerOptions) *Runner { + return &Runner{ + log: log, + matcher: matcher, + metaFormatter: metaFormatter, + opts: opts, + } +} + +func (c *Runner) Run(paths []string) error { + savedStdout, savedStderr := os.Stdout, os.Stderr + + if !logutils.HaveDebugTag(logutils.DebugKeyFormattersOutput) { + // Don't allow linters and loader to print anything + log.SetOutput(io.Discard) + c.setOutputToDevNull() + defer func() { + os.Stdout, os.Stderr = savedStdout, savedStderr + }() + } + + for _, path := range paths { + err := c.walk(path, savedStdout) + if err != nil { + return err + } + } + + return nil +} + +func (c *Runner) walk(root string, stdout *os.File) error { + return filepath.Walk(root, func(path string, f fs.FileInfo, err error) error { + if err != nil { + return err + } + + if f.IsDir() && skipDir(f.Name()) { + return fs.SkipDir + } + + if !isGoFile(f) { + return nil + } + + match, err := c.opts.MatchAnyPattern(path) + if err != nil || match { + return err + } + + input, err := os.ReadFile(path) + if err != nil { + return err + } + + match, err = c.matcher.IsGeneratedFile(path, input) + if err != nil || match { + return err + } + + output := c.metaFormatter.Format(path, input) + + if bytes.Equal(input, output) { + return nil + } + + if c.opts.diff { + newName := filepath.ToSlash(path) + oldName := newName + ".orig" + _, err = stdout.Write(diff.Diff(oldName, input, newName, output)) + if err != nil { + return err + } + + c.exitCode = 1 + + return nil + } + + c.log.Infof("format: %s", path) + + // On Windows, we need to re-set the permissions from the file. See golang/go#38225. + var perms os.FileMode + if fi, err := os.Stat(path); err == nil { + perms = fi.Mode() & os.ModePerm + } + + return os.WriteFile(path, output, perms) + }) +} + +func (c *Runner) setOutputToDevNull() { + devNull, err := os.Open(os.DevNull) + if err != nil { + c.log.Warnf("Can't open null device %q: %s", os.DevNull, err) + return + } + + os.Stdout, os.Stderr = devNull, devNull +} + +func (c *Runner) ExitCode() int { + return c.exitCode +} + +type RunnerOptions struct { + basePath string + patterns []*regexp.Regexp + generated string + diff bool +} + +func NewRunnerOptions(cfg *config.Config, diff bool) (RunnerOptions, error) { + basePath, err := fsutils.GetBasePath(context.Background(), cfg.Run.RelativePathMode, cfg.GetConfigDir()) + if err != nil { + return RunnerOptions{}, fmt.Errorf("get base path: %w", err) + } + + opts := RunnerOptions{ + basePath: basePath, + generated: cfg.Formatters.Exclusions.Generated, + diff: diff, + } + + for _, pattern := range cfg.Formatters.Exclusions.Paths { + exp, err := regexp.Compile(fsutils.NormalizePathInRegex(pattern)) + if err != nil { + return RunnerOptions{}, fmt.Errorf("compile path pattern %q: %w", pattern, err) + } + + opts.patterns = append(opts.patterns, exp) + } + + return opts, nil +} + +func (o RunnerOptions) MatchAnyPattern(path string) (bool, error) { + if len(o.patterns) == 0 { + return false, nil + } + + rel, err := filepath.Rel(o.basePath, path) + if err != nil { + return false, err + } + + for _, pattern := range o.patterns { + if pattern.MatchString(rel) { + return true, nil + } + } + + return false, nil +} + +func skipDir(name string) bool { + switch name { + case "vendor", "testdata", "node_modules": + return true + + case "third_party", "builtin": // For compatibility with `exclude-dirs-use-default`. + return true + + default: + return strings.HasPrefix(name, ".") + } +} + +func isGoFile(f fs.FileInfo) bool { + return !f.IsDir() && !strings.HasPrefix(f.Name(), ".") && strings.HasSuffix(f.Name(), ".go") +} diff --git a/pkg/goformatters/gofumpt/gofumpt.go b/pkg/goformatters/gofumpt/gofumpt.go index 7c548a2afde8..cfd7645e4526 100644 --- a/pkg/goformatters/gofumpt/gofumpt.go +++ b/pkg/goformatters/gofumpt/gofumpt.go @@ -14,7 +14,7 @@ type Formatter struct { options gofumpt.Options } -func New(settings *config.GofumptSettings, goVersion string) *Formatter { +func New(settings *config.GoFumptSettings, goVersion string) *Formatter { var options gofumpt.Options if settings != nil { diff --git a/pkg/goformatters/meta_formatter.go b/pkg/goformatters/meta_formatter.go index d66878c7abe5..0853c79ee2fa 100644 --- a/pkg/goformatters/meta_formatter.go +++ b/pkg/goformatters/meta_formatter.go @@ -4,13 +4,13 @@ import ( "bytes" "fmt" "go/format" + "slices" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/goformatters/gci" "github.com/golangci/golangci-lint/pkg/goformatters/gofmt" "github.com/golangci/golangci-lint/pkg/goformatters/gofumpt" "github.com/golangci/golangci-lint/pkg/goformatters/goimports" - "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/logutils" ) @@ -19,24 +19,30 @@ type MetaFormatter struct { formatters []Formatter } -func NewMetaFormatter(log logutils.Log, cfg *config.Config, enabledLinters map[string]*linter.Config) (*MetaFormatter, error) { +func NewMetaFormatter(log logutils.Log, cfg *config.Formatters, runCfg *config.Run) (*MetaFormatter, error) { + for _, formatter := range cfg.Enable { + if !IsFormatter(formatter) { + return nil, fmt.Errorf("invalid formatter %q", formatter) + } + } + m := &MetaFormatter{log: log} - if _, ok := enabledLinters[gofmt.Name]; ok { - m.formatters = append(m.formatters, gofmt.New(&cfg.LintersSettings.Gofmt)) + if slices.Contains(cfg.Enable, gofmt.Name) { + m.formatters = append(m.formatters, gofmt.New(&cfg.Settings.GoFmt)) } - if _, ok := enabledLinters[gofumpt.Name]; ok { - m.formatters = append(m.formatters, gofumpt.New(&cfg.LintersSettings.Gofumpt, cfg.Run.Go)) + if slices.Contains(cfg.Enable, gofumpt.Name) { + m.formatters = append(m.formatters, gofumpt.New(&cfg.Settings.GoFumpt, runCfg.Go)) } - if _, ok := enabledLinters[goimports.Name]; ok { - m.formatters = append(m.formatters, goimports.New(&cfg.LintersSettings.Goimports)) + if slices.Contains(cfg.Enable, goimports.Name) { + m.formatters = append(m.formatters, goimports.New(&cfg.Settings.GoImports)) } // gci is a last because the only goal of gci is to handle imports. - if _, ok := enabledLinters[gci.Name]; ok { - formatter, err := gci.New(&cfg.LintersSettings.Gci) + if slices.Contains(cfg.Enable, gci.Name) { + formatter, err := gci.New(&cfg.Settings.Gci) if err != nil { return nil, fmt.Errorf("gci: creating formatter: %w", err) } @@ -72,3 +78,7 @@ func (m *MetaFormatter) Format(filename string, src []byte) []byte { return data } + +func IsFormatter(name string) bool { + return slices.Contains([]string{gofmt.Name, gofumpt.Name, goimports.Name, gci.Name}, name) +} diff --git a/pkg/golinters/gofumpt/gofumpt.go b/pkg/golinters/gofumpt/gofumpt.go index 878a5c79b0aa..c6fdb81b331b 100644 --- a/pkg/golinters/gofumpt/gofumpt.go +++ b/pkg/golinters/gofumpt/gofumpt.go @@ -12,7 +12,7 @@ import ( const linterName = "gofumpt" -func New(settings *config.GofumptSettings) *goanalysis.Linter { +func New(settings *config.GoFumptSettings) *goanalysis.Linter { a := goformatters.NewAnalyzer( internal.LinterLogger.Child(linterName), "Checks if code and import statements are formatted, with additional rules.", diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index 7fa63e526676..066554df1a77 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -394,13 +394,13 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { WithAutoFix(). WithURL("https://github.com/Djarvur/go-err113"), - linter.NewConfig(gofmt.New(&cfg.LintersSettings.Gofmt)). + linter.NewConfig(gofmt.New(&cfg.LintersSettings.GoFmt)). WithSince("v1.0.0"). WithPresets(linter.PresetFormatting). WithAutoFix(). WithURL("https://pkg.go.dev/cmd/gofmt"), - linter.NewConfig(gofumpt.New(&cfg.LintersSettings.Gofumpt)). + linter.NewConfig(gofumpt.New(&cfg.LintersSettings.GoFumpt)). WithSince("v1.28.0"). WithPresets(linter.PresetFormatting). WithAutoFix(). @@ -412,7 +412,7 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { WithAutoFix(). WithURL("https://github.com/denis-tingaikin/go-header"), - linter.NewConfig(goimports.New(&cfg.LintersSettings.Goimports)). + linter.NewConfig(goimports.New(&cfg.LintersSettings.GoImports)). WithSince("v1.20.0"). WithPresets(linter.PresetFormatting, linter.PresetImport). WithAutoFix(). diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 4fe57a3b4857..c20aad409641 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -178,7 +178,7 @@ func (m *Manager) build(enabledByDefaultLinters []*linter.Config) map[string]*li } } - for _, name := range m.cfg.Linters.Enable { + for _, name := range slices.Concat(m.cfg.Linters.Enable, m.cfg.Formatters.Enable) { for _, lc := range m.GetLinterConfigs(name) { // it's important to use lc.Name() nor name because name can be alias resultLintersSet[lc.Name()] = lc diff --git a/pkg/lint/lintersdb/manager_test.go b/pkg/lint/lintersdb/manager_test.go index b3b6892443ea..4fec48d90246 100644 --- a/pkg/lint/lintersdb/manager_test.go +++ b/pkg/lint/lintersdb/manager_test.go @@ -65,96 +65,144 @@ func TestManager_GetOptimizedLinters(t *testing.T) { } func TestManager_build(t *testing.T) { - type cs struct { - cfg config.Linters - name string // test case name - def []string // enabled by default linters - exp []string // alphabetically ordered enabled linter names - } - allMegacheckLinterNames := []string{"gosimple", "staticcheck", "unused"} - cases := []cs{ + testCases := []struct { + desc string + cfg *config.Config + defaultSet []string // enabled by default linters + expected []string // alphabetically ordered enabled linter names + }{ + { + desc: "disable all linters from megacheck", + cfg: &config.Config{ + Linters: config.Linters{ + Disable: []string{"megacheck"}, + }, + }, + defaultSet: allMegacheckLinterNames, + expected: []string{"typecheck"}, // all disabled + }, { - cfg: config.Linters{ - Disable: []string{"megacheck"}, + desc: "disable only staticcheck", + cfg: &config.Config{ + Linters: config.Linters{ + Disable: []string{"staticcheck"}, + }, }, - name: "disable all linters from megacheck", - def: allMegacheckLinterNames, - exp: []string{"typecheck"}, // all disabled + defaultSet: allMegacheckLinterNames, + expected: []string{"gosimple", "typecheck", "unused"}, + }, + { + desc: "don't merge into megacheck", + defaultSet: allMegacheckLinterNames, + expected: []string{"gosimple", "staticcheck", "typecheck", "unused"}, }, { - cfg: config.Linters{ - Disable: []string{"staticcheck"}, + desc: "expand megacheck", + cfg: &config.Config{ + Linters: config.Linters{ + Enable: []string{"megacheck"}, + }, }, - name: "disable only staticcheck", - def: allMegacheckLinterNames, - exp: []string{"gosimple", "typecheck", "unused"}, + defaultSet: nil, + expected: []string{"gosimple", "staticcheck", "typecheck", "unused"}, + }, + { + desc: "don't disable anything", + defaultSet: []string{"gofmt", "govet", "typecheck"}, + expected: []string{"gofmt", "govet", "typecheck"}, }, { - name: "don't merge into megacheck", - def: allMegacheckLinterNames, - exp: []string{"gosimple", "staticcheck", "typecheck", "unused"}, + desc: "enable gosec by gas alias", + cfg: &config.Config{ + Linters: config.Linters{ + Enable: []string{"gas"}, + }, + }, + expected: []string{"gosec", "typecheck"}, }, { - name: "expand megacheck", - cfg: config.Linters{ - Enable: []string{"megacheck"}, + desc: "enable gosec by primary name", + cfg: &config.Config{ + Linters: config.Linters{ + Enable: []string{"gosec"}, + }, }, - def: nil, - exp: []string{"gosimple", "staticcheck", "typecheck", "unused"}, + expected: []string{"gosec", "typecheck"}, }, { - name: "don't disable anything", - def: []string{"gofmt", "govet", "typecheck"}, - exp: []string{"gofmt", "govet", "typecheck"}, + desc: "enable gosec by both names", + cfg: &config.Config{ + Linters: config.Linters{ + Enable: []string{"gosec", "gas"}, + }, + }, + expected: []string{"gosec", "typecheck"}, }, { - name: "enable gosec by gas alias", - cfg: config.Linters{ - Enable: []string{"gas"}, + desc: "disable gosec by gas alias", + cfg: &config.Config{ + Linters: config.Linters{ + Disable: []string{"gas"}, + }, }, - exp: []string{"gosec", "typecheck"}, + defaultSet: []string{"gosec"}, + expected: []string{"typecheck"}, }, { - name: "enable gosec by primary name", - cfg: config.Linters{ - Enable: []string{"gosec"}, + desc: "disable gosec by primary name", + cfg: &config.Config{ + Linters: config.Linters{ + Disable: []string{"gosec"}, + }, }, - exp: []string{"gosec", "typecheck"}, + defaultSet: []string{"gosec"}, + expected: []string{"typecheck"}, }, { - name: "enable gosec by both names", - cfg: config.Linters{ - Enable: []string{"gosec", "gas"}, + desc: "linters and formatters", + cfg: &config.Config{ + Linters: config.Linters{ + Enable: []string{"gosec"}, + }, + Formatters: config.Formatters{ + Enable: []string{"gofmt"}, + }, }, - exp: []string{"gosec", "typecheck"}, + expected: []string{"gosec", "gofmt", "typecheck"}, }, { - name: "disable gosec by gas alias", - cfg: config.Linters{ - Disable: []string{"gas"}, + desc: "linters and formatters but linters configuration disables the formatter", + cfg: &config.Config{ + Linters: config.Linters{ + Enable: []string{"gosec"}, + Disable: []string{"gofmt"}, + }, + Formatters: config.Formatters{ + Enable: []string{"gofmt"}, + }, }, - def: []string{"gosec"}, - exp: []string{"typecheck"}, + expected: []string{"gosec", "typecheck"}, }, { - name: "disable gosec by primary name", - cfg: config.Linters{ - Disable: []string{"gosec"}, + desc: "only formatters", + cfg: &config.Config{ + Formatters: config.Formatters{ + Enable: []string{"gofmt"}, + }, }, - def: []string{"gosec"}, - exp: []string{"typecheck"}, + expected: []string{"gofmt", "typecheck"}, }, } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - m, err := NewManager(logutils.NewStderrLog("skip"), &config.Config{Linters: c.cfg}, NewLinterBuilder()) + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + m, err := NewManager(logutils.NewStderrLog("skip"), test.cfg, NewLinterBuilder()) require.NoError(t, err) var defaultLinters []*linter.Config - for _, ln := range c.def { + for _, ln := range test.defaultSet { lcs := m.GetLinterConfigs(ln) assert.NotNil(t, lcs, ln) defaultLinters = append(defaultLinters, lcs...) @@ -167,7 +215,7 @@ func TestManager_build(t *testing.T) { enabledLinters = append(enabledLinters, ln) } - assert.ElementsMatch(t, c.exp, enabledLinters) + assert.ElementsMatch(t, test.expected, enabledLinters) }) } } diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 4e6d4692b565..a44d55122a9f 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "maps" "runtime/debug" "strings" @@ -71,7 +72,19 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti return nil, fmt.Errorf("failed to get enabled linters: %w", err) } - metaFormatter, err := goformatters.NewMetaFormatter(log, cfg, enabledLinters) + var enabledFormatters []string + for name := range maps.Keys(enabledLinters) { + if goformatters.IsFormatter(name) { + enabledFormatters = append(enabledFormatters, name) + } + } + + formattersCfg := &config.Formatters{ + Enable: enabledFormatters, + Settings: cfg.LintersSettings.FormatterSettings, + } + + metaFormatter, err := goformatters.NewMetaFormatter(log, formattersCfg, &cfg.Run) if err != nil { return nil, fmt.Errorf("failed to create meta-formatter: %w", err) } diff --git a/pkg/logutils/logutils.go b/pkg/logutils/logutils.go index 0454d7927162..2fbb4a7b2493 100644 --- a/pkg/logutils/logutils.go +++ b/pkg/logutils/logutils.go @@ -16,22 +16,22 @@ const EnvTestRun = "GL_TEST_RUN" const envDebug = "GL_DEBUG" const ( - DebugKeyBinSalt = "bin_salt" - DebugKeyConfigReader = "config_reader" - DebugKeyEmpty = "" - DebugKeyEnabledLinters = "enabled_linters" - DebugKeyExec = "exec" - DebugKeyFormatter = "formatter" - DebugKeyGoEnv = "goenv" - DebugKeyLinter = "linter" - DebugKeyLintersContext = "linters_context" - DebugKeyLintersDB = "lintersdb" - DebugKeyLintersOutput = "linters_output" - DebugKeyLoader = "loader" // Debugs packages loading (including `go/packages` internal debugging). - DebugKeyPkgCache = "pkgcache" - DebugKeyRunner = "runner" - DebugKeyStopwatch = "stopwatch" - DebugKeyTest = "test" + DebugKeyBinSalt = "bin_salt" + DebugKeyConfigReader = "config_reader" + DebugKeyEmpty = "" + DebugKeyEnabledLinters = "enabled_linters" + DebugKeyExec = "exec" + DebugKeyFormatter = "formatter" + DebugKeyFormattersOutput = "formatters_output" + DebugKeyGoEnv = "goenv" + DebugKeyLintersContext = "linters_context" + DebugKeyLintersDB = "lintersdb" + DebugKeyLintersOutput = "linters_output" + DebugKeyLoader = "loader" // Debugs packages loading (including `go/packages` internal debugging). + DebugKeyPkgCache = "pkgcache" + DebugKeyRunner = "runner" + DebugKeyStopwatch = "stopwatch" + DebugKeyTest = "test" ) // Printers. @@ -81,7 +81,8 @@ const ( DebugKeyForbidigo = "forbidigo" // Debugs `forbidigo` linter. DebugKeyGoCritic = "gocritic" // Debugs `gocritic` linter. DebugKeyGovet = "govet" // Debugs `govet` linter. - DebugKeyRevive = "revive" // Debugs `revive` linter. + DebugKeyLinter = "linter" + DebugKeyRevive = "revive" // Debugs `revive` linter. ) func getEnabledDebugs() map[string]bool { diff --git a/pkg/result/processors/exclusion_generated_file_filter.go b/pkg/result/processors/exclusion_generated_file_filter.go index ce4e2e214651..14a3640e951c 100644 --- a/pkg/result/processors/exclusion_generated_file_filter.go +++ b/pkg/result/processors/exclusion_generated_file_filter.go @@ -2,37 +2,13 @@ package processors import ( "fmt" - "go/parser" - "go/token" "path/filepath" - "regexp" - "strings" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) -const ( - AutogeneratedModeLax = "lax" - AutogeneratedModeStrict = "strict" - AutogeneratedModeDisable = "disable" -) - -// The values must be in lowercase. -const ( - genCodeGenerated = "code generated" - genDoNotEdit = "do not edit" - - // Related to easyjson. - genAutoFile = "autogenerated file" - - //nolint:lll // Long URL - // Related to Swagger Codegen. - // https://github.com/swagger-api/swagger-codegen/blob/61cfeac3b9d855b4eb8bffa0d118bece117bcb7d/modules/swagger-codegen/src/main/resources/go/partial_header.mustache#L16 - // https://github.com/swagger-api/swagger-codegen/issues/12358 - genSwaggerCodegen = "* generated by: swagger codegen " -) - var _ Processor = (*GeneratedFileFilter)(nil) type fileSummary struct { @@ -40,23 +16,22 @@ type fileSummary struct { } // GeneratedFileFilter filters generated files. -// - mode "lax": see `isGeneratedFileLax` documentation. -// - mode "strict": see `isGeneratedFileStrict` documentation. -// - mode "disable": skips this processor. type GeneratedFileFilter struct { debugf logutils.DebugFunc - mode string - strictPattern *regexp.Regexp + mode string + matcher *GeneratedFileMatcher fileSummaryCache map[string]*fileSummary } func NewGeneratedFileFilter(mode string) *GeneratedFileFilter { return &GeneratedFileFilter{ - debugf: logutils.Debug(logutils.DebugKeyGeneratedFileFilter), - mode: mode, - strictPattern: regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`), + debugf: logutils.Debug(logutils.DebugKeyGeneratedFileFilter), + + mode: mode, + matcher: NewGeneratedFileMatcher(mode), + fileSummaryCache: map[string]*fileSummary{}, } } @@ -66,7 +41,7 @@ func (*GeneratedFileFilter) Name() string { } func (p *GeneratedFileFilter) Process(issues []result.Issue) ([]result.Issue, error) { - if p.mode == AutogeneratedModeDisable { + if p.mode == config.GeneratedModeDisable { return issues, nil } @@ -89,19 +64,10 @@ func (p *GeneratedFileFilter) shouldPassIssue(issue *result.Issue) (bool, error) fs = &fileSummary{} p.fileSummaryCache[issue.FilePath()] = fs - if p.mode == AutogeneratedModeStrict { - var err error - fs.generated, err = p.isGeneratedFileStrict(issue.FilePath()) - if err != nil { - return false, fmt.Errorf("failed to get doc (strict) of file %s: %w", issue.FilePath(), err) - } - } else { - doc, err := getComments(issue.FilePath()) - if err != nil { - return false, fmt.Errorf("failed to get doc (lax) of file %s: %w", issue.FilePath(), err) - } - - fs.generated = p.isGeneratedFileLax(doc) + var err error + fs.generated, err = p.matcher.IsGeneratedFile(issue.FilePath(), nil) + if err != nil { + return false, fmt.Errorf("failed to get doc (%s) of file %s: %w", p.mode, issue.FilePath(), err) } p.debugf("file %q is generated: %t", issue.FilePath(), fs.generated) @@ -109,73 +75,3 @@ func (p *GeneratedFileFilter) shouldPassIssue(issue *result.Issue) (bool, error) // don't report issues for autogenerated files return !fs.generated, nil } - -// isGeneratedFileLax reports whether the source file is generated code. -// The function uses a bit laxer rules than isGeneratedFileStrict to match more generated code. -// See https://github.com/golangci/golangci-lint/issues/48 and https://github.com/golangci/golangci-lint/issues/72. -func (p *GeneratedFileFilter) isGeneratedFileLax(doc string) bool { - markers := []string{genCodeGenerated, genDoNotEdit, genAutoFile, genSwaggerCodegen} - - doc = strings.ToLower(doc) - - for _, marker := range markers { - if strings.Contains(doc, marker) { - p.debugf("doc contains marker %q: file is generated", marker) - - return true - } - } - - p.debugf("doc of len %d doesn't contain any of markers: %s", len(doc), markers) - - return false -} - -// isGeneratedFileStrict returns true if the source file has a line that matches the regular expression: -// -// ^// Code generated .* DO NOT EDIT\.$ -// -// This line must appear before the first non-comment, non-blank text in the file. -// Based on https://go.dev/s/generatedcode. -func (p *GeneratedFileFilter) isGeneratedFileStrict(filePath string) (bool, error) { - file, err := parser.ParseFile(token.NewFileSet(), filePath, nil, parser.PackageClauseOnly|parser.ParseComments) - if err != nil { - return false, fmt.Errorf("failed to parse file: %w", err) - } - - if file == nil || len(file.Comments) == 0 { - return false, nil - } - - for _, comment := range file.Comments { - if comment.Pos() > file.Package { - return false, nil - } - - for _, line := range comment.List { - generated := p.strictPattern.MatchString(line.Text) - if generated { - p.debugf("doc contains ignore expression: file is generated") - - return true, nil - } - } - } - - return false, nil -} - -func getComments(filePath string) (string, error) { - fset := token.NewFileSet() - syntax, err := parser.ParseFile(fset, filePath, nil, parser.PackageClauseOnly|parser.ParseComments) - if err != nil { - return "", fmt.Errorf("failed to parse file: %w", err) - } - - var docLines []string - for _, c := range syntax.Comments { - docLines = append(docLines, strings.TrimSpace(c.Text())) - } - - return strings.Join(docLines, "\n"), nil -} diff --git a/pkg/result/processors/exclusion_generated_file_filter_test.go b/pkg/result/processors/exclusion_generated_file_filter_test.go index fe810890ebd2..56570bd65ad1 100644 --- a/pkg/result/processors/exclusion_generated_file_filter_test.go +++ b/pkg/result/processors/exclusion_generated_file_filter_test.go @@ -10,145 +10,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/result" ) -func TestGeneratedFileFilter_isGeneratedFileLax_generated(t *testing.T) { - p := NewGeneratedFileFilter(AutogeneratedModeLax) - - comments := []string{ - ` // generated by stringer -type Pill pill.go; DO NOT EDIT`, - `// Code generated by "stringer -type Pill pill.go"; DO NOT EDIT`, - `// Code generated by vfsgen; DO NOT EDIT`, - `// Created by cgo -godefs - DO NOT EDIT`, - `/* Created by cgo - DO NOT EDIT. */`, - `// Generated by stringer -i a.out.go -o anames.go -p ppc64 -// Do not edit.`, - `// DO NOT EDIT -// generated by: x86map -fmt=decoder ../x86.csv`, - `// DO NOT EDIT. -// Generate with: go run gen.go -full -output md5block.go`, - `// generated by "go run gen.go". DO NOT EDIT.`, - `// DO NOT EDIT. This file is generated by mksyntaxgo from the RE2 distribution.`, - `// GENERATED BY make_perl_groups.pl; DO NOT EDIT.`, - `// generated by mknacl.sh - do not edit`, - `// DO NOT EDIT ** This file was generated with the bake tool ** DO NOT EDIT //`, - `// Generated by running -// maketables --tables=all --data=http://www.unicode.org/Public/8.0.0/ucd/UnicodeData.txt -// --casefolding=http://www.unicode.org/Public/8.0.0/ucd/CaseFolding.txt -// DO NOT EDIT`, - `/* -* CODE GENERATED AUTOMATICALLY WITH github.com/ernesto-jimenez/gogen/unmarshalmap -* THIS FILE SHOULD NOT BE EDITED BY HAND - */`, - `// AUTOGENERATED FILE: easyjson file.go`, - ` * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git)`, - } - - for _, comment := range comments { - t.Run(comment, func(t *testing.T) { - t.Parallel() - - generated := p.isGeneratedFileLax(comment) - assert.True(t, generated) - }) - } -} - -func TestGeneratedFileFilter_isGeneratedFileLax_nonGenerated(t *testing.T) { - p := NewGeneratedFileFilter(AutogeneratedModeLax) - - comments := []string{ - "code not generated by", - "test", - } - - for _, comment := range comments { - t.Run(comment, func(t *testing.T) { - t.Parallel() - - generated := p.isGeneratedFileLax(comment) - assert.False(t, generated) - }) - } -} - -func TestGeneratedFileFilter_isGeneratedFileStrict(t *testing.T) { - p := NewGeneratedFileFilter(AutogeneratedModeStrict) - - testCases := []struct { - desc string - filepath string - assert assert.BoolAssertionFunc - }{ - { - desc: "go strict", - filepath: filepath.FromSlash("testdata/exclusion_generated_file_filter/go_strict.go"), - assert: assert.True, - }, - { - desc: "go strict invalid", - filepath: filepath.FromSlash("testdata/exclusion_generated_file_filter/go_strict_invalid.go"), - assert: assert.False, - }, - } - - for _, test := range testCases { - t.Run(test.desc, func(t *testing.T) { - t.Parallel() - - generated, err := p.isGeneratedFileStrict(test.filepath) - require.NoError(t, err) - - test.assert(t, generated) - }) - } -} - -func Test_getComments(t *testing.T) { - testCases := []struct { - fpath string - doc string - }{ - { - fpath: filepath.FromSlash("testdata/exclusion_generated_file_filter/exclude.go"), - doc: `first line -second line -third line -this text also -and this text also`, - }, - { - fpath: filepath.FromSlash("testdata/exclusion_generated_file_filter/exclude_doc.go"), - doc: `DO NOT EDIT`, - }, - { - fpath: filepath.FromSlash("testdata/exclusion_generated_file_filter/exclude_block_comment.go"), - doc: `* first line - * - * second line - * third line -and this text also -this type of block comment also -this one line comment also`, - }, - } - - for _, tc := range testCases { - doc, err := getComments(tc.fpath) - require.NoError(t, err) - assert.Equal(t, tc.doc, doc) - } -} - -// Issue 954: Some lines can be very long, e.g. auto-generated -// embedded resources. Reported on file of 86.2KB. -func Test_getComments_fileWithLongLine(t *testing.T) { - fpath := filepath.FromSlash("testdata/exclusion_generated_file_filter/exclude_long_line.go") - _, err := getComments(fpath) - assert.NoError(t, err) -} - func TestGeneratedFileFilter_shouldPassIssue(t *testing.T) { testCases := []struct { desc string @@ -158,7 +23,7 @@ func TestGeneratedFileFilter_shouldPassIssue(t *testing.T) { }{ { desc: "lax ", - mode: AutogeneratedModeLax, + mode: config.GeneratedModeLax, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -169,7 +34,7 @@ func TestGeneratedFileFilter_shouldPassIssue(t *testing.T) { }, { desc: "strict ", - mode: AutogeneratedModeStrict, + mode: config.GeneratedModeStrict, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -208,7 +73,7 @@ func TestGeneratedFileFilter_shouldPassIssue_error(t *testing.T) { }{ { desc: "non-existing file (lax)", - mode: AutogeneratedModeLax, + mode: config.GeneratedModeLax, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -220,7 +85,7 @@ func TestGeneratedFileFilter_shouldPassIssue_error(t *testing.T) { }, { desc: "non-existing file (strict)", - mode: AutogeneratedModeStrict, + mode: config.GeneratedModeStrict, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ diff --git a/pkg/result/processors/exclusion_generated_file_matcher.go b/pkg/result/processors/exclusion_generated_file_matcher.go new file mode 100644 index 000000000000..170635a84b3d --- /dev/null +++ b/pkg/result/processors/exclusion_generated_file_matcher.go @@ -0,0 +1,107 @@ +package processors + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "strings" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/logutils" +) + +// The values must be in lowercase. +const ( + genCodeGenerated = "code generated" + genDoNotEdit = "do not edit" + + // Related to easyjson. + genAutoFile = "autogenerated file" + + //nolint:lll // Long URL + // Related to Swagger Codegen. + // https://github.com/swagger-api/swagger-codegen/blob/61cfeac3b9d855b4eb8bffa0d118bece117bcb7d/modules/swagger-codegen/src/main/resources/go/partial_header.mustache#L16 + // https://github.com/swagger-api/swagger-codegen/issues/12358 + genSwaggerCodegen = "* generated by: swagger codegen " +) + +// GeneratedFileMatcher detects generated files. +// - mode "lax": see `isGeneratedFileLax` documentation. +// - mode "strict": see `isGeneratedFileStrict` documentation. +// - mode "disable": skips this processor. +type GeneratedFileMatcher struct { + debugf logutils.DebugFunc + + mode string +} + +func NewGeneratedFileMatcher(mode string) *GeneratedFileMatcher { + return &GeneratedFileMatcher{ + debugf: logutils.Debug(logutils.DebugKeyGeneratedFileFilter), + mode: mode, + } +} + +func (p *GeneratedFileMatcher) IsGeneratedFile(filepath string, src any) (bool, error) { + if p.mode == config.GeneratedModeDisable { + return false, nil + } + + file, err := parser.ParseFile(token.NewFileSet(), filepath, src, parser.PackageClauseOnly|parser.ParseComments) + if err != nil { + return false, fmt.Errorf("failed to parse file: %w", err) + } + + if p.mode == config.GeneratedModeStrict { + return isGeneratedFileStrict(file), nil + } + + doc := getComments(file) + + return p.isGeneratedFileLax(doc), nil +} + +// isGeneratedFileLax reports whether the source file is generated code. +// The function uses a bit laxer rules than isGeneratedFileStrict to match more generated code. +// See https://github.com/golangci/golangci-lint/issues/48 and https://github.com/golangci/golangci-lint/issues/72. +func (p *GeneratedFileMatcher) isGeneratedFileLax(doc string) bool { + markers := []string{genCodeGenerated, genDoNotEdit, genAutoFile, genSwaggerCodegen} + + doc = strings.ToLower(doc) + + for _, marker := range markers { + if strings.Contains(doc, marker) { + p.debugf("doc contains marker %q: file is generated", marker) + + return true + } + } + + p.debugf("doc of len %d doesn't contain any of markers: %s", len(doc), markers) + + return false +} + +// isGeneratedFileStrict returns true if the source file has a line that matches the regular expression: +// +// ^// Code generated .* DO NOT EDIT\.$ +// +// This line must appear before the first non-comment, non-blank text in the file. +// Based on https://go.dev/s/generatedcode. +func isGeneratedFileStrict(file *ast.File) bool { + if file == nil || len(file.Comments) == 0 { + return false + } + + return ast.IsGenerated(file) +} + +func getComments(file *ast.File) string { + var docLines []string + for _, c := range file.Comments { + docLines = append(docLines, strings.TrimSpace(c.Text())) + } + + return strings.Join(docLines, "\n") +} diff --git a/pkg/result/processors/exclusion_generated_file_matcher_test.go b/pkg/result/processors/exclusion_generated_file_matcher_test.go new file mode 100644 index 000000000000..421bb63f95e3 --- /dev/null +++ b/pkg/result/processors/exclusion_generated_file_matcher_test.go @@ -0,0 +1,154 @@ +package processors + +import ( + "go/parser" + "go/token" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/golangci/golangci-lint/pkg/config" +) + +func TestGeneratedFileMatcher_isGeneratedFileLax_generated(t *testing.T) { + p := NewGeneratedFileMatcher(config.GeneratedModeLax) + + comments := []string{ + ` // generated by stringer -type Pill pill.go; DO NOT EDIT`, + `// Code generated by "stringer -type Pill pill.go"; DO NOT EDIT`, + `// Code generated by vfsgen; DO NOT EDIT`, + `// Created by cgo -godefs - DO NOT EDIT`, + `/* Created by cgo - DO NOT EDIT. */`, + `// Generated by stringer -i a.out.go -o anames.go -p ppc64 +// Do not edit.`, + `// DO NOT EDIT +// generated by: x86map -fmt=decoder ../x86.csv`, + `// DO NOT EDIT. +// Generate with: go run gen.go -full -output md5block.go`, + `// generated by "go run gen.go". DO NOT EDIT.`, + `// DO NOT EDIT. This file is generated by mksyntaxgo from the RE2 distribution.`, + `// GENERATED BY make_perl_groups.pl; DO NOT EDIT.`, + `// generated by mknacl.sh - do not edit`, + `// DO NOT EDIT ** This file was generated with the bake tool ** DO NOT EDIT //`, + `// Generated by running +// maketables --tables=all --data=http://www.unicode.org/Public/8.0.0/ucd/UnicodeData.txt +// --casefolding=http://www.unicode.org/Public/8.0.0/ucd/CaseFolding.txt +// DO NOT EDIT`, + `/* +* CODE GENERATED AUTOMATICALLY WITH github.com/ernesto-jimenez/gogen/unmarshalmap +* THIS FILE SHOULD NOT BE EDITED BY HAND + */`, + `// AUTOGENERATED FILE: easyjson file.go`, + ` * Generated by: Swagger Codegen (https://github.com/swagger-api/swagger-codegen.git)`, + } + + for _, comment := range comments { + t.Run(comment, func(t *testing.T) { + t.Parallel() + + generated := p.isGeneratedFileLax(comment) + assert.True(t, generated) + }) + } +} + +func TestGeneratedFileMatcher_isGeneratedFileLax_nonGenerated(t *testing.T) { + p := NewGeneratedFileMatcher(config.GeneratedModeLax) + + comments := []string{ + "code not generated by", + "test", + } + + for _, comment := range comments { + t.Run(comment, func(t *testing.T) { + t.Parallel() + + generated := p.isGeneratedFileLax(comment) + assert.False(t, generated) + }) + } +} + +func Test_isGeneratedFileStrict(t *testing.T) { + testCases := []struct { + desc string + filepath string + assert assert.BoolAssertionFunc + }{ + { + desc: "go strict", + filepath: filepath.FromSlash("testdata/exclusion_generated_file_filter/go_strict.go"), + assert: assert.True, + }, + { + desc: "go strict invalid", + filepath: filepath.FromSlash("testdata/exclusion_generated_file_filter/go_strict_invalid.go"), + assert: assert.False, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + file, err := parser.ParseFile(token.NewFileSet(), test.filepath, nil, parser.PackageClauseOnly|parser.ParseComments) + require.NoError(t, err) + + generated := isGeneratedFileStrict(file) + + test.assert(t, generated) + }) + } +} + +func Test_getComments(t *testing.T) { + testCases := []struct { + fpath string + doc string + }{ + { + fpath: filepath.FromSlash("testdata/exclusion_generated_file_filter/exclude.go"), + doc: `first line +second line +third line +this text also +and this text also`, + }, + { + fpath: filepath.FromSlash("testdata/exclusion_generated_file_filter/exclude_doc.go"), + doc: `DO NOT EDIT`, + }, + { + fpath: filepath.FromSlash("testdata/exclusion_generated_file_filter/exclude_block_comment.go"), + doc: `* first line + * + * second line + * third line +and this text also +this type of block comment also +this one line comment also`, + }, + } + + for _, tc := range testCases { + file, err := parser.ParseFile(token.NewFileSet(), tc.fpath, nil, parser.PackageClauseOnly|parser.ParseComments) + require.NoError(t, err) + + doc := getComments(file) + assert.Equal(t, tc.doc, doc) + } +} + +// Issue 954: Some lines can be very long, e.g. auto-generated +// embedded resources. Reported on file of 86.2KB. +func TestGeneratedFileMatcher_IsGeneratedFile_fileWithLongLine(t *testing.T) { + p := NewGeneratedFileMatcher(config.GeneratedModeLax) + + fpath := filepath.FromSlash("testdata/exclusion_generated_file_filter/exclude_long_line.go") + + _, err := p.IsGeneratedFile(fpath, nil) + assert.NoError(t, err) +}