From 4ec7afc1fb23977e6c7e66e80afb253d1365ecf4 Mon Sep 17 00:00:00 2001 From: "David L. Chandler" Date: Fri, 7 Nov 2025 23:26:43 -0500 Subject: [PATCH 1/4] Fixes an issue with the 'fmt' command and symbolic links. 'ln -s worktree wt && cd wt && golangci-lint fmt' now works Signed-off-by: David L. Chandler --- pkg/goformat/runner.go | 10 ++- pkg/goformat/runner_test.go | 131 ++++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 pkg/goformat/runner_test.go diff --git a/pkg/goformat/runner.go b/pkg/goformat/runner.go index f87626158179..300fbe60b0a0 100644 --- a/pkg/goformat/runner.go +++ b/pkg/goformat/runner.go @@ -251,7 +251,15 @@ func (o RunnerOptions) MatchAnyPattern(path string) (bool, error) { return false, nil } - rel, err := filepath.Rel(o.basePath, path) + // Resolve symlinks in the file path to ensure filepath.Rel works correctly + // when running inside symlinked directories. The basePath is already resolved + // via fsutils.Getwd() in NewRunnerOptions. + evalPath, err := fsutils.EvalSymlinks(path) + if err != nil { + return false, err + } + + rel, err := filepath.Rel(o.basePath, evalPath) if err != nil { return false, err } diff --git a/pkg/goformat/runner_test.go b/pkg/goformat/runner_test.go new file mode 100644 index 000000000000..255c8c515719 --- /dev/null +++ b/pkg/goformat/runner_test.go @@ -0,0 +1,131 @@ +package goformat + +import ( + "os" + "path/filepath" + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/golangci/golangci-lint/v2/pkg/fsutils" +) + +func TestMatchAnyPattern_WithSymlinks(t *testing.T) { + // Create a temporary directory structure + tmpDir, err := os.MkdirTemp("", "golangci-symlink-test-*") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + // Create two separate directories at the same level + realDir := filepath.Join(tmpDir, "real_project") + err = os.MkdirAll(realDir, 0o755) + require.NoError(t, err) + + testFile := filepath.Join(realDir, "test.go") + err = os.WriteFile(testFile, []byte("package main"), 0o600) + require.NoError(t, err) + + // Create a symlink in a completely different part of the tree + symlinkParent := filepath.Join(tmpDir, "symlinks") + err = os.MkdirAll(symlinkParent, 0o755) + require.NoError(t, err) + + symlinkDir := filepath.Join(symlinkParent, "project_link") + err = os.Symlink(realDir, symlinkDir) + require.NoError(t, err) + + // Simulate the actual scenario: + // - basePath is the resolved real path (as fsutils.Getwd does when you're in a symlinked dir) + // - filepath.Walk from the symlink directory provides unresolved paths + // IMPORTANT: On macOS, tmpDir might be /var/... which is itself a symlink to /private/var/... + // So we need to evaluate basePath as well to simulate what fsutils.Getwd() does + resolvedBasePath, err := fsutils.EvalSymlinks(realDir) + require.NoError(t, err) + + // Create RunnerOptions with a pattern that matches files in the root directory only + // This pattern will match "test.go" but NOT "../../../test.go" or similar broken paths + pattern := regexp.MustCompile(`^[^/\\]+\.go$`) + opts := RunnerOptions{ + basePath: resolvedBasePath, // Resolved path from Getwd + patterns: []*regexp.Regexp{pattern}, + excludedPathCounter: map[*regexp.Regexp]int{pattern: 0}, + } + + // filepath.Walk would provide the path through the symlink + // When you cd into a symlink and run the command, Walk uses the symlink path + unresolvedFile := filepath.Join(symlinkDir, "test.go") + + // The issue: When basePath is resolved (e.g., /private/var/...) + // but the file path from filepath.Walk is unresolved (e.g., /var/...), + // filepath.Rel produces an incorrect relative path with many ../ components + // like "../../../../var/.../test.go" which won't match the pattern ^[^/\\]+\.go$ + // + // The fix: EvalSymlinks on the file path before calling filepath.Rel + // ensures both paths are in their canonical form, producing "test.go" + // which correctly matches the pattern. + + match, err := opts.MatchAnyPattern(unresolvedFile) + + // With the fix, pattern matching should work correctly + require.NoError(t, err, "Should not error when matching pattern with symlinks") + assert.True(t, match, "Pattern should match test.go when accessed through symlink") + assert.Equal(t, 1, opts.excludedPathCounter[pattern], "Counter should be incremented") +} + +func TestMatchAnyPattern_NoPatterns(t *testing.T) { + opts := RunnerOptions{ + basePath: "/tmp", + patterns: []*regexp.Regexp{}, + excludedPathCounter: map[*regexp.Regexp]int{}, + } + + match, err := opts.MatchAnyPattern("/tmp/test.go") + require.NoError(t, err) + assert.False(t, match, "Should not match when no patterns are defined") +} + +func TestMatchAnyPattern_NoMatch(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "golangci-test-*") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + testFile := filepath.Join(tmpDir, "test.go") + err = os.WriteFile(testFile, []byte("package main"), 0o600) + require.NoError(t, err) + + pattern := regexp.MustCompile(`excluded\.go`) + opts := RunnerOptions{ + basePath: tmpDir, + patterns: []*regexp.Regexp{pattern}, + excludedPathCounter: map[*regexp.Regexp]int{pattern: 0}, + } + + match, err := opts.MatchAnyPattern(testFile) + require.NoError(t, err) + assert.False(t, match, "Pattern should not match test.go") + assert.Equal(t, 0, opts.excludedPathCounter[pattern], "Counter should not be incremented") +} + +func TestMatchAnyPattern_WithMatch(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "golangci-test-*") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + testFile := filepath.Join(tmpDir, "generated.go") + err = os.WriteFile(testFile, []byte("package main"), 0o600) + require.NoError(t, err) + + pattern := regexp.MustCompile(`generated\.go`) + opts := RunnerOptions{ + basePath: tmpDir, + patterns: []*regexp.Regexp{pattern}, + excludedPathCounter: map[*regexp.Regexp]int{pattern: 0}, + } + + match, err := opts.MatchAnyPattern(testFile) + require.NoError(t, err) + assert.True(t, match, "Pattern should match generated.go") + assert.Equal(t, 1, opts.excludedPathCounter[pattern], "Counter should be incremented") +} From 9597740b051be429d2981475162ad6bbbaaf9520 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 8 Nov 2025 07:07:31 +0100 Subject: [PATCH 2/4] review --- pkg/goformat/runner.go | 8 +- pkg/goformat/runner_test.go | 191 ++++++++++++++++++------------------ 2 files changed, 100 insertions(+), 99 deletions(-) diff --git a/pkg/goformat/runner.go b/pkg/goformat/runner.go index 300fbe60b0a0..3c65a79d8496 100644 --- a/pkg/goformat/runner.go +++ b/pkg/goformat/runner.go @@ -251,15 +251,13 @@ func (o RunnerOptions) MatchAnyPattern(path string) (bool, error) { return false, nil } - // Resolve symlinks in the file path to ensure filepath.Rel works correctly - // when running inside symlinked directories. The basePath is already resolved - // via fsutils.Getwd() in NewRunnerOptions. - evalPath, err := fsutils.EvalSymlinks(path) + // The basePath is already resolved via `fsutils.Getwd()` in `NewRunnerOptions`. + evaluatedPath, err := fsutils.EvalSymlinks(path) if err != nil { return false, err } - rel, err := filepath.Rel(o.basePath, evalPath) + rel, err := filepath.Rel(o.basePath, evaluatedPath) if err != nil { return false, err } diff --git a/pkg/goformat/runner_test.go b/pkg/goformat/runner_test.go index 255c8c515719..5ed4930cc562 100644 --- a/pkg/goformat/runner_test.go +++ b/pkg/goformat/runner_test.go @@ -3,129 +3,132 @@ package goformat import ( "os" "path/filepath" - "regexp" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/golangci/golangci-lint/v2/pkg/fsutils" + "github.com/golangci/golangci-lint/v2/pkg/config" ) -func TestMatchAnyPattern_WithSymlinks(t *testing.T) { - // Create a temporary directory structure - tmpDir, err := os.MkdirTemp("", "golangci-symlink-test-*") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) +func TestRunnerOptions_MatchAnyPattern(t *testing.T) { + testCases := []struct { + desc string + cfg *config.Config + filename string + + assertMatch assert.BoolAssertionFunc + expectedCount int + }{ + { + desc: "match", + cfg: &config.Config{ + Formatters: config.Formatters{ + Exclusions: config.FormatterExclusions{ + Paths: []string{`generated\.go`}, + }, + }, + }, + filename: "generated.go", + assertMatch: assert.True, + expectedCount: 1, + }, + { + desc: "no match", + cfg: &config.Config{ + Formatters: config.Formatters{ + Exclusions: config.FormatterExclusions{ + Paths: []string{`excluded\.go`}, + }, + }, + }, + filename: "test.go", + assertMatch: assert.False, + expectedCount: 0, + }, + { + desc: "no patterns", + cfg: &config.Config{}, + filename: "test.go", + assertMatch: assert.False, + }, + } - // Create two separate directories at the same level - realDir := filepath.Join(tmpDir, "real_project") - err = os.MkdirAll(realDir, 0o755) - require.NoError(t, err) + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() - testFile := filepath.Join(realDir, "test.go") - err = os.WriteFile(testFile, []byte("package main"), 0o600) - require.NoError(t, err) + tmpDir := t.TempDir() - // Create a symlink in a completely different part of the tree - symlinkParent := filepath.Join(tmpDir, "symlinks") - err = os.MkdirAll(symlinkParent, 0o755) - require.NoError(t, err) + testFile := filepath.Join(tmpDir, test.filename) - symlinkDir := filepath.Join(symlinkParent, "project_link") - err = os.Symlink(realDir, symlinkDir) - require.NoError(t, err) + err := os.WriteFile(testFile, []byte("package main"), 0o600) + require.NoError(t, err) - // Simulate the actual scenario: - // - basePath is the resolved real path (as fsutils.Getwd does when you're in a symlinked dir) - // - filepath.Walk from the symlink directory provides unresolved paths - // IMPORTANT: On macOS, tmpDir might be /var/... which is itself a symlink to /private/var/... - // So we need to evaluate basePath as well to simulate what fsutils.Getwd() does - resolvedBasePath, err := fsutils.EvalSymlinks(realDir) - require.NoError(t, err) + test.cfg.SetConfigDir(tmpDir) - // Create RunnerOptions with a pattern that matches files in the root directory only - // This pattern will match "test.go" but NOT "../../../test.go" or similar broken paths - pattern := regexp.MustCompile(`^[^/\\]+\.go$`) - opts := RunnerOptions{ - basePath: resolvedBasePath, // Resolved path from Getwd - patterns: []*regexp.Regexp{pattern}, - excludedPathCounter: map[*regexp.Regexp]int{pattern: 0}, - } + opts, err := NewRunnerOptions(test.cfg, false, false, false) + require.NoError(t, err) - // filepath.Walk would provide the path through the symlink - // When you cd into a symlink and run the command, Walk uses the symlink path - unresolvedFile := filepath.Join(symlinkDir, "test.go") - - // The issue: When basePath is resolved (e.g., /private/var/...) - // but the file path from filepath.Walk is unresolved (e.g., /var/...), - // filepath.Rel produces an incorrect relative path with many ../ components - // like "../../../../var/.../test.go" which won't match the pattern ^[^/\\]+\.go$ - // - // The fix: EvalSymlinks on the file path before calling filepath.Rel - // ensures both paths are in their canonical form, producing "test.go" - // which correctly matches the pattern. - - match, err := opts.MatchAnyPattern(unresolvedFile) - - // With the fix, pattern matching should work correctly - require.NoError(t, err, "Should not error when matching pattern with symlinks") - assert.True(t, match, "Pattern should match test.go when accessed through symlink") - assert.Equal(t, 1, opts.excludedPathCounter[pattern], "Counter should be incremented") -} + match, err := opts.MatchAnyPattern(testFile) + require.NoError(t, err) -func TestMatchAnyPattern_NoPatterns(t *testing.T) { - opts := RunnerOptions{ - basePath: "/tmp", - patterns: []*regexp.Regexp{}, - excludedPathCounter: map[*regexp.Regexp]int{}, - } + test.assertMatch(t, match) - match, err := opts.MatchAnyPattern("/tmp/test.go") - require.NoError(t, err) - assert.False(t, match, "Should not match when no patterns are defined") + require.Len(t, opts.patterns, len(test.cfg.Formatters.Exclusions.Paths)) + + if len(opts.patterns) == 0 { + assert.Empty(t, opts.excludedPathCounter) + } else { + assert.Equal(t, test.expectedCount, opts.excludedPathCounter[opts.patterns[0]]) + } + }) + } } -func TestMatchAnyPattern_NoMatch(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "golangci-test-*") +func TestRunnerOptions_MatchAnyPattern_withSymlinks(t *testing.T) { + tmpDir := t.TempDir() + + testFile := filepath.Join(tmpDir, "project", "test.go") + + realDir := filepath.Dir(testFile) + + err := os.MkdirAll(realDir, 0o755) require.NoError(t, err) - defer os.RemoveAll(tmpDir) - testFile := filepath.Join(tmpDir, "test.go") err = os.WriteFile(testFile, []byte("package main"), 0o600) require.NoError(t, err) - pattern := regexp.MustCompile(`excluded\.go`) - opts := RunnerOptions{ - basePath: tmpDir, - patterns: []*regexp.Regexp{pattern}, - excludedPathCounter: map[*regexp.Regexp]int{pattern: 0}, - } + // Create a symlink in a completely different part of the tree. + symlink := filepath.Join(tmpDir, "somewhere", "symlink") - match, err := opts.MatchAnyPattern(testFile) + err = os.MkdirAll(filepath.Dir(symlink), 0o755) require.NoError(t, err) - assert.False(t, match, "Pattern should not match test.go") - assert.Equal(t, 0, opts.excludedPathCounter[pattern], "Counter should not be incremented") -} -func TestMatchAnyPattern_WithMatch(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "golangci-test-*") + err = os.Symlink(realDir, symlink) require.NoError(t, err) - defer os.RemoveAll(tmpDir) - testFile := filepath.Join(tmpDir, "generated.go") - err = os.WriteFile(testFile, []byte("package main"), 0o600) - require.NoError(t, err) - - pattern := regexp.MustCompile(`generated\.go`) - opts := RunnerOptions{ - basePath: tmpDir, - patterns: []*regexp.Regexp{pattern}, - excludedPathCounter: map[*regexp.Regexp]int{pattern: 0}, + cfg := &config.Config{ + Formatters: config.Formatters{ + Exclusions: config.FormatterExclusions{ + // Creates a pattern that matches files in the root directory only. + // This pattern will match `test.go" but not `../../../test.go` or similar broken paths. + Paths: []string{`^[^/\\]+\.go$`}, + }, + }, } + cfg.SetConfigDir(realDir) + + opts, err := NewRunnerOptions(cfg, false, false, false) + require.NoError(t, err) - match, err := opts.MatchAnyPattern(testFile) + match, err := opts.MatchAnyPattern(filepath.Join(symlink, "test.go")) require.NoError(t, err) - assert.True(t, match, "Pattern should match generated.go") - assert.Equal(t, 1, opts.excludedPathCounter[pattern], "Counter should be incremented") + + assert.True(t, match) + + require.NotEmpty(t, opts.patterns) + require.Len(t, opts.patterns, len(cfg.Formatters.Exclusions.Paths)) + + assert.Equal(t, 1, opts.excludedPathCounter[opts.patterns[0]]) } From f467aa2da3252899eaab6b8937044bd80af6d569 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 8 Nov 2025 07:34:46 +0100 Subject: [PATCH 3/4] review --- pkg/goformat/runner_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pkg/goformat/runner_test.go b/pkg/goformat/runner_test.go index 5ed4930cc562..a8f8a0e1ad50 100644 --- a/pkg/goformat/runner_test.go +++ b/pkg/goformat/runner_test.go @@ -3,12 +3,14 @@ package goformat import ( "os" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/golangci/golangci-lint/v2/pkg/config" + "github.com/golangci/golangci-lint/v2/pkg/fsutils" ) func TestRunnerOptions_MatchAnyPattern(t *testing.T) { @@ -117,8 +119,17 @@ func TestRunnerOptions_MatchAnyPattern_withSymlinks(t *testing.T) { }, }, } + cfg.SetConfigDir(realDir) + if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { + // IMPORTANT: On macOS, `tmpDir` might be` /var/..`. which is itself a symlink to `/private/var/...` + resolvedBasePath, errEval := fsutils.EvalSymlinks(realDir) + require.NoError(t, errEval) + + cfg.SetConfigDir(resolvedBasePath) + } + opts, err := NewRunnerOptions(cfg, false, false, false) require.NoError(t, err) From 6d6855d9749df7fa3d2636c9fd6e9a0a053c2da3 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 8 Nov 2025 07:59:59 +0100 Subject: [PATCH 4/4] review --- pkg/goformat/runner.go | 8 ++++++-- pkg/goformat/runner_test.go | 10 ---------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/pkg/goformat/runner.go b/pkg/goformat/runner.go index 3c65a79d8496..7c8ce913d724 100644 --- a/pkg/goformat/runner.go +++ b/pkg/goformat/runner.go @@ -223,8 +223,13 @@ func NewRunnerOptions(cfg *config.Config, diff, diffColored, stdin bool) (Runner return RunnerOptions{}, fmt.Errorf("get base path: %w", err) } + evaluatedBasePath, err := fsutils.EvalSymlinks(basePath) + if err != nil { + return RunnerOptions{}, fmt.Errorf("evaluate base path: %w", err) + } + opts := RunnerOptions{ - basePath: basePath, + basePath: evaluatedBasePath, generated: cfg.Formatters.Exclusions.Generated, diff: diff || diffColored, colors: diffColored, @@ -251,7 +256,6 @@ func (o RunnerOptions) MatchAnyPattern(path string) (bool, error) { return false, nil } - // The basePath is already resolved via `fsutils.Getwd()` in `NewRunnerOptions`. evaluatedPath, err := fsutils.EvalSymlinks(path) if err != nil { return false, err diff --git a/pkg/goformat/runner_test.go b/pkg/goformat/runner_test.go index a8f8a0e1ad50..40df44abd9dd 100644 --- a/pkg/goformat/runner_test.go +++ b/pkg/goformat/runner_test.go @@ -3,14 +3,12 @@ package goformat import ( "os" "path/filepath" - "runtime" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/golangci/golangci-lint/v2/pkg/config" - "github.com/golangci/golangci-lint/v2/pkg/fsutils" ) func TestRunnerOptions_MatchAnyPattern(t *testing.T) { @@ -122,14 +120,6 @@ func TestRunnerOptions_MatchAnyPattern_withSymlinks(t *testing.T) { cfg.SetConfigDir(realDir) - if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { - // IMPORTANT: On macOS, `tmpDir` might be` /var/..`. which is itself a symlink to `/private/var/...` - resolvedBasePath, errEval := fsutils.EvalSymlinks(realDir) - require.NoError(t, errEval) - - cfg.SetConfigDir(resolvedBasePath) - } - opts, err := NewRunnerOptions(cfg, false, false, false) require.NoError(t, err)