From f8739c990fba8f36bd656bc439a3e19a8bef3c7a Mon Sep 17 00:00:00 2001 From: wenchy Date: Wed, 18 Mar 2026 00:34:20 +0800 Subject: [PATCH] feat(xfs): improve IsSamePath with abs path and refactor util.go to use it - IsSamePath now resolves both paths to absolute paths before comparison, so mixed relative/absolute or dot-segment paths pointing to the same location are correctly considered equal - Add comprehensive test cases for IsSamePath covering abs/rel, dot-segments, symlink-free equivalence, and cross-platform slash normalization - Refactor prepareOutdir in util.go to use IsSamePath instead of manually calling filepath.Abs + xfs.CleanSlashPath, simplifying the logic --- internal/protogen/util.go | 18 ++-- internal/x/xfs/path.go | 16 ++- internal/x/xfs/path_test.go | 203 ++++++++++++++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 8 deletions(-) create mode 100644 internal/x/xfs/path_test.go diff --git a/internal/protogen/util.go b/internal/protogen/util.go index cbbba078..fee23251 100644 --- a/internal/protogen/util.go +++ b/internal/protogen/util.go @@ -24,15 +24,22 @@ func prepareOutdir(outdir string, importFiles []string, delExisted bool) error { } if existed && delExisted { // remove all *.proto file but not Imports - imports := make(map[string]bool) + // Collect all glob-expanded import paths. + var importPaths []string for _, filename := range importFiles { matches, err := filepath.Glob(filename) if err != nil { return xerrors.Wrapf(err, "failed to glob files in %s", filename) } - for _, match := range matches { - imports[xfs.CleanSlashPath(match)] = true + importPaths = append(importPaths, matches...) + } + isImported := func(fpath string) bool { + for _, imp := range importPaths { + if xfs.IsSamePath(fpath, imp) { + return true + } } + return false } files, err := os.ReadDir(outdir) if err != nil { @@ -43,11 +50,10 @@ func prepareOutdir(outdir string, importFiles []string, delExisted bool) error { continue } fpath := filepath.Join(outdir, file.Name()) - if imports[xfs.CleanSlashPath(fpath)] { + if isImported(fpath) { continue } - err := os.Remove(fpath) - if err != nil { + if err := os.Remove(fpath); err != nil { return xerrors.WrapKV(err) } log.Debugf("remove existed proto file: %s", fpath) diff --git a/internal/x/xfs/path.go b/internal/x/xfs/path.go index c8d7684b..5fdcae74 100644 --- a/internal/x/xfs/path.go +++ b/internal/x/xfs/path.go @@ -24,9 +24,21 @@ func CleanSlashPath(path string) string { return filepath.ToSlash(filepath.Clean(path)) } -// IsSamePath checks if two paths are same based on clean slash path. +// IsSamePath reports whether leftPath and rightPath refer to the same file-system +// location. Both paths are first resolved to absolute paths (relative paths are +// resolved against the current working directory) and then cleaned and +// normalized to forward-slash form before comparison, so mixed relative/absolute +// or dot-segment paths that point to the same location are considered equal. func IsSamePath(leftPath, rightPath string) bool { - return CleanSlashPath(leftPath) == CleanSlashPath(rightPath) + leftAbs, err := filepath.Abs(leftPath) + if err != nil { + return CleanSlashPath(leftPath) == CleanSlashPath(rightPath) + } + rightAbs, err := filepath.Abs(rightPath) + if err != nil { + return CleanSlashPath(leftPath) == CleanSlashPath(rightPath) + } + return CleanSlashPath(leftAbs) == CleanSlashPath(rightAbs) } // Rel returns relative clean slash path. diff --git a/internal/x/xfs/path_test.go b/internal/x/xfs/path_test.go new file mode 100644 index 00000000..a70c642d --- /dev/null +++ b/internal/x/xfs/path_test.go @@ -0,0 +1,203 @@ +package xfs + +import ( + "os" + "path/filepath" + "testing" +) + +func TestDir(t *testing.T) { + tests := []struct { + name string + path string + want string + }{ + {"file in dir", "foo/bar/baz.txt", "foo/bar"}, + {"file in root", "baz.txt", "."}, + {"nested dirs", "a/b/c/d.proto", "a/b/c"}, + // Note: backslash is only a path separator on Windows; + // on macOS/Linux it is treated as a literal character. + {"absolute path", "/tmp/foo/bar/baz.txt", "/tmp/foo/bar"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := Dir(tt.path); got != tt.want { + t.Errorf("Dir(%q) = %q, want %q", tt.path, got, tt.want) + } + }) + } +} + +func TestJoin(t *testing.T) { + tests := []struct { + name string + elems []string + want string + }{ + {"simple join", []string{"foo", "bar", "baz.txt"}, "foo/bar/baz.txt"}, + {"single element", []string{"foo"}, "foo"}, + {"with dot", []string{"foo", ".", "bar"}, "foo/bar"}, + {"with double dot", []string{"foo", "bar", "..", "baz"}, "foo/baz"}, + {"empty elements", []string{"", "foo", "", "bar"}, "foo/bar"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := Join(tt.elems...); got != tt.want { + t.Errorf("Join(%v) = %q, want %q", tt.elems, got, tt.want) + } + }) + } +} + +func TestCleanSlashPath(t *testing.T) { + tests := []struct { + name string + path string + want string + }{ + {"already clean", "foo/bar/baz", "foo/bar/baz"}, + {"trailing slash", "foo/bar/", "foo/bar"}, + {"double slash", "foo//bar", "foo/bar"}, + {"dot segment", "foo/./bar", "foo/bar"}, + {"double dot segment", "foo/bar/../baz", "foo/baz"}, + // Note: backslash is only a path separator on Windows; + // on macOS/Linux filepath.ToSlash does not convert it. + {"absolute path", "/tmp/foo/bar", "/tmp/foo/bar"}, + {"empty", "", "."}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := CleanSlashPath(tt.path); got != tt.want { + t.Errorf("CleanSlashPath(%q) = %q, want %q", tt.path, got, tt.want) + } + }) + } +} + +func TestIsSamePath(t *testing.T) { + // Get current working directory to construct absolute paths + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get cwd: %v", err) + } + + tests := []struct { + name string + leftPath string + rightPath string + want bool + }{ + { + name: "identical relative paths", + leftPath: "foo/bar/baz.txt", + rightPath: "foo/bar/baz.txt", + want: true, + }, + { + name: "different relative paths", + leftPath: "foo/bar/a.txt", + rightPath: "foo/bar/b.txt", + want: false, + }, + { + name: "relative vs absolute same file", + leftPath: "foo/bar/baz.txt", + rightPath: filepath.Join(cwd, "foo/bar/baz.txt"), + want: true, + }, + { + name: "relative vs absolute different file", + leftPath: "foo/bar/a.txt", + rightPath: filepath.Join(cwd, "foo/bar/b.txt"), + want: false, + }, + { + name: "both absolute same path", + leftPath: filepath.Join(cwd, "foo/bar/baz.txt"), + rightPath: filepath.Join(cwd, "foo/bar/baz.txt"), + want: true, + }, + { + name: "both absolute different path", + leftPath: filepath.Join(cwd, "foo/bar/a.txt"), + rightPath: filepath.Join(cwd, "foo/bar/b.txt"), + want: false, + }, + { + name: "relative with dot segments vs absolute", + leftPath: "foo/bar/../bar/baz.txt", + rightPath: filepath.Join(cwd, "foo/bar/baz.txt"), + want: true, + }, + { + name: "identical absolute paths", + leftPath: "/tmp/foo/bar.txt", + rightPath: "/tmp/foo/bar.txt", + want: true, + }, + { + name: "different absolute paths", + leftPath: "/tmp/foo/a.txt", + rightPath: "/tmp/foo/b.txt", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsSamePath(tt.leftPath, tt.rightPath); got != tt.want { + t.Errorf("IsSamePath(%q, %q) = %v, want %v", tt.leftPath, tt.rightPath, got, tt.want) + } + }) + } +} + +func TestRel(t *testing.T) { + tests := []struct { + name string + basepath string + targetpath string + want string + wantErr bool + }{ + { + name: "simple relative", + basepath: "foo/bar", + targetpath: "foo/bar/baz.txt", + want: "baz.txt", + wantErr: false, + }, + { + name: "go up one level", + basepath: "foo/bar/baz", + targetpath: "foo/bar/other.txt", + want: "../other.txt", + wantErr: false, + }, + { + name: "same directory", + basepath: "foo/bar", + targetpath: "foo/bar", + want: ".", + wantErr: false, + }, + { + name: "nested target", + basepath: "foo", + targetpath: "foo/a/b/c.txt", + want: "a/b/c.txt", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Rel(tt.basepath, tt.targetpath) + if (err != nil) != tt.wantErr { + t.Errorf("Rel(%q, %q) error = %v, wantErr %v", tt.basepath, tt.targetpath, err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("Rel(%q, %q) = %q, want %q", tt.basepath, tt.targetpath, got, tt.want) + } + }) + } +}