fix(cli): unexpected lint warning when providing multiple files

cmd/api-linter/cli.go

@@ -19,6 +19,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"sync"
 
@@ -79,7 +80,7 @@ func newCli(args []string) *cli {
 	fs.StringArrayVar(&protoDescFlag, "descriptor-set-in", nil, "The file containing a FileDescriptorSet for searching proto imports.\nMay be specified multiple times.")
 	fs.StringArrayVar(&ruleEnableFlag, "enable-rule", nil, "Enable a rule with the given name.\nMay be specified multiple times.")
 	fs.StringArrayVar(&ruleDisableFlag, "disable-rule", nil, "Disable a rule with the given name.\nMay be specified multiple times.")
-	fs.BoolVar(&listRulesFlag, "list-rules", false, "Print the rules and exit.  Honors the output-format flag.")
+	fs.BoolVar(&listRulesFlag, "list-rules", false, "Print the rules and exit. Honors the output-format flag.")
 	fs.BoolVar(&debugFlag, "debug", false, "Run in debug mode. Panics will print stack.")
 	fs.BoolVar(&ignoreCommentDisablesFlag, "ignore-comment-disables", false, "If set to true, disable comments will be ignored.\nThis is helpful when strict enforcement of AIPs are necessary and\nproto definitions should not be able to disable checks.")
 
@@ -150,9 +151,12 @@ func (c *cli) lint(rules lint.RuleRegistry, configs lint.Configs) error {
 	}
 	var errorsWithPos []protoparse.ErrorWithPos
 	var lock sync.Mutex
+
+	imports := resolveImports(c.ProtoImportPaths)
+
 	// Parse proto files into `protoreflect` file descriptors.
 	p := protoparse.Parser{
-		ImportPaths:           append(c.ProtoImportPaths, "."),
+		ImportPaths:           imports,
 		IncludeSourceCodeInfo: true,
 		LookupImport:          lookupImport,
 		ErrorReporter: func(errorWithPos protoparse.ErrorWithPos) error {
@@ -165,21 +169,7 @@ func (c *cli) lint(rules lint.RuleRegistry, configs lint.Configs) error {
 		},
 	}
 	// Resolve file absolute paths to relative ones.
-	// Using supplied import paths first.
-	protoFiles := c.ProtoFiles
-	if len(c.ProtoImportPaths) > 0 {
-		protoFiles, err = protoparse.ResolveFilenames(c.ProtoImportPaths, c.ProtoFiles...)
-		if err != nil {
-			return err
-		}
-	}
-	// Then resolve again against ".", the local directory.
-	// This is necessary because ResolveFilenames won't resolve a path if it
-	// relative to *at least one* of the given import paths, which can result
-	// in duplicate file parsing and compilation errors, as seen in #1465 and
-	// #1471. So we resolve against local (default) and flag specified import
-	// paths separately.
-	protoFiles, err = protoparse.ResolveFilenames([]string{"."}, protoFiles...)
+	protoFiles, err := protoparse.ResolveFilenames(p.ImportPaths, c.ProtoFiles...)
 	if err != nil {
 		return err
 	}
@@ -305,3 +295,74 @@ func getOutputFormatFunc(formatType string) formatFunc {
 	}
 	return yaml.Marshal
 }
+
+func resolveImports(protoImportPaths []string) []string {
+	// If no import paths are provided, default to the current directory.
+	if len(protoImportPaths) == 0 {
+		return []string{"."}
+	}
+
+	// Get the absolute path of the current working directory.
+	cwd, err := os.Getwd()
+	if err != nil {
+		// Fallback: If we can't get CWD, return only the provided paths and "."
+		seen := make(map[string]struct{})
+		var result []string
+		result = append(result, ".") // Always include "."
+		seen["."] = struct{}{}
+		for _, p := range protoImportPaths {
+			if _, found := seen[p]; !found {
+				seen[p] = struct{}{}
+				result = append(result, p)
+			}
+		}
+		return result
+	}
+
+	// Resolve the canonical path for the current working directory.
+	// This helps with symlinks (e.g., /var vs /private/var on macOS).
+	evaluatedCwd, err := filepath.EvalSymlinks(cwd)
+	if err != nil {
+		// Fallback to Clean if EvalSymlinks fails (e.g., path does not exist)
+		evaluatedCwd = filepath.Clean(cwd)
+	}
+
+	// Initialize resolvedImports with "." and track its canonical absolute path.
+	resolvedImports := []string{"."}
+	seenAbsolutePaths := make(map[string]struct{})
+	seenAbsolutePaths[evaluatedCwd] = struct{}{} // Mark canonical CWD as seen
+
+	for _, p := range protoImportPaths {
+		absPath, err := filepath.Abs(p)
+		if err != nil {
+			// If we can't get the absolute path, treat it as an external path
+			// and add it if not already seen (by its original string form).
+			if _, found := seenAbsolutePaths[p]; !found {
+				seenAbsolutePaths[p] = struct{}{}
+				resolvedImports = append(resolvedImports, p)
+			}
+			continue
+		}
+
+		// Resolve the canonical path for the current import path.
+		evaluatedAbsPath, err := filepath.EvalSymlinks(absPath)
+		if err != nil {
+			// Fallback to Clean if EvalSymlinks fails
+			evaluatedAbsPath = filepath.Clean(absPath)
+		}
+
+		// Check if the current import path's canonical form is the CWD's canonical form
+		// or a subdirectory of it. If so, it's covered by ".", so we skip it.
+		if evaluatedAbsPath == evaluatedCwd || strings.HasPrefix(evaluatedAbsPath, evaluatedCwd+string(os.PathSeparator)) {
+			continue
+		}
+
+		// Add the original path if its canonical absolute form has not been seen before.
+		if _, found := seenAbsolutePaths[evaluatedAbsPath]; !found {
+			seenAbsolutePaths[evaluatedAbsPath] = struct{}{}
+			resolvedImports = append(resolvedImports, p)
+		}
+	}
+
+	return resolvedImports
+}

cmd/api-linter/cli_test.go

@@ -1,6 +1,9 @@
 package main
 
 import (
+	"os"
+	"path/filepath"
+	"sort"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -54,3 +57,141 @@ func TestNewCli(t *testing.T) {
 		})
 	}
 }
