Skip to content

Commit

Permalink
gopls/internal/regtest/marker: add missing tests for hover
Browse files Browse the repository at this point in the history
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 <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
  • Loading branch information
findleyr committed Feb 7, 2023
1 parent 24a13c6 commit 69920f2
Show file tree
Hide file tree
Showing 3 changed files with 181 additions and 6 deletions.
37 changes: 31 additions & 6 deletions gopls/internal/lsp/regtest/marker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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, '/')
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand Down
27 changes: 27 additions & 0 deletions gopls/internal/regtest/marker/testdata/hover/goprivate.txt
Original file line number Diff line number Diff line change
@@ -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.
123 changes: 123 additions & 0 deletions gopls/internal/regtest/marker/testdata/hover/linkable.txt
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 69920f2

Please sign in to comment.