From 69920f2e63b8c2fcebf58ce7b2a665cba9b2c5f1 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Tue, 7 Feb 2023 11:17:37 -0500 Subject: [PATCH] gopls/internal/regtest/marker: add missing tests for hover Add additional tests for hover behavior. These tests would have failed in earlier patchsets of the subsequent CL rewriting hover. Also, add support for the special "env" file to the marker framework. Change-Id: Iecbd4994a6c1261f87163d50793fbbc5f26ea1ba Reviewed-on: https://go-review.googlesource.com/c/tools/+/466135 TryBot-Result: Gopher Robot Run-TryBot: Robert Findley Reviewed-by: Alan Donovan gopls-CI: kokoro --- gopls/internal/lsp/regtest/marker.go | 37 +++++- .../marker/testdata/hover/goprivate.txt | 27 ++++ .../marker/testdata/hover/linkable.txt | 123 ++++++++++++++++++ 3 files changed, 181 insertions(+), 6 deletions(-) create mode 100644 gopls/internal/regtest/marker/testdata/hover/goprivate.txt create mode 100644 gopls/internal/regtest/marker/testdata/hover/linkable.txt diff --git a/gopls/internal/lsp/regtest/marker.go b/gopls/internal/lsp/regtest/marker.go index 1bba223ae70..3fe5ccd2870 100644 --- a/gopls/internal/lsp/regtest/marker.go +++ b/gopls/internal/lsp/regtest/marker.go @@ -95,6 +95,8 @@ var updateGolden = flag.Bool("update", false, "if set, update test data during m // for the test. // - "settings.json": this file is parsed as JSON, and used as the // session configuration (see gopls/doc/settings.md) +// - "env": this file is parsed as a list of VAR=VALUE fields specifying the +// editor environment. // - Golden files: Within the archive, file names starting with '@' are // treated as "golden" content, and are not written to disk, but instead are // made available to test methods expecting an argument of type *Golden, @@ -189,7 +191,6 @@ var updateGolden = flag.Bool("update", false, "if set, update test data during m // // Remaining TODO: // - parallelize/optimize test execution -// - add support for per-test environment? // - reorganize regtest packages (and rename to just 'test'?) // // Existing marker tests to port: @@ -246,9 +247,13 @@ func RunMarkerTests(t *testing.T, dir string) { testenv.NeedsGo1Point(t, 18) } test.executed = true + config := fake.EditorConfig{ + Settings: test.settings, + Env: test.env, + } c := &markerContext{ test: test, - env: newEnv(t, cache, test.files, test.settings), + env: newEnv(t, cache, test.files, config), locations: make(map[expect.Identifier]protocol.Location), diags: make(map[protocol.Location][]protocol.Diagnostic), @@ -380,6 +385,7 @@ type MarkerTest struct { fset *token.FileSet // fileset used for parsing notes archive *txtar.Archive // original test archive settings map[string]interface{} // gopls settings + env map[string]string // editor environment files map[string][]byte // data files from the archive (excluding special files) notes []*expect.Note // extracted notes from data files golden map[string]*Golden // extracted golden content, by identifier name @@ -490,6 +496,19 @@ func loadMarkerTest(name string, archive *txtar.Archive) (*MarkerTest, error) { } continue } + if file.Name == "env" { + test.env = make(map[string]string) + fields := strings.Fields(string(file.Data)) + for _, field := range fields { + // TODO: use strings.Cut once we are on 1.18+. + idx := strings.IndexByte(field, '=') + if idx < 0 { + return nil, fmt.Errorf("env vars must be formatted as var=value, got %q", field) + } + test.env[field[:idx]] = field[idx+1:] + } + continue + } if strings.HasPrefix(file.Name, "@") { // golden content // TODO: use strings.Cut once we are on 1.18+. idx := strings.IndexByte(file.Name, '/') @@ -541,6 +560,15 @@ func writeMarkerTests(dir string, tests []*MarkerTest) error { } arch.Files = append(arch.Files, txtar.File{Name: "settings.json", Data: data}) } + if len(test.env) > 0 { + var vars []string + for k, v := range test.env { + vars = append(vars, fmt.Sprintf("%s=%s", k, v)) + } + sort.Strings(vars) + data := []byte(strings.Join(vars, "\n")) + arch.Files = append(arch.Files, txtar.File{Name: "env", Data: data}) + } // ...followed by ordinary files. Preserve the order they appeared in the // original archive. @@ -578,7 +606,7 @@ func writeMarkerTests(dir string, tests []*MarkerTest) error { // // TODO(rfindley): simplify and refactor the construction of testing // environments across regtests, marker tests, and benchmarks. -func newEnv(t *testing.T, cache *cache.Cache, files map[string][]byte, settings map[string]interface{}) *Env { +func newEnv(t *testing.T, cache *cache.Cache, files map[string][]byte, config fake.EditorConfig) *Env { sandbox, err := fake.NewSandbox(&fake.SandboxConfig{ RootDir: t.TempDir(), GOPROXY: "https://proxy.golang.org", @@ -596,9 +624,6 @@ func newEnv(t *testing.T, cache *cache.Cache, files map[string][]byte, settings awaiter := NewAwaiter(sandbox.Workdir) ss := lsprpc.NewStreamServer(cache, false, hooks.Options) server := servertest.NewPipeServer(ss, jsonrpc2.NewRawStream) - config := fake.EditorConfig{ - Settings: settings, - } editor, err := fake.NewEditor(sandbox, config).Connect(ctx, server, awaiter.Hooks()) if err != nil { sandbox.Close() // ignore error diff --git a/gopls/internal/regtest/marker/testdata/hover/goprivate.txt b/gopls/internal/regtest/marker/testdata/hover/goprivate.txt new file mode 100644 index 00000000000..4c309ef38cf --- /dev/null +++ b/gopls/internal/regtest/marker/testdata/hover/goprivate.txt @@ -0,0 +1,27 @@ +This test checks that links in hover obey GOPRIVATE. +-- env -- +GOPRIVATE=mod.com +-- go.mod -- +module mod.com +-- p.go -- +package p + +// T should not be linked, as it is private. +type T struct{} //@hover("T", "T", T) +-- lib/lib.go -- +package lib + +// GOPRIVATE should also match nested packages. +type L struct{} //@hover("L", "L", L) +-- @L/hover.md -- +```go +type L struct{} +``` + +GOPRIVATE should also match nested packages. +-- @T/hover.md -- +```go +type T struct{} +``` + +T should not be linked, as it is private. diff --git a/gopls/internal/regtest/marker/testdata/hover/linkable.txt b/gopls/internal/regtest/marker/testdata/hover/linkable.txt new file mode 100644 index 00000000000..b82600a1e13 --- /dev/null +++ b/gopls/internal/regtest/marker/testdata/hover/linkable.txt @@ -0,0 +1,123 @@ +This test checks that we correctly determine pkgsite links for various +identifiers. + +We should only produce links that work, meaning the object is reachable via the +package's public API. +-- go.mod -- +module mod.com + +go 1.18 +-- p.go -- +package p + +type E struct { + Embed int +} + +// T is in the package scope, and so should be linkable. +type T struct{ //@hover("T", "T", T) + // Only exported fields should be linkable + + f int //@hover("f", "f", f) + F int //@hover("F", "F", F) + + E + + // TODO(rfindley): is the link here correct? It ignores N. + N struct { + // Nested fields should also be linkable. + Nested int //@hover("Nested", "Nested", Nested) + } +} +// M is an exported method, and so should be linkable. +func (T) M() {} + +// m is not exported, and so should not be linkable. +func (T) m() {} + +func _() { + var t T + + // Embedded fields should be linkable. + _ = t.Embed //@hover("Embed", "Embed", Embed) + + // Local variables should not be linkable, even if they are capitalized. + var X int //@hover("X", "X", X) + _ = X + + // Local types should not be linkable, even if they are capitalized. + type Local struct { //@hover("Local", "Local", Local) + E + } + + // But the embedded field should still be linkable. + var l Local + _ = l.Embed //@hover("Embed", "Embed", Embed) +} +-- @Embed/hover.md -- +```go +field Embed int +``` + +[`(p.E).Embed` on pkg.go.dev](https://pkg.go.dev/mod.com#E.Embed) +-- @F/hover.md -- +```go +field F int +``` + +@hover("F", "F", F) + + +[`(p.T).F` on pkg.go.dev](https://pkg.go.dev/mod.com#T.F) +-- @Local/hover.md -- +```go +type Local struct { + E +} +``` + +Local types should not be linkable, even if they are capitalized. +-- @Nested/hover.md -- +```go +field Nested int +``` + +Nested fields should also be linkable. + + +[`(p.T).Nested` on pkg.go.dev](https://pkg.go.dev/mod.com#T.Nested) +-- @T/hover.md -- +```go +type T struct { + f int //@hover("f", "f", f) + F int //@hover("F", "F", F) + + E + + // TODO(rfindley): is the link here correct? It ignores N. + N struct { + // Nested fields should also be linkable. + Nested int //@hover("Nested", "Nested", Nested) + } +} + +func (T).M() +func (T).m() +``` + +T is in the package scope, and so should be linkable. + + +[`p.T` on pkg.go.dev](https://pkg.go.dev/mod.com#T) +-- @X/hover.md -- +```go +var X int +``` + +Local variables should not be linkable, even if they are capitalized. +-- @f/hover.md -- +```go +field f int +``` + +@hover("f", "f", f)