internal/span, internal/lsp: fix URI escaping
We had previously worked around a VS Code URI bug by unescaping URIs.
This is incorrect, so stop doing it and then add a specific workaround
just for that one bug.
Fixes golang/go#36999
Change-Id: I92f1a5f71749af7a6b1020eee1272586515f7084
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217599
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
(cherry picked from commit 35ac94b00d9e8c6a60f7918389f0c289c680c098)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217639
diff --git a/internal/lsp/testdata/%percent/perc%ent.go b/internal/lsp/testdata/%percent/perc%ent.go
new file mode 100644
index 0000000..f993da8
--- /dev/null
+++ b/internal/lsp/testdata/%percent/perc%ent.go
@@ -0,0 +1,8 @@
+package percent
+
+import (
+)
+
+func _() {
+ var x int //@diag("x", "compiler", "x declared but not used")
+}
\ No newline at end of file
diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden
index bbb3909..7036c8e 100644
--- a/internal/lsp/testdata/summary.txt.golden
+++ b/internal/lsp/testdata/summary.txt.golden
@@ -6,7 +6,7 @@
FuzzyCompletionsCount = 8
RankedCompletionsCount = 66
CaseSensitiveCompletionsCount = 4
-DiagnosticsCount = 37
+DiagnosticsCount = 38
FoldingRangesCount = 2
FormatCount = 6
ImportCount = 7
diff --git a/internal/span/uri.go b/internal/span/uri.go
index 1c39134..26dc90c 100644
--- a/internal/span/uri.go
+++ b/internal/span/uri.go
@@ -46,24 +46,26 @@
if isWindowsDriveURIPath(u.Path) {
u.Path = strings.ToUpper(string(u.Path[1])) + u.Path[2:]
}
-
return u.Path, nil
}
// NewURI returns a span URI for the string.
// It will attempt to detect if the string is a file path or uri.
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+"://") {
+ if i := len(fileScheme + "://"); strings.HasPrefix(s, "file:///") {
+ // Handle microsoft/vscode#75027 by making it a special case.
+ // On Windows, VS Code sends file URIs that look like file:///C%3A/x/y/z.
+ // Replace the %3A so that the URI looks like: file:///C:/x/y/z.
+ if strings.ToLower(s[i+2:i+5]) == "%3a" {
+ s = s[:i+2] + ":" + s[i+5:]
+ }
// 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:]) {
+ if isWindowsDriveURIPath(s[i:]) {
s = s[:i+1] + strings.ToUpper(string(s[i+1])) + s[i+2:]
}
return URI(s)
@@ -136,11 +138,7 @@
Scheme: fileScheme,
Path: path,
}
- uri := u.String()
- if unescaped, err := url.PathUnescape(uri); err == nil {
- uri = unescaped
- }
- return URI(uri)
+ return URI(u.String())
}
// isWindowsDrivePath returns true if the file path is of the form used by
diff --git a/internal/span/uri_test.go b/internal/span/uri_test.go
index d29f130..a3754e3 100644
--- a/internal/span/uri_test.go
+++ b/internal/span/uri_test.go
@@ -54,21 +54,31 @@
{
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"),
+ wantURI: span.URI("file:///C:/Go/src/bob%20george/george/george.go"),
},
{
- path: `file:///c:/Go/src/bob george/george/george.go`,
+ path: `file:///c:/Go/src/bob%20george/george/george.go`,
wantFile: `C:/Go/src/bob george/george/george.go`,
- wantURI: span.URI("file:///C:/Go/src/bob george/george/george.go"),
+ wantURI: span.URI("file:///C:/Go/src/bob%20george/george/george.go"),
+ },
+ {
+ path: `file:///C%3A/Go/src/bob%20george/george/george.go`,
+ wantFile: `C:/Go/src/bob george/george/george.go`,
+ wantURI: span.URI("file:///C:/Go/src/bob%20george/george/george.go"),
+ },
+ {
+ path: `file:///path/to/%25p%25ercent%25/per%25cent.go`,
+ wantFile: `/path/to/%p%ercent%/per%cent.go`,
+ wantURI: span.URI(`file:///path/to/%25p%25ercent%25/per%25cent.go`),
},
} {
got := span.NewURI(test.path)
if got != test.wantURI {
- t.Errorf("ToURI: got %s, expected %s", got, test.wantURI)
+ t.Errorf("NewURI(%q): got %q, expected %q", test.path, got, test.wantURI)
}
gotFilename := got.Filename()
if gotFilename != test.wantFile {
- t.Errorf("Filename: got %s, expected %s", gotFilename, test.wantFile)
+ t.Errorf("Filename(%q): got %q, expected %q", got, gotFilename, test.wantFile)
}
}
}
diff --git a/internal/span/uri_windows_test.go b/internal/span/uri_windows_test.go
index 2eb07e7..1370b19 100644
--- a/internal/span/uri_windows_test.go
+++ b/internal/span/uri_windows_test.go
@@ -54,12 +54,22 @@
{
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"),
+ wantURI: span.URI("file:///C:/Go/src/bob%20george/george/george.go"),
},
{
- path: `file:///c:/Go/src/bob george/george/george.go`,
+ path: `file:///c:/Go/src/bob%20george/george/george.go`,
wantFile: `C:\Go\src\bob george\george\george.go`,
- wantURI: span.URI("file:///C:/Go/src/bob george/george/george.go"),
+ wantURI: span.URI("file:///C:/Go/src/bob%20george/george/george.go"),
+ },
+ {
+ path: `file:///C%3A/Go/src/bob%20george/george/george.go`,
+ wantFile: `C:\Go\src\bob george\george\george.go`,
+ wantURI: span.URI("file:///C:/Go/src/bob%20george/george/george.go"),
+ },
+ {
+ path: `file:///c:/path/to/%25p%25ercent%25/per%25cent.go`,
+ wantFile: `C:\path\to\%p%ercent%\per%cent.go`,
+ wantURI: span.URI(`file:///C:/path/to/%25p%25ercent%25/per%25cent.go`),
},
} {
got := span.NewURI(test.path)
