internal/lsp/source: avoid having build tagged files for uri

Create helper functions for the exported URI functions to test
the logic that isn't OS-specific (filepath.{To,From}Slash is the OS-specific part).
Also add helpers to determine is a file or URI path is Windows-specific.

Change-Id: I6ba5119424ad5edcd59b946276e4268b2525505f
Reviewed-on: https://go-review.googlesource.com/c/153867
Reviewed-by: Ian Cottrell <iancottrell@google.com>

internal/lsp/lsp_test.go

@@ -215,6 +215,9 @@ func (c completions) test(t *testing.T, exported *packagestest.Exported, s *serv
 				},
 			},
 		})
+		if err != nil {
+			t.Fatal(err)
+		}
 		var got []protocol.CompletionItem
 		for _, item := range list.Items {
 			// Skip all types with no details (builtin types).

internal/lsp/source/uri.go

@@ -10,36 +10,80 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+	"unicode"
 )
 
-// URI represents the full uri for a file.
+const fileScheme = "file"
+
+// URI represents the full URI for a file.
 type URI string
 
 // Filename gets the file path for the URI.
 // It will return an error if the uri is not valid, or if the URI was not
 // a file URI
 func (uri URI) Filename() (string, error) {
-	s := string(uri)
+	filename, err := filename(uri)
-	if !strings.HasPrefix(s, fileSchemePrefix) {
-		return "", fmt.Errorf("only file URI's are supported, got %v", uri)
-	}
-	s = s[len(fileSchemePrefix):]
-	s, err := url.PathUnescape(s)
 	if err != nil {
-		return s, err
+		return "", err
 	}
-	s = filepath.FromSlash(s)
+	return filepath.FromSlash(filename), nil
-	return s, nil
+}
+
+func filename(uri URI) (string, error) {
+	u, err := url.ParseRequestURI(string(uri))
+	if err != nil {
+		return "", err
+	}
+	if u.Scheme != fileScheme {
+		return "", fmt.Errorf("only file URIs are supported, got %v", u.Scheme)
+	}
+	if isWindowsDriveURI(u.Path) {
+		u.Path = u.Path[1:]
+	}
+	return u.Path, nil
 }
 
 // ToURI returns a protocol URI for the supplied path.
 // It will always have the file scheme.
 func ToURI(path string) URI {
+	u := toURI(path)
+	u.Path = filepath.ToSlash(u.Path)
+	return URI(u.String())
+}
+
+func toURI(path string) *url.URL {
+	// 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"
 	if strings.EqualFold(prefix, path[:len(prefix)]) {
 		suffix := path[len(prefix):]
-		//TODO: we need a better way to get the GOROOT that uses the packages api
 		path = runtime.GOROOT() + suffix
 	}
-	return URI(fileSchemePrefix + filepath.ToSlash(path))
+	if isWindowsDrivePath(path) {
+		path = "/" + path
+	}
+	return &url.URL{
+		Scheme: fileScheme,
+		Path:   path,
+	}
+}
+
+// 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 ":".
+func isWindowsDrivePath(path string) bool {
+	if len(path) < 4 {
+		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://github.com/golang/go/issues/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 {
+	if len(uri) < 4 {
+		return false
+	}
+	return uri[0] == '/' && unicode.IsLetter(rune(uri[1])) && uri[2] == ':'
 }

internal/lsp/source/uri_test.go

@@ -0,0 +1,52 @@
+// Copyright 2018 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.
+
+package source
+
+import (
+	"testing"
+)
+
+// 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 _, tt := range []struct {
+		uri      URI
+		filename string
+	}{
+		{
+			uri:      URI(`file:///C:/Windows/System32`),
+			filename: `C:/Windows/System32`,
+		},
+		{
+			uri:      URI(`file:///C:/Go/src/bob.go`),
+			filename: `C:/Go/src/bob.go`,
+		},
+		{
+			uri:      URI(`file:///c:/Go/src/bob.go`),
+			filename: `c:/Go/src/bob.go`,
+		},
+		{
+			uri:      URI(`file:///path/to/dir`),
+			filename: `/path/to/dir`,
+		},
+		{
+			uri:      URI(`file:///a/b/c/src/bob.go`),
+			filename: `/a/b/c/src/bob.go`,
+		},
+	} {
+		if string(tt.uri) != toURI(tt.filename).String() {
+			t.Errorf("ToURI: expected %s, got %s", tt.uri, ToURI(tt.filename))
+		}
+		filename, err := filename(tt.uri)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if tt.filename != filename {
+			t.Errorf("Filename: expected %s, got %s", tt.filename, filename)
+		}
+	}
+}

internal/lsp/source/uri_unix.go

@@ -1,9 +0,0 @@
-// Copyright 2018 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 source
-
-const fileSchemePrefix = "file://"

internal/lsp/source/uri_windows.go

@@ -1,9 +0,0 @@
-// Copyright 2018 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 source
-
-const fileSchemePrefix = "file:///"

internal/lsp/source/uri_windows_test.go

@@ -1,26 +0,0 @@
-// Copyright 2018 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 source
-
-import (
-	"testing"
-)
-
-func TestURIWindows(t *testing.T) {
-	s := `C:\Windows\System32`
-	uri := ToURI(s)
-	if uri != `file:///C:/Windows/System32` {
-		t.Fatalf("ToURI: got %v want %v", uri, s)
-	}
-	f, err := URI(uri).Filename()
-	if err != nil {
-		t.Fatal(err)
-	}
-	if f != s {
-		t.Fatalf("Filename: got %v want %v", f, s)
-	}
-}