+
+func TestResolveImports(t *testing.T) {
+	// Save the original working directory and restore it at the end of the test.
+	originalCWD, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Failed to get original working directory: %v", err)
+	}
+	defer func() {
+		if err := os.Chdir(originalCWD); err != nil {
+			t.Errorf("Failed to restore original working directory: %v", err)
+		}
+	}()
+
+	tests := []struct {
+		name             string
+		protoImportPaths []string
+		setupTempDir     func(t *testing.T) string // Function to set up a temporary directory
+		want             []string
+	}{
+		{
+			name:             "NoProtoImportPaths",
+			protoImportPaths: []string{},
+			want:             []string{"."},
+		},
+		{
+			name:             "ExplicitDot",
+			protoImportPaths: []string{"."},
+			want:             []string{"."},
+		},
+		{
+			name:             "SubdirectoryOfCWD",
+			protoImportPaths: []string{"test_dir"},
+			setupTempDir: func(t *testing.T) string {
+				tmpDir := t.TempDir()
+				if err := os.Mkdir(filepath.Join(tmpDir, "test_dir"), 0755); err != nil {
+					t.Fatalf("Failed to create temp subdirectory: %v", err)
+				}
+				return tmpDir
+			},
+			want: []string{"."}, // "test_dir" should be covered by "."
+		},
+		{
+			name:             "SubdirectoryOfCWDWithDot",
+			protoImportPaths: []string{".", "test_dir"},
+			setupTempDir: func(t *testing.T) string {
+				tmpDir := t.TempDir()
+				if err := os.Mkdir(filepath.Join(tmpDir, "test_dir"), 0755); err != nil {
+					t.Fatalf("Failed to create temp subdirectory: %v", err)
+				}
+				return tmpDir
+			},
+			want: []string{"."},
+		},
+		{
+			name:             "ExternalAbsolutePath",
+			protoImportPaths: []string{"/tmp/external_proto"},
+			setupTempDir: func(t *testing.T) string {
+				tmpDir := t.TempDir()
+				// Create a dummy external directory to ensure filepath.Abs works
+				if err := os.MkdirAll("/tmp/external_proto", 0755); err != nil {
+					t.Fatalf("Failed to create external temp directory: %v", err)
+				}
+				t.Cleanup(func() { os.RemoveAll("/tmp/external_proto") })
+				return tmpDir
+			},
+			want: []string{".", "/tmp/external_proto"},
+		},
+		{
+			name:             "MixedPaths",
+			protoImportPaths: []string{"./relative_dir", "/tmp/external_dir", "test_dir"},
+			setupTempDir: func(t *testing.T) string {
+				tmpDir := t.TempDir()
+				if err := os.Mkdir(filepath.Join(tmpDir, "relative_dir"), 0755); err != nil {
+					t.Fatalf("Failed to create relative_dir: %v", err)
+				}
+				if err := os.Mkdir(filepath.Join(tmpDir, "test_dir"), 0755); err != nil {
+					t.Fatalf("Failed to create test_dir: %v", err)
+				}
+				if err := os.MkdirAll("/tmp/external_dir", 0755); err != nil {
+					t.Fatalf("Failed to create external_dir: %v", err)
+				}
+				t.Cleanup(func() { os.RemoveAll("/tmp/external_dir") })
+				return tmpDir
+			},
+			want: []string{".", "/tmp/external_dir"},
+		},
+		{
+			name:             "NonExistentRelativePath",
+			protoImportPaths: []string{"non_existent_dir"},
+			setupTempDir: func(t *testing.T) string {
+				return t.TempDir() // No special setup needed, just a temp dir
+			},
+			want: []string{"."}, // Should still resolve to just "." as non_existent_dir is relative to CWD
+		},
+		{
+			name:             "CurrentDirAsAbsolutePath",
+			protoImportPaths: []string{}, // Will be set dynamically inside t.Run
+			setupTempDir: func(t *testing.T) string {
+				return t.TempDir() // No special setup needed, just a temp dir
+			},
+			want: []string{"."},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			// Set up a temporary directory for the test if a setup function is provided.
+			// Change to this directory.
+			var currentTestDir string
+			if test.setupTempDir != nil {
+				currentTestDir = test.setupTempDir(t)
+			} else {
+				currentTestDir = t.TempDir()
+			}
+
+			// Dynamically set protoImportPaths for this specific test case
+			var actualProtoImportPaths []string
+			if test.name == "CurrentDirAsAbsolutePath" {
+				actualProtoImportPaths = []string{currentTestDir}
+			} else {
+				actualProtoImportPaths = test.protoImportPaths
+			}
+
+			if err := os.Chdir(currentTestDir); err != nil {
+
+				t.Fatalf("Failed to change directory to %q: %v", currentTestDir, err)
+			}
+
+			got := resolveImports(actualProtoImportPaths)
+			sort.Strings(got)
+			sort.Strings(test.want)
+
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("resolveImports() mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}

cmd/api-linter/integration_test.go

@@ -182,9 +182,6 @@ func TestMultipleFilesFromParentDir(t *testing.T) {
 	// This test addresses a previously found bug:
 	// https://github.com/googleapis/api-linter/issues/1465
 
-	// Skipping until Issue # 1465 is addressed
-	t.Skip("Skipping until Issue # 1465 is addressed")
-
 	projDir, err := os.MkdirTemp("", "proj")
 	if err != nil {
 		t.Fatal(err)