internal/span: always uppercase the drive letter for Windows

Drive letters are always case-insensitive, so we should standardize them
by always keeping them uppercase.

Updates golang/go#36904

Change-Id: I8de25b175790b01627f947600c1511edf38c316c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217080
Run-TryBot: Rebecca Stambler <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>
(cherry picked from commit 0725381)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217088
Reviewed-by: Michael Matloob <[email protected]>

Author: stamblerre

internal/span/uri.go

@@ -41,9 +41,12 @@ func filename(uri URI) (string, error) {
 	if u.Scheme != fileScheme {
 		return "", fmt.Errorf("only file URIs are supported, got %q from %q", u.Scheme, uri)
 	}
-	if isWindowsDriveURI(u.Path) {
-		u.Path = u.Path[1:]
+	// If the URI is a Windows URI, we trim the leading "/" and lowercase
+	// the drive letter, which will never be case sensitive.
+	if isWindowsDriveURIPath(u.Path) {
+		u.Path = strings.ToUpper(string(u.Path[1])) + u.Path[2:]
 	}
+
 	return u.Path, nil
 }
 
@@ -53,7 +56,16 @@ func NewURI(s string) URI {
 	if u, err := url.PathUnescape(s); err == nil {
 		s = u
 	}
+	// If a path has a scheme, it is already a URI.
+	// We only handle the file:// scheme.
 	if strings.HasPrefix(s, fileScheme+"://") {
+		// File URIs from Windows may have lowercase drive letters.
+		// Since drive letters are guaranteed to be case insensitive,
+		// we change them to uppercase to remain consistent.
+		// For example, file:///c:/x/y/z becomes file:///C:/x/y/z.
+		if i := len(fileScheme + "://"); isWindowsDriveURIPath(s[i:]) {
+			s = s[:i+1] + strings.ToUpper(string(s[i+1])) + s[i+2:]
+		}
 		return URI(s)
 	}
 	return FileURI(s)
@@ -117,7 +129,7 @@ func FileURI(path string) URI {
 	}
 	// Check the file path again, in case it became absolute.
 	if isWindowsDrivePath(path) {
-		path = "/" + path
+		path = "/" + strings.ToUpper(string(path[0])) + path[1:]
 	}
 	path = filepath.ToSlash(path)
 	u := url.URL{
@@ -133,18 +145,18 @@ func FileURI(path string) URI {
 
 // isWindowsDrivePath returns true if the file path is of the form used by
 // Windows. We check if the path begins with a drive letter, followed by a ":".
+// For example: C:/x/y/z.
 func isWindowsDrivePath(path string) bool {
-	if len(path) < 4 {
+	if len(path) < 3 {
 		return false
 	}
 	return unicode.IsLetter(rune(path[0])) && path[1] == ':'
 }
 
 // isWindowsDriveURI returns true if the file URI is of the format used by
 // Windows URIs. The url.Parse package does not specially handle Windows paths
-// (see https://golang.org/issue/6027). We check if the URI path has
-// a drive prefix (e.g. "/C:"). If so, we trim the leading "/".
-func isWindowsDriveURI(uri string) bool {
+// (see golang/go#6027). We check if the URI path has a drive prefix (e.g. "/C:").
+func isWindowsDriveURIPath(uri string) bool {
 	if len(uri) < 4 {
 		return false
 	}

internal/span/uri_test.go

@@ -2,10 +2,11 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+// +build !windows
+
 package span_test
 
 import (
-	"path/filepath"
 	"testing"
 
 	"golang.org/x/tools/internal/span"
@@ -16,36 +17,58 @@ import (
 // tests by using only forward slashes, assuming that the standard library
 // 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`,
-		`/path/to/dir`,
-		`/a/b/c/src/bob.go`,
-		`c:/Go/src/bob george/george/george.go`,
+	for _, test := range []struct {
+		path, wantFile string
+		wantURI        span.URI
+	}{
+		{
+			path:     ``,
+			wantFile: ``,
+			wantURI:  span.URI(""),
+		},
+		{
+			path:     `C:/Windows/System32`,
+			wantFile: `C:/Windows/System32`,
+			wantURI:  span.URI("file:///C:/Windows/System32"),
+		},
+		{
+			path:     `C:/Go/src/bob.go`,
+			wantFile: `C:/Go/src/bob.go`,
+			wantURI:  span.URI("file:///C:/Go/src/bob.go"),
+		},
+		{
+			path:     `c:/Go/src/bob.go`,
+			wantFile: `C:/Go/src/bob.go`,
+			wantURI:  span.URI("file:///C:/Go/src/bob.go"),
+		},
+		{
+			path:     `/path/to/dir`,
+			wantFile: `/path/to/dir`,
+			wantURI:  span.URI("file:///path/to/dir"),
+		},
+		{
+			path:     `/a/b/c/src/bob.go`,
+			wantFile: `/a/b/c/src/bob.go`,
+			wantURI:  span.URI("file:///a/b/c/src/bob.go"),
+		},
+		{
+			path:     `c:/Go/src/bob george/george/george.go`,
+			wantFile: `C:/Go/src/bob george/george/george.go`,
+			wantURI:  span.URI("file:///C:/Go/src/bob george/george/george.go"),
+		},
+		{
+			path:     `file:///c:/Go/src/bob george/george/george.go`,
+			wantFile: `C:/Go/src/bob george/george/george.go`,
+			wantURI:  span.URI("file:///C:/Go/src/bob george/george/george.go"),
+		},
 	} {
-		testPath := filepath.FromSlash(test)
-		expectPath := testPath
-		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)
+		got := span.NewURI(test.path)
+		if got != test.wantURI {
+			t.Errorf("ToURI: got %s, expected %s", got, test.wantURI)
 		}
-		filename := uri.Filename()
-		if expectPath != filename {
-			t.Errorf("Filename: expected %s, got %s", expectPath, filename)
+		gotFilename := got.Filename()
+		if gotFilename != test.wantFile {
+			t.Errorf("Filename: got %s, expected %s", gotFilename, test.wantFile)
 		}
 	}
 }

internal/span/uri_windows_test.go

@@ -0,0 +1,73 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build windows
+
+package span_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/internal/span"
+)
+
+// TestURI tests the conversion between URIs and filenames. The test cases
+// include Windows-style URIs and filepaths, but we avoid having OS-specific
+// tests by using only forward slashes, assuming that the standard library
+// functions filepath.ToSlash and filepath.FromSlash do not need testing.
+func TestURI(t *testing.T) {
+	for _, test := range []struct {
+		path, wantFile string
+		wantURI        span.URI
+	}{
+		{
+			path:     ``,
+			wantFile: ``,
+			wantURI:  span.URI(""),
+		},
+		{
+			path:     `C:\Windows\System32`,
+			wantFile: `C:\Windows\System32`,
+			wantURI:  span.URI("file:///C:/Windows/System32"),
+		},
+		{
+			path:     `C:\Go\src\bob.go`,
+			wantFile: `C:\Go\src\bob.go`,
+			wantURI:  span.URI("file:///C:/Go/src/bob.go"),
+		},
+		{
+			path:     `c:\Go\src\bob.go`,
+			wantFile: `C:\Go\src\bob.go`,
+			wantURI:  span.URI("file:///C:/Go/src/bob.go"),
+		},
+		{
+			path:     `\path\to\dir`,
+			wantFile: `C:\path\to\dir`,
+			wantURI:  span.URI("file:///C:/path/to/dir"),
+		},
+		{
+			path:     `\a\b\c\src\bob.go`,
+			wantFile: `C:\a\b\c\src\bob.go`,
+			wantURI:  span.URI("file:///C:/a/b/c/src/bob.go"),
+		},
+		{
+			path:     `c:\Go\src\bob george\george\george.go`,
+			wantFile: `C:\Go\src\bob george\george\george.go`,
+			wantURI:  span.URI("file:///C:/Go/src/bob george/george/george.go"),
+		},
+		{
+			path:     `file:///c:/Go/src/bob george/george/george.go`,
+			wantFile: `C:\Go\src\bob george\george\george.go`,
+			wantURI:  span.URI("file:///C:/Go/src/bob george/george/george.go"),
+		},
+	} {
+		got := span.NewURI(test.path)
+		if got != test.wantURI {
+			t.Errorf("ToURI: got %s, expected %s", got, test.wantURI)
+		}
+		if got.Filename() != test.wantFile {
+			t.Errorf("Filename: got %s, expected %s", got.Filename(), test.wantFile)
+		}
+	}
+}