Skip to content

Commit f2e330f

Browse files
committed
gopls/test: add type checking for debug server templates
When internal data structures in gopls changed, the templates driving the debug server failed because fields or methods they relied on had gone away. At some cost, this change adds tests that all the fields and methods in the templates can be satisfied. The test assumes that all the template uses are through render() in debug/serve.go. It checks that is list of templates is the same as the server's, and then type checks each template. The costs are 1. the template type checking is done by a new external dependency, github.com/jba/templatecheck 2. To avoid adding a new dependency to x/tools, the test is in the gopls module, not the debug package, so variables from the debug package have to be exported In an ideal world the debug package, and much of internal/lsp, would be in the gopls package. In that case the cost 2 about could be avoided. In an ideal world, the core runtime template packages would contain template checking code, and cost 1 would be unnecessary. In a more likely (or not so distant) world, perhaps jba would move (or allow us to move) the template check package into x/tools. Change-Id: I36b509a00cbdcb5323ee1af3c1193b603c7a907f Reviewed-on: https://go-review.googlesource.com/c/tools/+/277292 Run-TryBot: Peter Weinberger <pjw@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Rebecca Stambler <rstambler@golang.org> Trust: Peter Weinberger <pjw@google.com>
1 parent 1965356 commit f2e330f

File tree

6 files changed

+220
-36
lines changed

6 files changed

+220
-36
lines changed

gopls/go.mod

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@ module golang.org/x/tools/gopls
33
go 1.12
44

