internal/span: handle empty paths correctly

Change-Id: I3f411c5e27bcada26a756485a3b9de3514ac7955
Reviewed-on: https://go-review.googlesource.com/c/tools/+/181677
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

internal/span/uri.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -30,6 +31,9 @@ func (uri URI) Filename() string {
 }
 
 func filename(uri URI) (string, error) {
+	if uri == "" {
+		return "", nil
+	}
 	u, err := url.ParseRequestURI(string(uri))
 	if err != nil {
 		return "", err
@@ -56,28 +60,49 @@ func NewURI(s string) URI {
 }
 
 func CompareURI(a, b URI) int {
-	if a == b {
+	if equalURI(a, b) {
 		return 0
 	}
+	if a < b {
+		return -1
+	}
+	return 1
+}
+
+func equalURI(a, b URI) bool {
+	if a == b {
+		return true
+	}
 	// If we have the same URI basename, we may still have the same file URIs.
-	fa := a.Filename()
+	if !strings.EqualFold(path.Base(string(a)), path.Base(string(b))) {
-	fb := b.Filename()
+		return false
-	if strings.EqualFold(filepath.Base(fa), filepath.Base(fb)) {
+	}
+	fa, err := filename(a)
+	if err != nil {
+		return false
+	}
+	fb, err := filename(b)
+	if err != nil {
+		return false
+	}
 	// Stat the files to check if they are equal.
-		if infoa, err := os.Stat(fa); err == nil {
+	infoa, err := os.Stat(filepath.FromSlash(fa))
-			if infob, err := os.Stat(fb); err == nil {
+	if err != nil {
-				if os.SameFile(infoa, infob) {
+		return false
-					return 0
 	}
+	infob, err := os.Stat(filepath.FromSlash(fb))
+	if err != nil {
+		return false
 	}
-		}
+	return os.SameFile(infoa, infob)
-	}
-	return strings.Compare(fa, fb)
 }
 
 // FileURI returns a span URI for the supplied file path.
 // It will always have the file scheme.
 func FileURI(path string) URI {
+	if path == "" {
+		return ""
+	}
 	// Handle standard library paths that contain the literal "$GOROOT".
 	// TODO(rstambler): The go/packages API should allow one to determine a user's $GOROOT.
 	const prefix = "$GOROOT"

internal/span/uri_test.go

@@ -17,6 +17,7 @@ import (
 // functions filepath.ToSlash and filepath.FromSlash do not need testing.
 func TestURI(t *testing.T) {
 	for _, test := range []string{
+		``,
 		`C:/Windows/System32`,
 		`C:/Go/src/bob.go`,
 		`c:/Go/src/bob.go`,
@@ -25,16 +26,18 @@ func TestURI(t *testing.T) {
 	} {
 		testPath := filepath.FromSlash(test)
 		expectPath := testPath
-		if test[0] == '/' {
+		if len(test) > 0 && test[0] == '/' {
 			if abs, err := filepath.Abs(expectPath); err == nil {
 				expectPath = abs
 			}
 		}
 		expectURI := filepath.ToSlash(expectPath)
+		if len(expectURI) > 0 {
 			if expectURI[0] != '/' {
 				expectURI = "/" + expectURI
 			}
 			expectURI = "file://" + expectURI
+		}
 		uri := span.FileURI(testPath)
 		if expectURI != string(uri) {
 			t.Errorf("ToURI: expected %s, got %s", expectURI, uri)