fix: combination of --fix and --path-prefix (#3700)

Co-authored-by: Fernandez Ludovic <[email protected]>

Author: pohly

pkg/commands/run.go

@@ -23,7 +23,6 @@ import (
 	"github.com/golangci/golangci-lint/pkg/packages"
 	"github.com/golangci/golangci-lint/pkg/printers"
 	"github.com/golangci/golangci-lint/pkg/result"
-	"github.com/golangci/golangci-lint/pkg/result/processors"
 )
 
 const defaultFileMode = 0644
@@ -360,18 +359,12 @@ func (e *Executor) runAnalysis(ctx context.Context, args []string) ([]result.Iss
 	lintCtx.Log = e.log.Child(logutils.DebugKeyLintersContext)
 
 	runner, err := lint.NewRunner(e.cfg, e.log.Child(logutils.DebugKeyRunner),
-		e.goenv, e.EnabledLintersSet, e.lineCache, e.DBManager, lintCtx.Packages)
+		e.goenv, e.EnabledLintersSet, e.lineCache, e.fileCache, e.DBManager, lintCtx.Packages)
 	if err != nil {
 		return nil, err
 	}
 
-	issues, err := runner.Run(ctx, lintersToRun, lintCtx)
-	if err != nil {
-		return nil, err
-	}
-
-	fixer := processors.NewFixer(e.cfg, e.log, e.fileCache)
-	return fixer.Process(issues), nil
+	return runner.Run(ctx, lintersToRun, lintCtx)
 }
 
 func (e *Executor) setOutputToDevNull() (savedStdout, savedStderr *os.File) {

pkg/lint/runner.go

@@ -27,8 +27,10 @@ type Runner struct {
 	Log        logutils.Log
 }
 
-func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lintersdb.EnabledSet,
-	lineCache *fsutils.LineCache, dbManager *lintersdb.Manager, pkgs []*gopackages.Package) (*Runner, error) {
+func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env,
+	es *lintersdb.EnabledSet,
+	lineCache *fsutils.LineCache, fileCache *fsutils.FileCache,
+	dbManager *lintersdb.Manager, pkgs []*gopackages.Package) (*Runner, error) {
 	// Beware that some processors need to add the path prefix when working with paths
 	// because they get invoked before the path prefixer (exclude and severity rules)
 	// or process other paths (skip files).
@@ -98,6 +100,11 @@ func NewRunner(cfg *config.Config, log logutils.Log, goenv *goutil.Env, es *lint
 			processors.NewSourceCode(lineCache, log.Child(logutils.DebugKeySourceCode)),
 			processors.NewPathShortener(),
 			getSeverityRulesProcessor(&cfg.Severity, log, files),
+
+			// The fixer still needs to see paths for the issues that are relative to the current directory.
+			processors.NewFixer(cfg, log, fileCache),
+
+			// Now we can modify the issues for output.
 			processors.NewPathPrefixer(cfg.Output.PathPrefix),
 			processors.NewSortResults(cfg),
 		},

pkg/result/processors/fixer.go

@@ -16,6 +16,8 @@ import (
 	"github.com/golangci/golangci-lint/pkg/timeutils"
 )
 
+var _ Processor = Fixer{}
+
 type Fixer struct {
 	cfg       *config.Config
 	log       logutils.Log
@@ -36,9 +38,9 @@ func (f Fixer) printStat() {
 	f.sw.PrintStages()
 }
 
-func (f Fixer) Process(issues []result.Issue) []result.Issue {
+func (f Fixer) Process(issues []result.Issue) ([]result.Issue, error) {
 	if !f.cfg.Issues.NeedFix {
-		return issues
+		return issues, nil
 	}
 
 	outIssues := make([]result.Issue, 0, len(issues))
@@ -67,9 +69,15 @@ func (f Fixer) Process(issues []result.Issue) []result.Issue {
 	}
 
 	f.printStat()
-	return outIssues
+	return outIssues, nil
+}
+
+func (f Fixer) Name() string {
+	return "fixer"
 }
 
+func (f Fixer) Finish() {}
+
 func (f Fixer) fixIssuesInFile(filePath string, issues []result.Issue) error {
 	// TODO: don't read the whole file into memory: read line by line;
 	// can't just use bufio.scanner: it has a line length limit

test/fix_test.go

@@ -14,7 +14,9 @@ import (
 // value: "1"
 const envKeepTempFiles = "GL_KEEP_TEMP_FILES"
 
-func TestFix(t *testing.T) {
+func setupTestFix(t *testing.T) []string {
+	t.Helper()
+
 	testshared.SkipOnWindows(t)
 
 	tmpDir := filepath.Join(testdataDir, "fix.tmp")
@@ -33,7 +35,48 @@ func TestFix(t *testing.T) {
 
 	testshared.InstallGolangciLint(t)
 
-	sources := findSources(t, tmpDir, "in", "*.go")
+	return findSources(t, tmpDir, "in", "*.go")
+}
+
+func TestFix(t *testing.T) {
+	sources := setupTestFix(t)
+
+	for _, input := range sources {
+		input := input
+		t.Run(filepath.Base(input), func(t *testing.T) {
+			t.Parallel()
+
+			rc := testshared.ParseTestDirectives(t, input)
+			if rc == nil {
+				t.Logf("Skipped: %s", input)
+				return
+			}
+
+			testshared.NewRunnerBuilder(t).
+				WithArgs("--disable-all",
+					"--print-issued-lines=false",
+					"--print-linter-name=false",
+					"--out-format=line-number",
+					"--fix").
+				WithRunContext(rc).
+				WithTargetPath(input).
+				Runner().
+				Run().
+				ExpectExitCode(rc.ExitCode)
+
+			output, err := os.ReadFile(input)
+			require.NoError(t, err)
+
+			expectedOutput, err := os.ReadFile(filepath.Join(testdataDir, "fix", "out", filepath.Base(input)))
+			require.NoError(t, err)
+
+			require.Equal(t, string(expectedOutput), string(output))
+		})
+	}
+}
+
+func TestFix_pathPrefix(t *testing.T) {
+	sources := setupTestFix(t)
 
 	for _, input := range sources {
 		input := input
@@ -47,13 +90,12 @@ func TestFix(t *testing.T) {
 			}
 
 			testshared.NewRunnerBuilder(t).
-				WithArgs(
-					"--disable-all",
+				WithArgs("--disable-all",
 					"--print-issued-lines=false",
 					"--print-linter-name=false",
 					"--out-format=line-number",
 					"--fix",
-				).
+					"--path-prefix=foobar/").
 				WithRunContext(rc).
 				WithTargetPath(input).
 				Runner().