review

Author: ldez

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
 	}

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]])
 }