Skip to content

Commit 4ec7afc

Browse files
committed
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 <david.chandler@solo.io>
1 parent b92f577 commit 4ec7afc

File tree

2 files changed

+140
-1
lines changed

2 files changed

+140
-1
lines changed

pkg/goformat/runner.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,15 @@ func (o RunnerOptions) MatchAnyPattern(path string) (bool, error) {
251251
return false, nil
252252
}
253253

254-
rel, err := filepath.Rel(o.basePath, path)
254+
// Resolve symlinks in the file path to ensure filepath.Rel works correctly
255+
// when running inside symlinked directories. The basePath is already resolved
256+
// via fsutils.Getwd() in NewRunnerOptions.
257+
evalPath, err := fsutils.EvalSymlinks(path)
258+
if err != nil {
259+
return false, err
260+
}
261+
262+
rel, err := filepath.Rel(o.basePath, evalPath)
255263
if err != nil {
256264
return false, err
257265
}

pkg/goformat/runner_test.go

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
package goformat
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"regexp"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
12+
"github.com/golangci/golangci-lint/v2/pkg/fsutils"
13+
)
14+
15+
func TestMatchAnyPattern_WithSymlinks(t *testing.T) {
16+
// Create a temporary directory structure
17+
tmpDir, err := os.MkdirTemp("", "golangci-symlink-test-*")
18+
require.NoError(t, err)
19+
defer os.RemoveAll(tmpDir)
20+
21+
// Create two separate directories at the same level
22+
realDir := filepath.Join(tmpDir, "real_project")
23+
err = os.MkdirAll(realDir, 0o755)
24+
require.NoError(t, err)
25+
26+
testFile := filepath.Join(realDir, "test.go")
27+
err = os.WriteFile(testFile, []byte("package main"), 0o600)
28+
require.NoError(t, err)
29+
30+
// Create a symlink in a completely different part of the tree
31+
symlinkParent := filepath.Join(tmpDir, "symlinks")
32+
err = os.MkdirAll(symlinkParent, 0o755)
33+
require.NoError(t, err)
34+
35+
symlinkDir := filepath.Join(symlinkParent, "project_link")
36+
err = os.Symlink(realDir, symlinkDir)
37+
require.NoError(t, err)
38+
39+
// Simulate the actual scenario:
40+
// - basePath is the resolved real path (as fsutils.Getwd does when you're in a symlinked dir)
41+
// - filepath.Walk from the symlink directory provides unresolved paths
42+
// IMPORTANT: On macOS, tmpDir might be /var/... which is itself a symlink to /private/var/...
43+
// So we need to evaluate basePath as well to simulate what fsutils.Getwd() does
44+
resolvedBasePath, err := fsutils.EvalSymlinks(realDir)
45+
require.NoError(t, err)
46+
47+
// Create RunnerOptions with a pattern that matches files in the root directory only
48+
// This pattern will match "test.go" but NOT "../../../test.go" or similar broken paths
49+
pattern := regexp.MustCompile(`^[^/\\]+\.go$`)
50+
opts := RunnerOptions{
51+
basePath: resolvedBasePath, // Resolved path from Getwd
52+
patterns: []*regexp.Regexp{pattern},
53+
excludedPathCounter: map[*regexp.Regexp]int{pattern: 0},
54+
}
55+
56+
// filepath.Walk would provide the path through the symlink
57+
// When you cd into a symlink and run the command, Walk uses the symlink path
58+
unresolvedFile := filepath.Join(symlinkDir, "test.go")
59+
60+
// The issue: When basePath is resolved (e.g., /private/var/...)
61+
// but the file path from filepath.Walk is unresolved (e.g., /var/...),
62+
// filepath.Rel produces an incorrect relative path with many ../ components
63+
// like "../../../../var/.../test.go" which won't match the pattern ^[^/\\]+\.go$
64+
//
65+
// The fix: EvalSymlinks on the file path before calling filepath.Rel
66+
// ensures both paths are in their canonical form, producing "test.go"
67+
// which correctly matches the pattern.
68+
69+
match, err := opts.MatchAnyPattern(unresolvedFile)
70+
71+
// With the fix, pattern matching should work correctly
72+
require.NoError(t, err, "Should not error when matching pattern with symlinks")
73+
assert.True(t, match, "Pattern should match test.go when accessed through symlink")
74+
assert.Equal(t, 1, opts.excludedPathCounter[pattern], "Counter should be incremented")
75+
}
76+
77+
func TestMatchAnyPattern_NoPatterns(t *testing.T) {
78+
opts := RunnerOptions{
79+
basePath: "/tmp",
80+
patterns: []*regexp.Regexp{},
81+
excludedPathCounter: map[*regexp.Regexp]int{},
82+
}
83+
84+
match, err := opts.MatchAnyPattern("/tmp/test.go")
85+
require.NoError(t, err)
86+
assert.False(t, match, "Should not match when no patterns are defined")
87+
}
88+
89+
func TestMatchAnyPattern_NoMatch(t *testing.T) {
90+
tmpDir, err := os.MkdirTemp("", "golangci-test-*")
91+
require.NoError(t, err)
92+
defer os.RemoveAll(tmpDir)
93+
94+
testFile := filepath.Join(tmpDir, "test.go")
95+
err = os.WriteFile(testFile, []byte("package main"), 0o600)
96+
require.NoError(t, err)
97+
98+
pattern := regexp.MustCompile(`excluded\.go`)
99+
opts := RunnerOptions{
100+
basePath: tmpDir,
101+
patterns: []*regexp.Regexp{pattern},
102+
excludedPathCounter: map[*regexp.Regexp]int{pattern: 0},
103+
}
104+
105+
match, err := opts.MatchAnyPattern(testFile)
106+
require.NoError(t, err)
107+
assert.False(t, match, "Pattern should not match test.go")
108+
assert.Equal(t, 0, opts.excludedPathCounter[pattern], "Counter should not be incremented")
109+
}
110+
111+
func TestMatchAnyPattern_WithMatch(t *testing.T) {
112+
tmpDir, err := os.MkdirTemp("", "golangci-test-*")
113+
require.NoError(t, err)
114+
defer os.RemoveAll(tmpDir)
115+
116+
testFile := filepath.Join(tmpDir, "generated.go")
117+
err = os.WriteFile(testFile, []byte("package main"), 0o600)
118+
require.NoError(t, err)
119+
120+
pattern := regexp.MustCompile(`generated\.go`)
121+
opts := RunnerOptions{
122+
basePath: tmpDir,
123+
patterns: []*regexp.Regexp{pattern},
124+
excludedPathCounter: map[*regexp.Regexp]int{pattern: 0},
125+
}
126+
127+
match, err := opts.MatchAnyPattern(testFile)
128+
require.NoError(t, err)
129+
assert.True(t, match, "Pattern should match generated.go")
130+
assert.Equal(t, 1, opts.excludedPathCounter[pattern], "Counter should be incremented")
131+
}

0 commit comments

Comments
 (0)