Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: format fixed files with the same formatters as the issues related to formatting #5267

Merged
merged 4 commits into from
Jan 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions pkg/goformatters/meta_formatter.go
Original file line number Diff line number Diff line change
@@ -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
}
ldez marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 4 additions & 5 deletions pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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),
Expand Down
36 changes: 30 additions & 6 deletions pkg/result/processors/fixer.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ package processors
import (
"errors"
"fmt"
"go/format"
"os"
"slices"

Expand All @@ -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"
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
128 changes: 0 additions & 128 deletions pkg/result/processors/formatter.go

This file was deleted.

Loading