55
require (
6+
github.com/jba/templatecheck v0.4.0
67
github.com/sanity-io/litter v1.3.0
78
github.com/sergi/go-diff v1.1.0
8-
golang.org/x/tools v0.0.0-20201021214918-23787c007979
9+
golang.org/x/tools v0.0.0-20201211185031-d93e913c1a58
910
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1
1011
honnef.co/go/tools v0.0.1-2020.1.6
1112
mvdan.cc/gofumpt v0.0.0-20200927160801-5bfeb2e70dd6

gopls/go.sum

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
77
github.com/google/go-cmp v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k=
88
github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
99
github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
10+
github.com/google/safehtml v0.0.2 h1:ZOt2VXg4x24bW0m2jtzAOkhoXV0iM8vNKc0paByCZqM=
11+
github.com/google/safehtml v0.0.2/go.mod h1:L4KWwDsUJdECRAEpZoBn3O64bQaywRscowZjJAzjHnU=
12+
github.com/jba/templatecheck v0.2.0 h1:OHHJOumS3D6HiHiRp7FuRF17icl7AenH2cvufaBw5Ss=
13+
github.com/jba/templatecheck v0.2.0/go.mod h1:HzXVhxZv+uArJx7Reareec/jWUvKoHpKDvs6I3wdsRw=
14+
github.com/jba/templatecheck v0.4.0 h1:fJ/X92oBGpaAx9SjL57nmVkA1yrB2otPCFWrPVuTnaM=
15+
github.com/jba/templatecheck v0.4.0/go.mod h1:/1k7EajoSErFI9GLHAsiIJEaNLt3ALKNw2TV7z2SYv4=
1016
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
1117
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
1218
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
@@ -42,7 +48,10 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5h
4248
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
4349
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
4450
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
51+
golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k=
4552
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
53+
golang.org/x/text v0.3.4 h1:0YWbFKbhXG/wIiuHDSKpS0Iy7FSA+u45VtBMfQcFTTc=
54+
golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
4655
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
4756
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
4857
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=

gopls/test/debug/debug_test.go

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
package debug_test
2+
3+
// Provide 'static type checking' of the templates. This guards against changes is various
4+
// gopls datastructures causing template execution to fail. The checking is done by
5+
// the github.com/jba/templatecheck pacakge. Before that is run, the test checks that
6+
// its list of templates and their arguments corresponds to the arguments in
7+
// calls to render(). The test assumes that all uses of templates are done through render().
8+
9+
import (
10+
"go/ast"
11+
"html/template"
12+
"log"
13+
"runtime"
14+
"sort"
15+
"strings"
16+
"testing"
17+
18+
"github.com/jba/templatecheck"
19+
"golang.org/x/tools/go/packages"
20+
"golang.org/x/tools/internal/lsp/cache"
21+
"golang.org/x/tools/internal/lsp/debug"
22+
"golang.org/x/tools/internal/lsp/source"
23+
"golang.org/x/tools/internal/span"
24+
)
25+
26+
type tdata struct {
27+
tmpl *template.Template
28+
data interface{} // a value of the needed type
29+
}
30+
31+
var templates = map[string]tdata{
32+
"MainTmpl": {debug.MainTmpl, &debug.Instance{}},
33+
"DebugTmpl": {debug.DebugTmpl, nil},
34+
"RPCTmpl": {debug.RPCTmpl, &debug.Rpcs{}},
35+
"TraceTmpl": {debug.TraceTmpl, debug.TraceResults{}},
36+
"CacheTmpl": {debug.CacheTmpl, &cache.Cache{}},
37+
"SessionTmpl": {debug.SessionTmpl, &cache.Session{}},
38+
"ViewTmpl": {debug.ViewTmpl, &cache.View{}},
39+
"ClientTmpl": {debug.ClientTmpl, &debug.Client{}},
40+
"ServerTmpl": {debug.ServerTmpl, &debug.Server{}},
41+
//"FileTmpl": {FileTmpl, source.Overlay{}}, // need to construct a source.Overlay in init
42+
"InfoTmpl": {debug.InfoTmpl, "something"},
43+
"MemoryTmpl": {debug.MemoryTmpl, runtime.MemStats{}},
44+
}
45+
46+
// construct a source.Overlay for fileTmpl
47+
type fakeOverlay struct{}
48+
49+
func (fakeOverlay) Version() float64 {
50+
return 0
51+
}
52+
func (fakeOverlay) Session() string {
53+
return ""
54+
}
55+
func (fakeOverlay) VersionedFileIdentity() source.VersionedFileIdentity {
56+
return source.VersionedFileIdentity{}
57+
}
58+
func (fakeOverlay) FileIdentity() source.FileIdentity {
59+
return source.FileIdentity{}
60+
}
61+
func (fakeOverlay) Kind() source.FileKind {
62+
return 0
63+
}
64+
func (fakeOverlay) Read() ([]byte, error) {
65+
return nil, nil
66+
}
67+
func (fakeOverlay) Saved() bool {
68+
return true
69+
}
70+
func (fakeOverlay) URI() span.URI {
71+
return ""
72+
}
73+
74+
var _ source.Overlay = fakeOverlay{}
75+
76+
func init() {
77+
log.SetFlags(log.Lshortfile)
78+
var v fakeOverlay
79+
templates["FileTmpl"] = tdata{debug.FileTmpl, v}
80+
}
81+
82+
func TestTemplates(t *testing.T) {
83+
cfg := &packages.Config{
84+
Mode: packages.NeedTypesInfo | packages.LoadAllSyntax, // figure out what's necessary PJW
85+
}
86+
pkgs, err := packages.Load(cfg, "golang.org/x/tools/internal/lsp/debug")
87+
if err != nil {
88+
t.Fatal(err)
89+
}
90+
if len(pkgs) != 1 {
91+
t.Fatalf("expected a single package, but got %d", len(pkgs))
92+
}
93+
p := pkgs[0]
94+
if len(p.Errors) != 0 {
95+
t.Fatalf("compiler error, e.g. %v", p.Errors[0])
96+
}
97+
// find the calls to render in serve.go
98+
tree := treeOf(p, "serve.go")
99+
if tree == nil {
100+
t.Fatalf("found no syntax tree for %s", "serve.go")
101+
}
102+
renders := callsOf(p, tree, "render")
103+
if len(renders) == 0 {
104+
t.Fatalf("found no calls to render")
105+
}
106+
var found = make(map[string]bool)
107+
for _, r := range renders {
108+
if len(r.Args) != 2 {
109+
// template, func
110+
t.Fatalf("got %d args, expected 2", len(r.Args))
111+
}
112+
t0, ok := p.TypesInfo.Types[r.Args[0]]
113+
if !ok || !t0.IsValue() || t0.Type.String() != "*html/template.Template" {
114+
t.Fatalf("no type info for template")
115+
}
116+
if id, ok := r.Args[0].(*ast.Ident); !ok {
117+
t.Errorf("expected *ast.Ident, got %T", r.Args[0])
118+
} else {
119+
found[id.Name] = true
120+
}
121+
}
122+
// make sure found and templates have the same templates
123+
for k := range found {
124+
if _, ok := templates[k]; !ok {
125+
t.Errorf("code has template %s, but test does not", k)
126+
}
127+
}
128+
for k := range templates {
129+
if _, ok := found[k]; !ok {
130+
t.Errorf("test has template %s, code does not", k)
131+
}
132+
}
133+
// now check all the known templates, in alphabetic order, for determinacy
134+
keys := []string{}
135+
for k := range templates {
136+
keys = append(keys, k)
137+
}
138+
sort.Strings(keys)
139+
for _, k := range keys {
140+
v := templates[k]
141+
// the FuncMap is an annoyance; should not be necessary
142+
if err := templatecheck.CheckHTML(v.tmpl, v.data); err != nil {
143+
t.Errorf("%s: %v", k, err)
144+
}
145+
}
146+
}
147+
148+
func callsOf(p *packages.Package, tree *ast.File, name string) []*ast.CallExpr {
149+
var ans []*ast.CallExpr
150+
f := func(n ast.Node) bool {
151+
x, ok := n.(*ast.CallExpr)
152+
if !ok {
153+
return true
154+
}
155+
if y, ok := x.Fun.(*ast.Ident); ok {
156+
if y.Name == name {
157+
ans = append(ans, x)
158+
}
159+
}
160+
return true
161+
}
162+
ast.Inspect(tree, f)
163+
return ans
164+
}
165+
func treeOf(p *packages.Package, fname string) *ast.File {
166+
for _, tree := range p.Syntax {
167+
loc := tree.Package
168+
pos := p.Fset.PositionFor(loc, false)
169+
if strings.HasSuffix(pos.Filename, fname) {
170+
return tree
171+
}
172+
}
173+
return nil
174+
}

internal/lsp/debug/rpc.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"golang.org/x/tools/internal/lsp/debug/tag"
2121
)
2222

