Skip to content

Commit ce06cfc

Browse files
committed
Basic typechecker
1 parent 2a11246 commit ce06cfc

File tree

6 files changed

+185
-0
lines changed

6 files changed

+185
-0
lines changed

analysis/typecheck/typecheck.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package typecheck
2+
3+
import (
4+
"fmt"
5+
"go/ast"
6+
"go/types"
7+
8+
"golang.org/x/tools/go/analysis"
9+
"golang.org/x/tools/go/analysis/passes/inspect"
10+
"golang.org/x/tools/go/ast/inspector"
11+
"golang.org/x/tools/go/types/typeutil"
12+
)
13+
14+
const Doc = `check bigslice exec call arguments
15+
16+
Basic typechecker for bigslice programs that inspects session.Run and
17+
session.Must calls to ensure the arguments are compatible with the Func.
18+
Checks are limited by static analysis and are best-effort. For example, the call
19+
session.Must(ctx, chooseFunc(), args...)
20+
cannot be checked, because it uses chooseFunc() instead of a simple identifier.
21+
22+
Typechecking does not include any slice operations yet.`
23+
24+
var Analyzer = &analysis.Analyzer{
25+
Name: "bigslice_typecheck",
26+
Doc: Doc,
27+
Requires: []*analysis.Analyzer{inspect.Analyzer},
28+
Run: run,
29+
}
30+
31+
const (
32+
bigslicePkgPath = "github.com/grailbio/bigslice"
33+
execPkgPath = "github.com/grailbio/bigslice/exec"
34+
funcValueTypeString = "*github.com/grailbio/bigslice.FuncValue"
35+
)
36+
37+
func run(pass *analysis.Pass) (interface{}, error) {
38+
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
39+
40+
// funcTypes describes the types of declared bigslice.FuncValues.
41+
// TODO: As stated below, we're only recording top-level func for now.
42+
// If that changes, this map should be keyed by a global identifier, not *ast.Ident.
43+
// TODO: We may also want to return these types as Facts to allow checking across packages.
44+
funcTypes := map[string]*types.Signature{}
45+
46+
// Collect the types of bigslice.Funcs.
47+
// TODO: ValueSpec captures top-level vars, but maybe we should include non-top-level ones, too.
48+
inspect.Preorder([]ast.Node{&ast.ValueSpec{}}, func(node ast.Node) {
49+
valueSpec := node.(*ast.ValueSpec)
50+
for i := range valueSpec.Values {
51+
// valueType := pass.TypesInfo.TypeOf(valueSpec.Values[i])
52+
// if valueType.String() != funcValueTypeString {
53+
// continue
54+
// }
55+
call, ok := valueSpec.Values[i].(*ast.CallExpr)
56+
if !ok {
57+
continue
58+
}
59+
fn := typeutil.StaticCallee(pass.TypesInfo, call)
60+
if fn == nil {
61+
continue
62+
}
63+
if fn.Pkg().Path() != bigslicePkgPath || fn.Name() != "Func" {
64+
continue
65+
}
66+
if len(call.Args) != 1 {
67+
panic(fmt.Errorf("unexpected arguments to bigslice.Func: %v", call.Args))
68+
}
69+
funcType := pass.TypesInfo.TypeOf(call.Args[0]).Underlying().(*types.Signature)
70+
funcTypes[valueSpec.Names[i].Name] = funcType
71+
}
72+
})
73+
74+
inspect.Preorder([]ast.Node{&ast.CallExpr{}}, func(node ast.Node) {
75+
call := node.(*ast.CallExpr)
76+
fn := typeutil.StaticCallee(pass.TypesInfo, call)
77+
if fn == nil {
78+
return
79+
}
80+
if fn.Pkg().Path() != execPkgPath || (fn.Name() != "Must" && fn.Name() != "Run") {
81+
return
82+
}
83+
84+
funcValueIdent, ok := call.Args[1].(*ast.Ident)
85+
if !ok {
86+
// This function invocation is more complicated than a simple identifier.
87+
// Give up on typechecking this call.
88+
return
89+
}
90+
funcType, ok := funcTypes[funcValueIdent.Name]
91+
if !ok {
92+
// TODO: Propagate bigslice.Func types as Facts so we can do a better job here.
93+
return
94+
}
95+
96+
wantArgTypes := funcType.Params()
97+
gotArgs := call.Args[2:]
98+
if want, got := wantArgTypes.Len(), len(gotArgs); want != got {
99+
pass.Report(analysis.Diagnostic{
100+
Pos: funcValueIdent.Pos(),
101+
End: funcValueIdent.End(),
102+
Message: fmt.Sprintf(
103+
"bigslice type error: %s requires %d arguments, but got %d",
104+
funcValueIdent.Name, want, got),
105+
})
106+
return
107+
}
108+
109+
for i, gotArg := range gotArgs {
110+
wantType := wantArgTypes.At(i).Type()
111+
gotType := pass.TypesInfo.TypeOf(gotArg)
112+
if !types.AssignableTo(gotType, wantType) {
113+
pass.Report(analysis.Diagnostic{
114+
Pos: gotArg.Pos(),
115+
End: gotArg.End(),
116+
Message: fmt.Sprintf(
117+
"bigslice type error: func %q argument %q requires %v, but got %v",
118+
funcValueIdent.Name, wantArgTypes.At(i).Name(), wantType, gotType),
119+
})
120+
}
121+
}
122+
})
123+
124+
return nil, nil
125+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package typecheck_test
2+
3+
// TODO: Figure out how to test this analyzer.
4+
// The golang.org/x/tools/go/analysis/analysistest testing tools set up a
5+
// "GOPATH-style project" [1], which seems to require vendoring dependencies.
6+
// This is ok for testing the Go standard library but isn't practical for
7+
// code with several transitive dependencies, as bigslice has.
8+
// [1] https://pkg.go.dev/golang.org/x/[email protected]/go/analysis/analysistest?tab=doc
9+
//
10+
// Until then, run and see no errors for urls but a type error for wrongarg:
11+
// $ go run github.com/grailbio/bigslice/cmd/slicetypecheck \
12+
// github.com/grailbio/bigslice/cmd/urls \
13+
// github.com/grailbio/bigslice/analysis/typecheck/typechecktest/wrongarg
14+
// <snip>/wrongarg.go:22:34: bigslice type error: func "testFunc" argument "argInt" requires int, but got string
15+
// exit status 3
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Tests the static slice typechecker.
2+
//
3+
// This is a correctly-typed Go program (even though it's incomplete and thus
4+
// not runnable, for simplicity). However, its bigslice Func invocation is
5+
// incorrectly typed, and the static typechecker find that, in this case.
6+
package main
7+
8+
import (
9+
"context"
10+
11+
"github.com/grailbio/bigslice"
12+
"github.com/grailbio/bigslice/exec"
13+
)
14+
15+
var testFunc = bigslice.Func(func(argInt int, argString string) bigslice.Slice {
16+
return nil
17+
})
18+
19+
func main() {
20+
ctx := context.Background()
21+
var session *exec.Session
22+
_ = session.Must(ctx, testFunc, "i should be an int", "i'm ok")
23+
}

cmd/slicetypecheck/main.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Standalone bigslice typechecker (alpha).
2+
//
3+
// This is new and in testing. Please report any issues:
4+
// https://github.com/grailbio/bigslice/issues.
5+
//
6+
// TODO: Consider merging this into the main `bigslice` command, when it's well-tested.
7+
// TODO: Consider supporting the golangci-lint plugin interface.
8+
package main
9+
10+
import (
11+
"github.com/grailbio/bigslice/analysis/typecheck"
12+
"golang.org/x/tools/go/analysis/singlechecker"
13+
)
14+
15+
func main() {
16+
singlechecker.Main(typecheck.Analyzer)
17+
}

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,5 @@ require (
1010
github.com/grailbio/testutil v0.0.3
1111
github.com/spaolacci/murmur3 v1.1.0
1212
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
13+
golang.org/x/tools v0.0.0-20190911174233-4f2ddba30aff
1314
)

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ github.com/grailbio/bigmachine v0.5.7 h1:RaYi4wa4el62yqrw1qTB+KVmmlcS8VhDy59/l+8
101101
github.com/grailbio/bigmachine v0.5.7/go.mod h1:wvOUthoZPxKKJ829ClWaO/uRTxaW3DLm7LyUQHO8ed0=
102102
github.com/grailbio/bigmachine v0.5.8 h1:LNOiBTPjk6P8JODqqItcysRzftEPhE59B1wSlKnpGy4=
103103
github.com/grailbio/bigmachine v0.5.8/go.mod h1:O9UMGp6FPD6dRJhpIImjnTs8uFG8smEDYSqdIoadS+Y=
104+
github.com/grailbio/testutil v0.0.1/go.mod h1:j7teGaXqRY1n6m7oM8oy954lxL37Myt7nEJZlif3nMA=
105+
github.com/grailbio/testutil v0.0.3 h1:Um0OOTtYVvyxwQbO48K3t6lNmLPY4sL3Vn6Sw0srNy8=
106+
github.com/grailbio/testutil v0.0.3/go.mod h1:f9+y7xMXeXwyNcdV5cmo6GzRiitSOubMmqcqEON7NQQ=
104107
github.com/grailbio/v23/factories/grail v0.0.0-20190904050408-8a555d238e9a h1:kAl1x1ErQgs55bcm/WdoKCPny/kIF7COmC+UGQ9GKcM=
105108
github.com/grailbio/v23/factories/grail v0.0.0-20190904050408-8a555d238e9a/go.mod h1:2g5HI42KHw+BDBdjLP3zs+WvTHlDK3RoE8crjCl26y4=
106109
github.com/hanwen/go-fuse v1.0.0/go.mod h1:unqXarDXqzAk0rt98O2tVndEPIpUgLD9+rwFisZH3Ok=
@@ -256,6 +259,7 @@ golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b/go.mod h1:/rFqwRUd4F7ZHNgw
256259
golang.org/x/tools v0.0.0-20190621195816-6e04913cbbac/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
257260
golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
258261
golang.org/x/tools v0.0.0-20190816200558-6889da9d5479/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
262+
golang.org/x/tools v0.0.0-20190911174233-4f2ddba30aff h1:On1qIo75ByTwFJ4/W2bIqHcwJ9XAqtSWUs8GwRrIhtc=
259263
golang.org/x/tools v0.0.0-20190911174233-4f2ddba30aff/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
260264
golang.org/x/tools v0.0.0-20191130070609-6e064ea0cf2d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
261265
golang.org/x/tools v0.0.0-20200227200655-6862ededa516/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=

0 commit comments

Comments
 (0)