From db32b8c7e4804e148d7571ad637d152e42e9bd3a Mon Sep 17 00:00:00 2001 From: jojosenthusiast Date: Sun, 28 Jun 2026 18:10:13 -0600 Subject: [PATCH] fix(stats): handle windows file project roots --- bindings/go/scip/project_root.go | 45 +++++++++++++++++++++++++++ bindings/go/scip/project_root_test.go | 42 +++++++++++++++++++++++++ bindings/go/scip/testutil/format.go | 6 ++-- cmd/scip/stats.go | 8 ++--- cmd/scip/stats_test.go | 42 +++++++++++++++++++++++++ 5 files changed, 135 insertions(+), 8 deletions(-) create mode 100644 bindings/go/scip/project_root.go create mode 100644 bindings/go/scip/project_root_test.go create mode 100644 cmd/scip/stats_test.go diff --git a/bindings/go/scip/project_root.go b/bindings/go/scip/project_root.go new file mode 100644 index 00000000..26d193eb --- /dev/null +++ b/bindings/go/scip/project_root.go @@ -0,0 +1,45 @@ +package scip + +import "net/url" + +// ProjectRootToLocalPath converts an Index.Metadata.ProjectRoot value into a +// local filesystem path. +// +// It handles the Windows form "file://X:\..." (where X is a drive letter) +// which net/url.Parse rejects with `invalid port ":\..." after host`, because +// it parses "X:" as a host:port pattern. See issue #282. +// +// For all other inputs it preserves the previous behavior of using +// url.Parse(rawProjectRoot).Path. +func ProjectRootToLocalPath(rawProjectRoot string) (string, error) { + if isWindowsFileURIWithDriveLetter(rawProjectRoot) { + // Strip the "file://" prefix; the remainder is the Windows path. + // ponytail: only the "file://X:..." form is handled here; the + // canonical "file:///X:/..." form parses fine via url.Parse and is + // out of scope for this fix. + return rawProjectRoot[len("file://"):], nil + } + u, err := url.Parse(rawProjectRoot) + if err != nil { + return "", err + } + return u.Path, nil +} + +// isWindowsFileURIWithDriveLetter reports whether s matches "file://X:..." +// where X is an ASCII letter and the position after the scheme is NOT a third +// slash (which would indicate the canonical "file:///..." form). +func isWindowsFileURIWithDriveLetter(s string) bool { + const prefix = "file://" + if len(s) < len(prefix)+2 { + return false + } + if s[:len(prefix)] != prefix { + return false + } + c := s[len(prefix)] + if !(c >= 'a' && c <= 'z') && !(c >= 'A' && c <= 'Z') { + return false + } + return s[len(prefix)+1] == ':' +} diff --git a/bindings/go/scip/project_root_test.go b/bindings/go/scip/project_root_test.go new file mode 100644 index 00000000..da27eba6 --- /dev/null +++ b/bindings/go/scip/project_root_test.go @@ -0,0 +1,42 @@ +package scip + +import "testing" + +func TestProjectRootToLocalPath(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + // Regression for https://github.com/scip-code/scip/issues/282 — + // Windows ProjectRoot of the form "file://X:\..." used to fail + // url.Parse with "invalid port" because "X:" looked like host:port. + { + name: "windows file uri with drive letter and backslashes", + in: `file://D:\dev\project`, + want: `D:\dev\project`, + }, + // Unix/macOS forms must keep their existing behavior unchanged. + { + name: "unix file uri", + in: "file:///test/project", + want: "/test/project", + }, + { + name: "unix file uri single slash", + in: "file:/root", + want: "/root", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := ProjectRootToLocalPath(tc.in) + if err != nil { + t.Fatalf("ProjectRootToLocalPath(%q) unexpected error: %v", tc.in, err) + } + if got != tc.want { + t.Fatalf("ProjectRootToLocalPath(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} diff --git a/bindings/go/scip/testutil/format.go b/bindings/go/scip/testutil/format.go index 9396490b..4faf1594 100644 --- a/bindings/go/scip/testutil/format.go +++ b/bindings/go/scip/testutil/format.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "log" - "net/url" "os" "path/filepath" "sort" @@ -22,18 +21,17 @@ func FormatSnapshots( customProjectRoot string, ) ([]*scip.SourceFile, error) { var result []*scip.SourceFile - projectRootUrl, err := url.Parse(index.Metadata.ProjectRoot) + localSourcesRoot, err := scip.ProjectRootToLocalPath(index.Metadata.ProjectRoot) if err != nil { return nil, err } - localSourcesRoot := projectRootUrl.Path if customProjectRoot != "" { localSourcesRoot = customProjectRoot } else if _, err := os.Stat(localSourcesRoot); errors.Is(err, os.ErrNotExist) { cwd, _ := os.Getwd() log.Printf("Project root [%s] doesn't exist, using current working directory [%s] instead. "+ - "To override this behaviour, use --project-root= option directly", projectRootUrl.Path, cwd) + "To override this behaviour, use --project-root= option directly", localSourcesRoot, cwd) localSourcesRoot = cwd } diff --git a/cmd/scip/stats.go b/cmd/scip/stats.go index 789ad5ed..5ff16998 100644 --- a/cmd/scip/stats.go +++ b/cmd/scip/stats.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "log" - "net/url" "os" "path/filepath" @@ -153,18 +152,19 @@ func percentile(buf []float64, percent float64) int32 { func countLinesOfCode(index *scip.Index, customProjectRoot string) (*gocloc.Result, error) { var localSource string - root, err := url.Parse(index.Metadata.ProjectRoot) + rootPath, err := scip.ProjectRootToLocalPath(index.Metadata.ProjectRoot) if err != nil { return nil, fmt.Errorf("failed to parse Index.Metadata.ProjectRoot as a URI %s: %w", index.Metadata.ProjectRoot, err) } if customProjectRoot != "" { localSource = customProjectRoot } else { - _, err := os.Stat(root.Path) + localSource = rootPath + _, err := os.Stat(rootPath) if errors.Is(err, os.ErrNotExist) { cwd, _ := os.Getwd() log.Printf("Project root [%s] doesn't exist, using current working directory [%s] instead. "+ - "To override this behaviour, specify --project-root= option", root.Path, cwd) + "To override this behaviour, specify --project-root= option", rootPath, cwd) localSource = cwd } else if err != nil { return nil, err diff --git a/cmd/scip/stats_test.go b/cmd/scip/stats_test.go new file mode 100644 index 00000000..4444d5c1 --- /dev/null +++ b/cmd/scip/stats_test.go @@ -0,0 +1,42 @@ +package main + +import ( + "testing" + + "github.com/scip-code/scip/bindings/go/scip" +) + +// Regression for https://github.com/scip-code/scip/issues/282. +// +// Before the fix, countLinesOfCode called url.Parse on Index.Metadata.ProjectRoot +// directly. A Windows ProjectRoot of the form "file://X:\..." was rejected by +// url.Parse with `invalid port ":\..." after host` (it parses "X:" as +// host:port), so `scip stats` errored before reading any document. +// +// Exercising countLinesOfCode keeps the externally visible seam covered: +// scip stats -> statsMain -> countStatistics -> countLinesOfCode. +func TestCountLinesOfCode_WindowsFileURI(t *testing.T) { + idx := &scip.Index{ + Metadata: &scip.Metadata{ProjectRoot: `file://D:\dev\scip-issue-282-does-not-exist`}, + } + if _, err := countLinesOfCode(idx, ""); err != nil { + t.Fatalf("countLinesOfCode returned error for Windows file URI: %v", err) + } +} + +// Regression for the localSource bug surfaced in the #282 review: when +// customProjectRoot is empty and the parsed rootPath exists on disk, +// localSource was left empty and the subsequent os.Stat("") returned +// `stat : no such file or directory`. This drives the "rootPath exists" +// branch on both platforms by using t.TempDir() as the ProjectRoot: +// - POSIX: "file:///tmp/xxx" is parsed by url.Parse. +// - Windows: "file://C:\Users\..." is handled by ProjectRootToLocalPath. +func TestCountLinesOfCode_RootPathExists(t *testing.T) { + tmp := t.TempDir() + idx := &scip.Index{ + Metadata: &scip.Metadata{ProjectRoot: "file://" + tmp}, + } + if _, err := countLinesOfCode(idx, ""); err != nil { + t.Fatalf("countLinesOfCode returned error when rootPath exists: %v", err) + } +}