diff --git a/pkg/goformatters/meta_formatter.go b/pkg/goformatters/meta_formatter.go new file mode 100644 index 000000000000..e07b3aad8055 --- /dev/null +++ b/pkg/goformatters/meta_formatter.go @@ -0,0 +1,74 @@ +package goformatters + +import ( + "bytes" + "fmt" + "go/format" + + "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" +) + +type MetaFormatter struct { + log logutils.Log + formatters []Formatter +} + +func NewMetaFormatter(log logutils.Log, cfg *config.Config, enabledLinters map[string]*linter.Config) (*MetaFormatter, error) { + m := &MetaFormatter{log: log} + + if _, ok := enabledLinters[gofmt.Name]; ok { + m.formatters = append(m.formatters, gofmt.New(cfg.LintersSettings.Gofmt)) + } + + if _, ok := enabledLinters[gofumpt.Name]; ok { + m.formatters = append(m.formatters, gofumpt.New(cfg.LintersSettings.Gofumpt, cfg.Run.Go)) + } + + if _, ok := enabledLinters[goimports.Name]; ok { + m.formatters = append(m.formatters, goimports.New()) + } + + // 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 err != nil { + return nil, fmt.Errorf("gci: creating formatter: %w", err) + } + + m.formatters = append(m.formatters, formatter) + } + + return m, nil +} + +func (m *MetaFormatter) Format(filename string, src []byte) []byte { + if len(m.formatters) == 0 { + data, err := format.Source(src) + if err != nil { + m.log.Warnf("(fmt) formatting file %s: %v", filename, err) + return src + } + + return data + } + + data := bytes.Clone(src) + + for _, formatter := range m.formatters { + formatted, err := formatter.Format(filename, data) + if err != nil { + m.log.Warnf("(%s) formatting file %s: %v", formatter.Name(), filename, err) + continue + } + + data = formatted + } + + return data +} diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index 5a40c58ea6d4..157fde715f7b 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -10,6 +10,7 @@ import ( "github.com/golangci/golangci-lint/internal/errorutil" "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/goutil" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/lint/lintersdb" @@ -60,9 +61,9 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti return nil, fmt.Errorf("failed to get enabled linters: %w", err) } - formatter, err := processors.NewFormatter(log, cfg, enabledLinters) + metaFormatter, err := goformatters.NewMetaFormatter(log, cfg, enabledLinters) if err != nil { - return nil, fmt.Errorf("failed to create formatter: %w", err) + return nil, fmt.Errorf("failed to create meta-formatter: %w", err) } return &Runner{ @@ -99,9 +100,7 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti processors.NewSeverity(log.Child(logutils.DebugKeySeverityRules), files, &cfg.Severity), // The fixer still needs to see paths for the issues that are relative to the current directory. - processors.NewFixer(cfg, log, fileCache), - // The formatter needs to be after the fixer and the last processor that write files. - formatter, + processors.NewFixer(cfg, log, fileCache, metaFormatter), // Now we can modify the issues for output. processors.NewPathPrefixer(cfg.Output.PathPrefix), diff --git a/pkg/result/processors/fixer.go b/pkg/result/processors/fixer.go index f6a40c92bbfb..b82c7f207192 100644 --- a/pkg/result/processors/fixer.go +++ b/pkg/result/processors/fixer.go @@ -9,7 +9,6 @@ package processors import ( "errors" "fmt" - "go/format" "os" "slices" @@ -18,6 +17,7 @@ import ( "github.com/golangci/golangci-lint/internal/x/tools/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/goformatters/gci" "github.com/golangci/golangci-lint/pkg/goformatters/gofmt" "github.com/golangci/golangci-lint/pkg/goformatters/gofumpt" @@ -36,14 +36,16 @@ type Fixer struct { log logutils.Log fileCache *fsutils.FileCache sw *timeutils.Stopwatch + formatter *goformatters.MetaFormatter } -func NewFixer(cfg *config.Config, log logutils.Log, fileCache *fsutils.FileCache) *Fixer { +func NewFixer(cfg *config.Config, log logutils.Log, fileCache *fsutils.FileCache, formatter *goformatters.MetaFormatter) *Fixer { return &Fixer{ cfg: cfg, log: log, fileCache: fileCache, sw: timeutils.NewStopwatch("fixer", log), + formatter: formatter, } } @@ -79,11 +81,13 @@ func (p Fixer) process(issues []result.Issue) ([]result.Issue, error) { var notFixableIssues []result.Issue + toBeFormattedFiles := make(map[string]struct{}) + for i := range issues { issue := issues[i] if slices.Contains(formatters, issue.FromLinter) { - notFixableIssues = append(notFixableIssues, issue) + toBeFormattedFiles[issue.FilePath()] = struct{}{} continue } @@ -173,6 +177,8 @@ func (p Fixer) process(issues []result.Issue) ([]result.Issue, error) { var editError error + var formattedFiles []string + // Now we've got a set of valid edits for each file. Apply them. for path, edits := range editsByPath { contents, err := p.fileCache.GetFileBytes(path) @@ -188,14 +194,32 @@ func (p Fixer) process(issues []result.Issue) ([]result.Issue, error) { } // Try to format the file. - if formatted, err := format.Source(out); err == nil { - out = formatted - } + out = p.formatter.Format(path, out) if err := os.WriteFile(path, out, filePerm); err != nil { editError = errors.Join(editError, fmt.Errorf("%s: %w", path, err)) continue } + + formattedFiles = append(formattedFiles, path) + } + + for path := range toBeFormattedFiles { + // Skips files already formatted by the previous fix step. + if !slices.Contains(formattedFiles, path) { + content, err := p.fileCache.GetFileBytes(path) + if err != nil { + p.log.Warnf("Error reading file %s: %v", path, err) + continue + } + + out := p.formatter.Format(path, content) + + if err := os.WriteFile(path, out, filePerm); err != nil { + editError = errors.Join(editError, fmt.Errorf("%s: %w", path, err)) + continue + } + } } return notFixableIssues, editError diff --git a/pkg/result/processors/formatter.go b/pkg/result/processors/formatter.go deleted file mode 100644 index 4a31c5bdbba5..000000000000 --- a/pkg/result/processors/formatter.go +++ /dev/null @@ -1,128 +0,0 @@ -package processors - -import ( - "bytes" - "fmt" - "os" - "slices" - - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/goformatters" - "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" - "github.com/golangci/golangci-lint/pkg/result" -) - -// Formatter runs all the "formatters". -// It should be run after the [Fixer] because: -// - The code format is applied after the fixes to avoid changing positions. -// - The [Fixer] writes the files on the disk (so the file cache cannot be used as it contains the files before fixes). -type Formatter struct { - log logutils.Log - cfg *config.Config - formatters []goformatters.Formatter -} - -// NewFormatter creates a new [Formatter]. -func NewFormatter(log logutils.Log, cfg *config.Config, enabledLinters map[string]*linter.Config) (*Formatter, error) { - p := &Formatter{ - log: log, - cfg: cfg, - } - - if _, ok := enabledLinters[gofmt.Name]; ok { - p.formatters = append(p.formatters, gofmt.New(cfg.LintersSettings.Gofmt)) - } - - if _, ok := enabledLinters[gofumpt.Name]; ok { - p.formatters = append(p.formatters, gofumpt.New(cfg.LintersSettings.Gofumpt, cfg.Run.Go)) - } - - if _, ok := enabledLinters[goimports.Name]; ok { - p.formatters = append(p.formatters, goimports.New()) - } - - // 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 err != nil { - return nil, fmt.Errorf("gci: creating formatter: %w", err) - } - - p.formatters = append(p.formatters, formatter) - } - - return p, nil -} - -func (*Formatter) Name() string { - return "formatter" -} - -func (p *Formatter) Process(issues []result.Issue) ([]result.Issue, error) { - if !p.cfg.Issues.NeedFix { - return issues, nil - } - - if len(p.formatters) == 0 { - return issues, nil - } - - all := []string{gofumpt.Name, goimports.Name, gofmt.Name, gci.Name} - - var notFixableIssues []result.Issue - - files := make(map[string]struct{}) - - for i := range issues { - issue := issues[i] - - if slices.Contains(all, issue.FromLinter) { - files[issue.FilePath()] = struct{}{} - } else { - notFixableIssues = append(notFixableIssues, issue) - } - } - - for target := range files { - content, err := os.ReadFile(target) - if err != nil { - p.log.Warnf("Error reading file %s: %v", target, err) - continue - } - - formatted := p.format(target, content) - if bytes.Equal(content, formatted) { - continue - } - - err = os.WriteFile(target, formatted, filePerm) - if err != nil { - p.log.Warnf("Writing file %s: %v", target, err) - } - } - - return notFixableIssues, nil -} - -func (p *Formatter) format(filename string, src []byte) []byte { - data := bytes.Clone(src) - - for _, formatter := range p.formatters { - formatted, err := formatter.Format(filename, data) - if err != nil { - p.log.Warnf("(%s) formatting file %s: %v", formatter.Name(), filename, err) - continue - } - - data = formatted - } - - return data -} - -func (*Formatter) Finish() {}