23-
var rpcTmpl = template.Must(template.Must(baseTemplate.Clone()).Parse(`
23+
var RPCTmpl = template.Must(template.Must(BaseTemplate.Clone()).Parse(`
2424
{{define "title"}}RPC Information{{end}}
2525
{{define "body"}}
2626
<H2>Inbound</H2>
@@ -44,7 +44,7 @@ var rpcTmpl = template.Must(template.Must(baseTemplate.Clone()).Parse(`
4444
{{end}}
4545
`))
4646

47-
type rpcs struct {
47+
type Rpcs struct { // exported for testing
4848
mu sync.Mutex
4949
Inbound []*rpcStats // stats for incoming lsp rpcs sorted by method name
5050
Outbound []*rpcStats // stats for outgoing lsp rpcs sorted by method name
@@ -79,7 +79,7 @@ type rpcCodeBucket struct {
7979
Count int64
8080
}
8181

82-
func (r *rpcs) ProcessEvent(ctx context.Context, ev core.Event, lm label.Map) context.Context {
82+
func (r *Rpcs) ProcessEvent(ctx context.Context, ev core.Event, lm label.Map) context.Context {
8383
r.mu.Lock()
8484
defer r.mu.Unlock()
8585
switch {
@@ -152,7 +152,7 @@ func endRPC(ctx context.Context, ev core.Event, span *export.Span, stats *rpcSta
152152
}
153153
}
154154

155-
func (r *rpcs) getRPCSpan(ctx context.Context, ev core.Event) (*export.Span, *rpcStats) {
155+
func (r *Rpcs) getRPCSpan(ctx context.Context, ev core.Event) (*export.Span, *rpcStats) {
156156
// get the span
157157
span := export.GetSpan(ctx)
158158
if span == nil {
@@ -163,7 +163,7 @@ func (r *rpcs) getRPCSpan(ctx context.Context, ev core.Event) (*export.Span, *rp
163163
return span, r.getRPCStats(span.Start())
164164
}
165165

166-
func (r *rpcs) getRPCStats(lm label.Map) *rpcStats {
166+
func (r *Rpcs) getRPCStats(lm label.Map) *rpcStats {
167167
method := tag.Method.Get(lm)
168168
if method == "" {
169169
return nil
@@ -209,7 +209,7 @@ func getStatusCode(span *export.Span) string {
209209
return ""
210210
}
211211

212-
func (r *rpcs) getData(req *http.Request) interface{} {
212+
func (r *Rpcs) getData(req *http.Request) interface{} {
213213
return r
214214
}
215215

0 commit comments

Comments
 (0)