From db1d1e0d3387b47f53b696e1ba08fc6349611854 Mon Sep 17 00:00:00 2001 From: Viktor Blomqvist Date: Fri, 29 Sep 2023 17:25:34 +0200 Subject: [PATCH] gopls/internal/lsp: go to definition from embed directive Enable jump to definition on //go:embed directive arguments. If multiple files match the pattern one is picked at random. Improve the pattern matching for both definition and hover to exclude directories since they are not embeddable in themselves. Updates golang/go#50262 Change-Id: I09da40f195e8edfe661acaacd99f62d9f577e9ea Reviewed-on: https://go-review.googlesource.com/c/tools/+/531775 Reviewed-by: Robert Findley Reviewed-by: Suzy Mueller LUCI-TryBot-Result: Go LUCI --- gopls/internal/lsp/definition.go | 6 -- gopls/internal/lsp/source/definition.go | 13 +++++ gopls/internal/lsp/source/embeddirective.go | 56 +++++++++++++++++++ gopls/internal/lsp/source/hover.go | 4 +- .../internal/regtest/misc/definition_test.go | 40 +++++++++++++ gopls/internal/regtest/misc/hover_test.go | 11 ++++ 6 files changed, 122 insertions(+), 8 deletions(-) diff --git a/gopls/internal/lsp/definition.go b/gopls/internal/lsp/definition.go index a438bebc8d3..892e48d6377 100644 --- a/gopls/internal/lsp/definition.go +++ b/gopls/internal/lsp/definition.go @@ -6,7 +6,6 @@ package lsp import ( "context" - "errors" "fmt" "golang.org/x/tools/gopls/internal/lsp/protocol" @@ -36,11 +35,6 @@ func (s *Server) definition(ctx context.Context, params *protocol.DefinitionPara case source.Tmpl: return template.Definition(snapshot, fh, params.Position) case source.Go: - // Partial support for jumping from linkname directive (position at 2nd argument). - locations, err := source.LinknameDefinition(ctx, snapshot, fh, params.Position) - if !errors.Is(err, source.ErrNoLinkname) { - return locations, err - } return source.Definition(ctx, snapshot, fh, params.Position) default: return nil, fmt.Errorf("can't find definitions for file type %s", kind) diff --git a/gopls/internal/lsp/source/definition.go b/gopls/internal/lsp/source/definition.go index 90a432966b8..dd3feda70a2 100644 --- a/gopls/internal/lsp/source/definition.go +++ b/gopls/internal/lsp/source/definition.go @@ -6,6 +6,7 @@ package source import ( "context" + "errors" "fmt" "go/ast" "go/token" @@ -58,6 +59,18 @@ func Definition(ctx context.Context, snapshot Snapshot, fh FileHandle, position return []protocol.Location{loc}, nil } + // Handle the case where the cursor is in a linkname directive. + locations, err := LinknameDefinition(ctx, snapshot, fh, position) + if !errors.Is(err, ErrNoLinkname) { + return locations, err + } + + // Handle the case where the cursor is in an embed directive. + locations, err = EmbedDefinition(pgf.Mapper, position) + if !errors.Is(err, ErrNoEmbed) { + return locations, err + } + // The general case: the cursor is on an identifier. _, obj, _ := referencedObject(pkg, pgf, pos) if obj == nil { diff --git a/gopls/internal/lsp/source/embeddirective.go b/gopls/internal/lsp/source/embeddirective.go index b1a0ff9d235..d4e85d7add2 100644 --- a/gopls/internal/lsp/source/embeddirective.go +++ b/gopls/internal/lsp/source/embeddirective.go @@ -5,7 +5,10 @@ package source import ( + "errors" "fmt" + "io/fs" + "path/filepath" "strconv" "strings" "unicode" @@ -14,6 +17,59 @@ import ( "golang.org/x/tools/gopls/internal/lsp/protocol" ) +// ErrNoEmbed is returned by EmbedDefinition when no embed +// directive is found at a particular position. +// As such it indicates that other definitions could be worth checking. +var ErrNoEmbed = errors.New("no embed directive found") + +var errStopWalk = errors.New("stop walk") + +// EmbedDefinition finds a file matching the embed directive at pos in the mapped file. +// If there is no embed directive at pos, returns ErrNoEmbed. +// If multiple files match the embed pattern, one is picked at random. +func EmbedDefinition(m *protocol.Mapper, pos protocol.Position) ([]protocol.Location, error) { + pattern, _ := parseEmbedDirective(m, pos) + if pattern == "" { + return nil, ErrNoEmbed + } + + // Find the first matching file. + var match string + dir := filepath.Dir(m.URI.Filename()) + err := filepath.WalkDir(dir, func(abs string, d fs.DirEntry, e error) error { + if e != nil { + return e + } + rel, err := filepath.Rel(dir, abs) + if err != nil { + return err + } + ok, err := filepath.Match(pattern, rel) + if err != nil { + return err + } + if ok && !d.IsDir() { + match = abs + return errStopWalk + } + return nil + }) + if err != nil && !errors.Is(err, errStopWalk) { + return nil, err + } + if match == "" { + return nil, fmt.Errorf("%q does not match any files in %q", pattern, dir) + } + + loc := protocol.Location{ + URI: protocol.URIFromPath(match), + Range: protocol.Range{ + Start: protocol.Position{Line: 0, Character: 0}, + }, + } + return []protocol.Location{loc}, nil +} + // parseEmbedDirective attempts to parse a go:embed directive argument at pos. // If successful it return the directive argument and its range, else zero values are returned. func parseEmbedDirective(m *protocol.Mapper, pos protocol.Position) (string, protocol.Range) { diff --git a/gopls/internal/lsp/source/hover.go b/gopls/internal/lsp/source/hover.go index 8578f13e2ca..95317833489 100644 --- a/gopls/internal/lsp/source/hover.go +++ b/gopls/internal/lsp/source/hover.go @@ -640,7 +640,7 @@ func hoverEmbed(fh FileHandle, rng protocol.Range, pattern string) (protocol.Ran dir := filepath.Dir(fh.URI().Filename()) var matches []string - err := filepath.WalkDir(dir, func(abs string, _ fs.DirEntry, e error) error { + err := filepath.WalkDir(dir, func(abs string, d fs.DirEntry, e error) error { if e != nil { return e } @@ -652,7 +652,7 @@ func hoverEmbed(fh FileHandle, rng protocol.Range, pattern string) (protocol.Ran if err != nil { return err } - if ok { + if ok && !d.IsDir() { matches = append(matches, rel) } return nil diff --git a/gopls/internal/regtest/misc/definition_test.go b/gopls/internal/regtest/misc/definition_test.go index f11b2073292..d16539f0dbb 100644 --- a/gopls/internal/regtest/misc/definition_test.go +++ b/gopls/internal/regtest/misc/definition_test.go @@ -529,3 +529,43 @@ const _ = b.K } }) } + +const embedDefinition = ` +-- go.mod -- +module mod.com + +-- main.go -- +package main + +import ( + "embed" +) + +//go:embed *.txt +var foo embed.FS + +func main() {} + +-- skip.sql -- +SKIP + +-- foo.txt -- +FOO + +-- skip.bat -- +SKIP +` + +func TestGoToEmbedDefinition(t *testing.T) { + Run(t, embedDefinition, func(t *testing.T, env *Env) { + env.OpenFile("main.go") + + start := env.RegexpSearch("main.go", `\*.txt`) + loc := env.GoToDefinition(start) + + name := env.Sandbox.Workdir.URIToPath(loc.URI) + if want := "foo.txt"; name != want { + t.Errorf("GoToDefinition: got file %q, want %q", name, want) + } + }) +} diff --git a/gopls/internal/regtest/misc/hover_test.go b/gopls/internal/regtest/misc/hover_test.go index fadaf7f79bc..7b84f8aa871 100644 --- a/gopls/internal/regtest/misc/hover_test.go +++ b/gopls/internal/regtest/misc/hover_test.go @@ -458,6 +458,8 @@ BAR BAZ -- other.sql -- SKIPPED +-- dir.txt/skip.txt -- +SKIPPED ` func TestHoverEmbedDirective(t *testing.T) { @@ -478,5 +480,14 @@ func TestHoverEmbedDirective(t *testing.T) { t.Errorf("hover: %q does not contain: %q", content, want) } } + + // A directory should never be matched, even if it happens to have a matching name. + // Content in subdirectories should not match on only one asterisk. + skips := []string{"other.sql", "dir.txt", "skip.txt"} + for _, skip := range skips { + if strings.Contains(content, skip) { + t.Errorf("hover: %q should not contain: %q", content, skip) + } + } }) }