diff --git a/internal/lsp/cache/external.go b/internal/lsp/cache/external.go index dfb97ae3..d2b82866 100644 --- a/internal/lsp/cache/external.go +++ b/internal/lsp/cache/external.go @@ -42,11 +42,7 @@ func (h *nativeFileHandle) Identity() source.FileIdentity { } 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 2c3b2422..64bd142e 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -249,9 +249,7 @@ func (s *session) buildOverlay() map[string][]byte { 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 269aec77..b5b59685 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -114,13 +114,9 @@ func (v *view) Folder() span.URI { // 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 @@ func (v *view) getFile(uri span.URI) (viewFile, error) { } 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 @@ func (v *view) findFile(uri span.URI) (viewFile, error) { } // 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 fad0d78b..dba72c79 100644 --- a/internal/lsp/cmd/check_test.go +++ b/internal/lsp/cmd/check_test.go @@ -20,10 +20,7 @@ func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) { 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 b66039c5..f0b15409 100644 --- a/internal/lsp/cmd/cmd.go +++ b/internal/lsp/cmd/cmd.go @@ -318,11 +318,7 @@ func (c *cmdClient) getFile(ctx context.Context, uri span.URI) *cmdFile { 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 36b61eff..7b4a9a84 100644 --- a/internal/lsp/cmd/definition_test.go +++ b/internal/lsp/cmd/definition_test.go @@ -78,10 +78,6 @@ func (r *runner) Definition(t *testing.T, data tests.Definitions) { } 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 @@ func (r *runner) Definition(t *testing.T, data tests.Definitions) { 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 4075ec46..64375527 100644 --- a/internal/lsp/cmd/format.go +++ b/internal/lsp/cmd/format.go @@ -62,7 +62,7 @@ func (f *format) Run(ctx context.Context, args ...string) error { 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 3e83d8d7..1ff8b866 100644 --- a/internal/lsp/cmd/format_test.go +++ b/internal/lsp/cmd/format_test.go @@ -26,10 +26,7 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { 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 6867bfb6..ee6e02c8 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -285,10 +285,7 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { 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 @@ func (r *runner) Import(t *testing.T, data tests.Imports) { 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 @@ func (r *runner) Definition(t *testing.T, data tests.Definitions) { } 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) Link(t *testing.T, data tests.Links) { } 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 8d37a52b..12c3a578 100644 --- a/internal/lsp/source/diagnostics_test.go +++ b/internal/lsp/source/diagnostics_test.go @@ -29,10 +29,7 @@ func TestParseErrorMessage(t *testing.T) { 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 5d957b6f..f071fb84 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -271,10 +271,7 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { 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 @@ func (r *runner) Import(t *testing.T, data tests.Imports) { 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 @@ func (r *runner) Definition(t *testing.T, data tests.Definitions) { } 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 55176198..fbbd82d2 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -71,13 +71,9 @@ func (s *Server) applyChanges(ctx context.Context, params *protocol.DidChangeTex 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 33ed8652..3984ed85 100644 --- a/internal/lsp/util.go +++ b/internal/lsp/util.go @@ -18,15 +18,11 @@ func getSourceFile(ctx context.Context, v source.View, uri span.URI) (source.Fil 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 6da7a05f..b66bd7a5 100644 --- a/internal/span/span.go +++ b/internal/span/span.go @@ -171,9 +171,7 @@ func (s Span) Format(f fmt.State, c rune) { 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 c1c068b7..32634374 100644 --- a/internal/span/uri.go +++ b/internal/span/uri.go @@ -19,14 +19,14 @@ const fileScheme = "file" // 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 @@ func CompareURI(a, b URI) int { 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 b35fa047..7b7c8b90 100644 --- a/internal/span/uri_test.go +++ b/internal/span/uri_test.go @@ -39,10 +39,7 @@ func TestURI(t *testing.T) { 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) }