本記事はGo3 Advent Calendar 2019の12日目の記事です。前回の記事は Goで別パッケージの関数呼び出しは禁止すべき でした。
概要
gormは便利ですが、色々な記述の仕方ができたり、エラーの検知がしづらかったりと意図しないコードを作ってしまい、それがまたコンパイルを通ってしまうことがあります。この記事ではそれを静的解析で検知できないかというアプローチをします。
gormで困る事
gormで困る事といえば、色々な書き方ができてしまうという事と、明らかにおかしな処理でもコンパイルを通って動いてしまうということがあります。
例えば、
db = db.Where(“条件A”)
db = db.Where(“条件B”)
db = db.Find(&someModel)
という処理は
db = db.Where(“条件A”).Where(“条件B”).Find(&someModel)
ともかけるし、
db = db.Where(“条件A”)
db = db.Find(&someModel,“条件B”)
とも書くことができます。また、
db = db.Where(“条件A”)
db = db.Find(&someModel)
db = db.Where(“条件B”)
と書いてもコンパイルは通ってしまいます。
苦肉の策として、以下のようにルールづけをして業務を行ってますが、なんとか機械的に検知したいところです。
作成できたもの
まだ作成途中ですが、パイプを使った書き方についてについては検知ができました！以下で公開をしています。今後機能を追加していく予定です。
作成するまでの流れ
Analyzerのスケルトンコードを作る
gostaticanalysis/skeletonというツールででベースとなるコード一式を作ることができ、非常に便利です。
今回作成したツールもメインとなるgormchecker.goと、テストデータ以外変更をせずに作成できています。
go get -u github.com/gostaticanalysis/skeleton
skeleton gormchecker
一番シンプルなチェッカー
以下で、変数名のチェックができています。動作確認はgo testで行えます。これをベースに作っていきます。
func run(pass *analysis.Pass) (interface{}, error) {
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
nodeFilter := []ast.Node{
(*ast.Ident)(nil),
}
inspect.Preorder(nodeFilter, func(n ast.Node) {
switch n := n.(type) {
case *ast.Ident:
if n.Name == "Gopher" {
pass.Reportf(n.Pos(), "name of identifier must not be ’Gopher’")
}
}
})
return nil, nil
}
package a
var Gopher int // want "name of identifier must not be ’Gopher’"
var gopher int // OK
作るまでのアプローチ
実際に意図した記述と、意図していない記述を書いてその構造上の違いをみていくのがわかりやすそうです。
まず以下のようにテストを書きます。パイプを検知できればゴールです。
func check() {
db, _ := getConnection()
db = db.Where("column_a = xxx")
db = db.Where("column_a = xxx").Where("column_b = xxx")
}
ast.Printを使えばデバッグができますが、大量に出て読みづらいので以下のようにデバッグを入れます。
func run(pass *analysis.Pass) (interface{}, error) {
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
nodeFilter := []ast.Node{
(*ast.Ident)(nil),
}
inspect.Preorder(nodeFilter, func(n ast.Node) {
//デバッグ追加ここから
fmt.Printf("### pos %v\n", n.Pos())
position := pass.Fset.Position(n.Pos())
fmt.Println(position)
ast.Print(pass.Fset, n)
//デバッグ追加ここまで
switch n := n.(type) {
case *ast.Ident:
if n.Name == "Gopher" {
pass.Reportf(n.Pos(), "name of identifier must not be ’Gopher’")
}
}
})
return nil, nil
}
これでデバッグしつつ進めることができそうです。
### pos 3480661
/Users/gosagawa/go/src/github.com/gosagawa/gormchecker/testdata/src/a/a.go:77:10
0 *ast.Ident {
1 . NamePos: /Users/gosagawa/go/src/github.com/gosagawa/gormchecker/testdata/src/a/a.go:77:10
2 . Name: "Where"
3 }
### pos 3480685
/Users/gosagawa/go/src/github.com/gosagawa/gormchecker/testdata/src/a/a.go:77:34
0 *ast.Ident {
1 . NamePos: /Users/gosagawa/go/src/github.com/gosagawa/gormchecker/testdata/src/a/a.go:77:34
2 . Name: "Where"
3 }
根本に戻って、パイプを検知するためには一行で二回呼んでいる関数を検知すればいけそうです。*ast.Identでフィルタリングしているものは変数なので、関数を検知する必要がありそうです。ast.Nodeインターフェースを実装する型をみた際*ast.CallExprがそのようです。nodefilterを書き換えます。
nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
}
すると、以下のような構造になっていることがわかります。非常に多くの情報を持っていますが、
(*ast.CallExpr).Fun.X.Nameで呼び出しもと、(*ast.CallExpr).Fun.Sel.Nameで関数名を取れそうです。
ast.SelectorExprとはxxx.yyyというようなセレクターを示すノードであることも知ることができます。
/Users/gosagawa/go/src/github.com/gosagawa/gormchecker/testdata/src/a/a.go:77:7
0 *ast.CallExpr {
1 . Fun: *ast.SelectorExpr {
2 . . X: *ast.Ident {
3 . . . NamePos: /Users/gosagawa/go/src/github.com/gosagawa/gormchecker/testdata/src/a/a.go:77:7
4 . . . Name: "db"
5 . . . Obj: *ast.Object {
6 . . . . Kind: var
7 . . . . Name: "db"
（中略）
104 . . }
105 . . Sel: *ast.Ident {
106 . . . NamePos: /Users/gosagawa/go/src/github.com/gosagawa/gormchecker/testdata/src/a/a.go:77:10
107 . . . Name: "Where"
108 . . }
109 . }
110 . Lparen: /Users/gosagawa/go/src/github.com/gosagawa/gormchecker/testdata/src/a/a.go:77:15
111 . Args: []ast.Expr (len = 1) {
112 . . 0: *ast.BasicLit {
113 . . . ValuePos: /Users/gosagawa/go/src/github.com/gosagawa/gormchecker/testdata/src/a/a.go:77:16
114 . . . Kind: STRING
115 . . . Value: "\"column_a = xxx\""
116 . . }
117 . }
118 . Ellipsis: -
119 . Rparen: /Users/gosagawa/go/src/github.com/gosagawa/gormchecker/testdata/src/a/a.go:77:32
120 }
あとは以下のようにして、同じ行の関数の数を調べればやりたいことが実現できました。本当はdbという変数がgorm.DBであるかをチェックできれば良いのですが、そこまではできておらず変数名がdbか？という苦肉の策をとってます。
func run(pass *analysis.Pass) (interface{}, error) {
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
}
functions := make(map[string]map[int][]string)
inspect.Preorder(nodeFilter, func(n ast.Node) {
switch n := n.(type) {
case *ast.CallExpr:
switch f := n.Fun.(type) {
case *ast.SelectorExpr:
if x, ok := f.X.(*ast.Ident); ok && x.Name != "db" {
break
}
functionName := f.Sel.Name
if _, ok := functions[position.Filename]; !ok {
functions[position.Filename] = make(map[int][]string)
}
functions[position.Filename][position.Line] = append(functions[position.Filename][position.Line], functionName)
if len(functions[position.Filename][position.Line]) > 1 {
pass.Reportf(n.Pos(), "do not use pipe")
}
}
}
})
return nil, nil
}
今後の展開
今できるのはパイプの検知のみですが、以下のケースを関数の位置、パラメータ数などを取得することにより取得できそうです。
- 関数内でfindを複数回呼ばない、findの後にWhere等他の関数を書かない
- findのなかに条件式を入れない
- スライスを取得するものにfirstを使わない(一件しか使わない）
まとめ
静的解析というと非常に難しい印象だったのですが、このようにテンプレートがあった上で、デバッグをしながらやると比較的容易に構文を把握しつつ静的解析ツールが作れてしまうので素晴らしいなと思います。gormchecker自体は業務で使えるレベルにすべくより機能を加えていければと思います。
謝辞
静的解析自体は以下の本を非常に参考にさせていただきました。
この記事では静的解析自体の細かな説明は省きましたが、原理や仕組みが丁寧に解説されています。
逆引きGoによる静的解析入門
https://knsh14.booth.pm/items/1319336
また、gcpub/zaganeというspannerの問題のあるコードを検知するツールが、やりたいと思う事と近く参考になりました。
https://github.com/gcpug/zagane
ありがとうございます！