internal/span: change URI.Filename so it just returns the filename
We panic if the uri was not a valid file uri instead
They always are a valid file URI, and we would fail miserably to cope if they were
not anyway, and there are lots of places where we need to be able to get the filename
and don't want to cope with an error that cannot occur.
If we ever have not file uri's, you will have to check if it is a file before calling
.Filename, which seems reasonable anyway.
Change-Id: Ifb26a165bd43c2d310378314550b5749b09e2ebd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/181017
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/external.go b/internal/lsp/cache/external.go
index dfb97ae..d2b8286 100644
--- a/internal/lsp/cache/external.go
+++ b/internal/lsp/cache/external.go
@@ -42,11 +42,7 @@
}
func (h *nativeFileHandle) Read(ctx context.Context) ([]byte, string, error) {
- filename, err := h.identity.URI.Filename()
- if err != nil {
- return nil, "", err
- }
- data, err := ioutil.ReadFile(filename)
+ data, err := ioutil.ReadFile(h.identity.URI.Filename())
if err != nil {
return nil, "", err
}
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 2c3b242..64bd142 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -249,9 +249,7 @@
if overlay.onDisk {
continue
}
- if filename, err := uri.Filename(); err == nil {
- overlays[filename] = overlay.data
- }
+ overlays[uri.Filename()] = overlay.data
}
return overlays
}
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index 269aec7..b5b5968 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -114,13 +114,9 @@
// go/packages API. It is shared across all views.
func (v *view) buildConfig() *packages.Config {
//TODO:should we cache the config and/or overlay somewhere?
- folderPath, err := v.folder.Filename()
- if err != nil {
- folderPath = ""
- }
return &packages.Config{
Context: v.backgroundCtx,
- Dir: folderPath,
+ Dir: v.folder.Filename(),
Env: v.env,
BuildFlags: v.buildFlags,
Mode: packages.NeedName |
@@ -315,10 +311,7 @@
} else if f != nil {
return f, nil
}
- filename, err := uri.Filename()
- if err != nil {
- return nil, err
- }
+ filename := uri.Filename()
var f viewFile
switch ext := filepath.Ext(filename); ext {
case ".go":
@@ -363,10 +356,7 @@
}
// no exact match stored, time to do some real work
// check for any files with the same basename
- fname, err := uri.Filename()
- if err != nil {
- return nil, err
- }
+ fname := uri.Filename()
basename := basename(fname)
if candidates := v.filesByBase[basename]; candidates != nil {
pathStat, err := os.Stat(fname)
diff --git a/internal/lsp/cmd/check_test.go b/internal/lsp/cmd/check_test.go
index fad0d78..dba72c7 100644
--- a/internal/lsp/cmd/check_test.go
+++ b/internal/lsp/cmd/check_test.go
@@ -20,10 +20,7 @@
if len(want) == 1 && want[0].Message == "" {
continue
}
- fname, err := uri.Filename()
- if err != nil {
- t.Fatal(err)
- }
+ fname := uri.Filename()
args := []string{"-remote=internal", "check", fname}
out := captureStdOut(t, func() {
tool.Main(context.Background(), r.app, args)
diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go
index b66039c..f0b1540 100644
--- a/internal/lsp/cmd/cmd.go
+++ b/internal/lsp/cmd/cmd.go
@@ -318,11 +318,7 @@
c.files[uri] = file
}
if file.mapper == nil {
- fname, err := uri.Filename()
- if err != nil {
- file.err = fmt.Errorf("%v: %v", uri, err)
- return file
- }
+ fname := uri.Filename()
content, err := ioutil.ReadFile(fname)
if err != nil {
file.err = fmt.Errorf("%v: %v", uri, err)
diff --git a/internal/lsp/cmd/definition_test.go b/internal/lsp/cmd/definition_test.go
index 36b61ef..7b4a9a8 100644
--- a/internal/lsp/cmd/definition_test.go
+++ b/internal/lsp/cmd/definition_test.go
@@ -78,10 +78,6 @@
}
args = append(args, "definition")
uri := d.Src.URI()
- filename, err := uri.Filename()
- if err != nil {
- t.Fatal(err)
- }
args = append(args, fmt.Sprint(d.Src))
got := captureStdOut(t, func() {
tool.Main(context.Background(), r.app, args)
@@ -90,7 +86,7 @@
if mode&jsonGoDef != 0 && runtime.GOOS == "windows" {
got = strings.Replace(got, "file:///", "file://", -1)
}
- expect := strings.TrimSpace(string(r.data.Golden(tag, filename, func() ([]byte, error) {
+ expect := strings.TrimSpace(string(r.data.Golden(tag, uri.Filename(), func() ([]byte, error) {
return []byte(got), nil
})))
if expect != "" && !strings.HasPrefix(got, expect) {
diff --git a/internal/lsp/cmd/format.go b/internal/lsp/cmd/format.go
index 4075ec4..6437552 100644
--- a/internal/lsp/cmd/format.go
+++ b/internal/lsp/cmd/format.go
@@ -62,7 +62,7 @@
if file.err != nil {
return file.err
}
- filename, _ := spn.URI().Filename() // this cannot fail, already checked in AddFile above
+ filename := spn.URI().Filename()
loc, err := file.mapper.Location(spn)
if err != nil {
return err
diff --git a/internal/lsp/cmd/format_test.go b/internal/lsp/cmd/format_test.go
index 3e83d8d..1ff8b86 100644
--- a/internal/lsp/cmd/format_test.go
+++ b/internal/lsp/cmd/format_test.go
@@ -26,10 +26,7 @@
for _, mode := range formatModes {
tag := "gofmt" + strings.Join(mode, "")
uri := spn.URI()
- filename, err := uri.Filename()
- if err != nil {
- t.Fatal(err)
- }
+ filename := uri.Filename()
args := append(mode, filename)
expect := string(r.data.Golden(tag, filename, func() ([]byte, error) {
cmd := exec.Command("gofmt", args...)
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 6867bfb..ee6e02c 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -285,10 +285,7 @@
ctx := context.Background()
for _, spn := range data {
uri := spn.URI()
- filename, err := uri.Filename()
- if err != nil {
- t.Fatal(err)
- }
+ filename := uri.Filename()
gofmted := string(r.data.Golden("gofmt", filename, func() ([]byte, error) {
cmd := exec.Command("gofmt", filename)
out, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files
@@ -326,10 +323,7 @@
ctx := context.Background()
for _, spn := range data {
uri := spn.URI()
- filename, err := uri.Filename()
- if err != nil {
- t.Fatal(err)
- }
+ filename := uri.Filename()
goimported := string(r.data.Golden("goimports", filename, func() ([]byte, error) {
cmd := exec.Command("goimports", filename)
out, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files
@@ -402,11 +396,7 @@
}
if hover != nil {
tag := fmt.Sprintf("%s-hover", d.Name)
- filename, err := d.Src.URI().Filename()
- if err != nil {
- t.Fatalf("failed for %v: %v", d.Def, err)
- }
- expectHover := string(r.data.Golden(tag, filename, func() ([]byte, error) {
+ expectHover := string(r.data.Golden(tag, d.Src.URI().Filename(), func() ([]byte, error) {
return []byte(hover.Contents.Value), nil
}))
if hover.Contents.Value != expectHover {
@@ -675,10 +665,7 @@
}
func (r *runner) mapper(uri span.URI) (*protocol.ColumnMapper, error) {
- filename, err := uri.Filename()
- if err != nil {
- return nil, err
- }
+ filename := uri.Filename()
fset := r.data.Exported.ExpectFileSet
var f *token.File
fset.Iterate(func(check *token.File) bool {
diff --git a/internal/lsp/source/diagnostics_test.go b/internal/lsp/source/diagnostics_test.go
index 8d37a52..12c3a57 100644
--- a/internal/lsp/source/diagnostics_test.go
+++ b/internal/lsp/source/diagnostics_test.go
@@ -29,10 +29,7 @@
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
spn := parseDiagnosticMessage(tt.in)
- fn, err := spn.URI().Filename()
- if err != nil {
- t.Fatalf("unexpected error: %v", err)
- }
+ fn := spn.URI().Filename()
if !strings.HasSuffix(fn, tt.expectedFileName) {
t.Errorf("expected filename with suffix %v but got %v", tt.expectedFileName, fn)
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index 5d957b6..f071fb8 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -271,10 +271,7 @@
ctx := context.Background()
for _, spn := range data {
uri := spn.URI()
- filename, err := uri.Filename()
- if err != nil {
- t.Fatal(err)
- }
+ filename := uri.Filename()
gofmted := string(r.data.Golden("gofmt", filename, func() ([]byte, error) {
cmd := exec.Command("gofmt", filename)
out, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files
@@ -312,10 +309,7 @@
ctx := context.Background()
for _, spn := range data {
uri := spn.URI()
- filename, err := uri.Filename()
- if err != nil {
- t.Fatal(err)
- }
+ filename := uri.Filename()
goimported := string(r.data.Golden("goimports", filename, func() ([]byte, error) {
cmd := exec.Command("goimports", filename)
out, _ := cmd.Output() // ignore error, sometimes we have intentionally ungofmt-able files
@@ -373,11 +367,7 @@
}
if hover != "" {
tag := fmt.Sprintf("%s-hover", d.Name)
- filename, err := d.Src.URI().Filename()
- if err != nil {
- t.Fatalf("failed for %v: %v", d.Def, err)
- }
- expectHover := string(r.data.Golden(tag, filename, func() ([]byte, error) {
+ expectHover := string(r.data.Golden(tag, d.Src.URI().Filename(), func() ([]byte, error) {
return []byte(hover), nil
}))
if hover != expectHover {
diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go
index 5517619..fbbd82d 100644
--- a/internal/lsp/text_synchronization.go
+++ b/internal/lsp/text_synchronization.go
@@ -71,13 +71,9 @@
return "", jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "file not found")
}
fset := s.session.Cache().FileSet()
- filename, err := uri.Filename()
- if err != nil {
- return "", jsonrpc2.NewErrorf(jsonrpc2.CodeInternalError, "no filename for %s", uri)
- }
for _, change := range params.ContentChanges {
// Update column mapper along with the content.
- m := protocol.NewColumnMapper(uri, filename, fset, nil, content)
+ m := protocol.NewColumnMapper(uri, uri.Filename(), fset, nil, content)
spn, err := m.RangeSpan(*change.Range)
if err != nil {
diff --git a/internal/lsp/util.go b/internal/lsp/util.go
index 33ed865..3984ed8 100644
--- a/internal/lsp/util.go
+++ b/internal/lsp/util.go
@@ -18,15 +18,11 @@
if err != nil {
return nil, nil, err
}
- filename, err := f.URI().Filename()
- if err != nil {
- return nil, nil, err
- }
data, _, err := f.Handle(ctx).Read(ctx)
if err != nil {
return nil, nil, err
}
- m := protocol.NewColumnMapper(f.URI(), filename, f.FileSet(), f.GetToken(ctx), data)
+ m := protocol.NewColumnMapper(f.URI(), f.URI().Filename(), f.FileSet(), f.GetToken(ctx), data)
return f, m, nil
}
diff --git a/internal/span/span.go b/internal/span/span.go
index 6da7a05..b66bd7a 100644
--- a/internal/span/span.go
+++ b/internal/span/span.go
@@ -171,9 +171,7 @@
if c == 'f' {
uri = path.Base(uri)
} else if !fullForm {
- if filename, err := s.v.URI.Filename(); err == nil {
- uri = filename
- }
+ uri = s.v.URI.Filename()
}
fmt.Fprint(f, uri)
if !s.IsValid() || (!fullForm && s.v.Start.isZero() && s.v.End.isZero()) {
diff --git a/internal/span/uri.go b/internal/span/uri.go
index c1c068b..3263437 100644
--- a/internal/span/uri.go
+++ b/internal/span/uri.go
@@ -19,14 +19,14 @@
// URI represents the full URI for a file.
type URI string
-// Filename returns the file path for the given URI. It will return an error if
-// the URI is invalid, or if the URI does not have the file scheme.
-func (uri URI) Filename() (string, error) {
+// Filename returns the file path for the given URI.
+// It is an error to call this on a URI that is not a valid filename.
+func (uri URI) Filename() string {
filename, err := filename(uri)
if err != nil {
- return "", err
+ panic(err)
}
- return filepath.FromSlash(filename), nil
+ return filepath.FromSlash(filename)
}
func filename(uri URI) (string, error) {
@@ -60,22 +60,19 @@
return 0
}
// If we have the same URI basename, we may still have the same file URIs.
- if fa, err := a.Filename(); err == nil {
- if fb, err := b.Filename(); err == nil {
- if strings.EqualFold(filepath.Base(fa), filepath.Base(fb)) {
- // Stat the files to check if they are equal.
- if infoa, err := os.Stat(fa); err == nil {
- if infob, err := os.Stat(fb); err == nil {
- if os.SameFile(infoa, infob) {
- return 0
- }
- }
+ fa := a.Filename()
+ fb := b.Filename()
+ if strings.EqualFold(filepath.Base(fa), filepath.Base(fb)) {
+ // Stat the files to check if they are equal.
+ if infoa, err := os.Stat(fa); err == nil {
+ if infob, err := os.Stat(fb); err == nil {
+ if os.SameFile(infoa, infob) {
+ return 0
}
}
- return strings.Compare(fa, fb)
}
}
- return strings.Compare(string(a), string(b))
+ return strings.Compare(fa, fb)
}
// FileURI returns a span URI for the supplied file path.
diff --git a/internal/span/uri_test.go b/internal/span/uri_test.go
index b35fa04..7b7c8b9 100644
--- a/internal/span/uri_test.go
+++ b/internal/span/uri_test.go
@@ -39,10 +39,7 @@
if expectURI != string(uri) {
t.Errorf("ToURI: expected %s, got %s", expectURI, uri)
}
- filename, err := uri.Filename()
- if err != nil {
- t.Fatal(err)
- }
+ filename := uri.Filename()
if expectPath != filename {
t.Errorf("Filename: expected %s, got %s", expectPath, filename)
}
