SODA Engineering Blog
🛠️

Goで不正なreturn errを検知する静的解析ツールを作りました!

2023/12/17に公開

作ったもの

https://github.com/snkrdunk/empty_err_checker

どんなツール?

Goでエラーハンドリングを行う際に、以下のようにerrがnilかどうかをチェックしてreturnするコードがよく書かれます。

if err := doSomething(); err != nil {
	return err
}

このように条件でnilチェックをしていれば問題ないのですが、nilチェックせずにreturn errする場合に戻り値がnilとなる可能性があり不具合が発生するコードになってしまいます。(具体的なコードは後述してます)
empty_err_checkerはそのようなコードを検知する静的解析ツールです。

empty_err_checkerを作った動機

以下のようなコードによる不具合が発生しました。(実際のコードではありません)

func UpdateOrders() error {
  orders, err := getOrders()
  if err != nil {
    return err
  }
  
  ticker := time.NewTicker(1 * time.Second)
  defer ticker.Stop()
  var count int
  
  for i := range orders {
    preUpdate := func() error {
      count++
      invalidStatus := checkStatus(orders[i])
      if invalidStatus {
        return err // return errors.New("invalid status") を返すのが正しい
      }
      if err := preUpdateOrder(orders[i]); err != nil {
        return err
      }
      return nil
    }

    if count < 5 {
      if err := preUpdate(orders[i]); err != nil {
        continue
      }
    } else {
      <-ticker.C
      count = 0
      if err := preUpdate(orders[i]); err != nil {
        continue
      }
    }

    if err := postUpdate(orders[i]); err != nil {
      continue
    }
  }
  return nil
}

このコードの問題はpreUpdate関数の中でinvalidStatustrueの場合にreturn errしていることです。本来はinvalidStatusなのでerrorを返さなければいけませんが、nilerr変数を返却しているので、後続のpostUpdate関数まで実行されてしまいます。
このレベルの不具合であればテストコードやレビューで気付くべきですが、このコードのようにすり抜けてくる可能性もあるので静的解析ツールを作ろうと思いました。
あとは社内でGoの静的解析勉強会も行われていたので、ツールを作ってみたかったというモチベーションもありました。

skeletonについて

skeletonは静的解析ツールに必要なファイル群を一括で作成してくれる便利なツールです。指定したGoのモジュール名に対してskeletonコマンドを実行すると、必要な実装ファイルやテストコードを生成してくれます。--pluginオプションをつけると、golangci-lintのプラグインに必要な実装ファイルも自動的に追加してくれます。

$ skeleton github.com/snkrdunk/empty_err_checker

skeletonはテストコードも自動生成してくれます。
解析対象にしたいソースコードをtestdata/src配下に置いてテストを実行します。
テストコードはではerrorを検知したい箇所に// want "error message"のようなコメントを入れておくだけで良いので、直感的で分かりやすいです。

func invalidErrChecker() error {
	var err error
	isValid := isValid()
	if !isValid {
		return err // want "returned error is not checked."
	}
	return nil
}

empty_err_checkerのテストコード

実装解説

静的解析の実行にはsinglechecker.Mainを使っています。
singlechecker.Mainは単一のanalysis.Analyzerを実行する時に使います。

func main() { singlechecker.Main(empty_err_checker.Analyzer) }

ツールの実装に関してはanalysis.AnalyzerのRunフィールドに渡す関数を実装していきます。

// Runのsignature
Run func(*Pass) (interface{}, error)

emtpy_err_checkerのanalysis.Analyzer

analysis.AnalyzerRequiresフィールドでは依存するanalysis.Analyzerを指定でき、実装したanalyzerよりも先に実行されます。
empty_err_checkerではinspect.Analyzerを指定していて、これはast探索を最適化してくれるanalyzerです。

https://github.com/snkrdunk/empty_err_checker/blob/main/empty_err_checker.go#L11-L18

run関数

run関数の処理はざっくり以下のようなになってます。

  • ast.IfStmtだけを対象に処理
  • stackから親のast.IfStmtを取得してparentIfStmtSliceに詰める
  • current(ast.Node)parentIfStmtSliceを渡してgetEmptyErrReturnStmtを実行
  • getEmptyErrReturnStmtのresponseがnilでない場合はレポートする

pass.ResultOfにはRequiresで指定した事前に実行されるAnalyzerの解析結果が入っています。
なのでpass.ResultOf[inspect.Analyzer].(*inspector.Inspector)で取り出した、inspector.Inspectorstructを使ってast探索することができます。
nodeFilterは、解析するastの探索対象にしたいast.Nodeをフィルタリングするために使います。

https://github.com/snkrdunk/empty_err_checker/blob/main/empty_err_checker.go#L20-L48

inspector.Inspector

inspector.Inspectorでast探索する場合は以下3つのメソッドが使えます。
第一引数のtypesで特定のast.Nodeを指定することでフィルタリングできるのと、深さ優先探索でASTを探索するという共通点以外の差分は↓のようになってます。

Preorder

func (in *Inspector) Preorder(types []ast.Node, f func(ast.Node))

Preorderは事前にトラバースされた順に全てのast.Nodeを探索します。

Nodes

func (in *Inspector) Nodes(types []ast.Node, f func(n ast.Node, push bool) (proceed bool))

NodesPreorderと基本は同じですが、第二引数で指定している関数のsignatureが違います。
子のast.Node探索前にf(ast.Node, true)が呼ばれます。ffalseを返却する場合は、子のast.Nodeのサブツリーは探索されないようになっています。
またツリーの探索を抜ける時には、f(ast.Node, false)が呼ばれます。
この仕組みによって、子のサブツリーまで探索する必要ない場合の制御などが可能になっています。(そのような理解をしています)

WithStack

func (in *Inspector) WithStack(types []ast.Node, f func(n ast.Node, push bool, stack []ast.Node) (proceed bool))

WithStackは基本的にNodesと同じですが、第二引数で指定している関数のsignatureが違います。
fの第三引数で[]ast.Node型で探索中のast.Nodeに対する親ast.Nodeのstack情報を受け取れます。
empty_err_checkerでは、return errしてるast.Nodeを起点にif文の条件式をチェックしており、if文がネストしてる場合などを考慮して事前にstackから親のast.IfStmtを全て取得してます。

getEmptyErrReturnStmt関数

この関数ではまず引数で受け取った*ast.IfStmtBody.Listをループして、*ast.ReturnStmtを取得してます。
その後は↓のバリデーション関数を通してます。

  • isReturnErr
    • errという変数名でerror型をreturnしてるか確認
  • isCheckedIfErrIsNil
    • return errしてる場合にifの条件でerrのnilチェックをしてるか確認
  • isAssignedErr
    • ifブロックの中でerr変数に代入されているか確認

currentの*ast.IfStmtで↑の3つを通過した場合は、親の*ast.IfStmtでもnilチェックされてるかなどをループを回して確認してます。

https://github.com/snkrdunk/empty_err_checker/blob/main/empty_err_checker.go#L50-L76

ast.IfStmtのstructは以下のようになっていて、Bodyからはifがtrueの場合のコードブロックを解析することができます。
elseのコードブロックはElseフィールドから解析できます。

// An IfStmt node represents an if statement.
IfStmt struct {
	If   token.Pos // position of "if" keyword
	Init Stmt      // initialization statement; or nil
	Cond Expr      // condition
	Body *BlockStmt
	Else Stmt // else branch; or nil
}

BlockStmtのstructは以下のようになっていて、ListのStmtinterface{}なので適宜TypeAssertionして使う必要があります。getEmptyErrReturnStmtは*ast.ReturnStmtにTypeAssertionしてます。

// A BlockStmt node represents a braced statement list.
BlockStmt struct {
	Lbrace token.Pos // position of "{"
	List   []Stmt
	Rbrace token.Pos // position of "}", if any (may be absent due to syntax error)
}

isReturnErr関数

この関数は第二引数で受ける*ast.ReturnStmtからerror型のerrという変数をreturnしているか確認しています。

https://github.com/snkrdunk/empty_err_checker/blob/main/empty_err_checker.go#L78-L91

isCheckedIfErrIsNil関数

これはifの条件式でerr != nilのチェックがある場合はtrueを返却する関数です。
条件式はast.IfStmt.Condから取得でき、型はExprというinterface{}です。
err != nilのような条件の場合Condの型は*ast.BinaryExprになります。
printすると以下のような構造であることがわかります。

// formatが%+vの出力結果
&{X:err OpPos:3771754 Op:!= Y:nil}

// formatが%#vの出力結果
&ast.BinaryExpr{X:(*ast.Ident)(0x1400336fce0), OpPos:3771754, Op:44, Y:(*ast.Ident)(0x1400336fd00)}

条件式が全てerr != nilであれば、X, Yをそれぞれ*ast.IndentにTypeAssertionして、その文字列をチェックすれば良いのですが、err != nil && isValidのように条件式が複数ケースも考慮する必要があります。
err != nil && isValidのように条件が複数の場合は、↓のような構造になります

// formatが%+vの出力結果
&{X:0x14002fffbc0 OpPos:3771765 Op:&& Y:isValid}

// formatが%#vの出力結果
&ast.BinaryExpr{X:(*ast.BinaryExpr)(0x14002fffbc0), OpPos:3771765, Op:34, Y:(*ast.Ident)(0x140035e20a0)}

Yは*ast.Identなので文字列を取得して条件式をチェックすることができますが、Xはまだ*ast.BinaryExprなのでX,Yを取り出して*ast.IndetにTypeAssertionする必要があります。
このように複数条件式の場合は、*ast.BinaryExprのXとYがそれぞれ*ast.IdentにTypeAssertionできるまで再起的にチェックする必要があるので、isCheckedIfErrIsNilはそのような実装にしました。

https://github.com/snkrdunk/empty_err_checker/blob/main/empty_err_checker.go#L93-L115

第一引数で受け取るcondの型が*ast.BinaryExprの場合は、XとYでそれぞれ再起的にisCheckedIfErrIsNilを実行して、err != nilの条件が含まれているかどうか確認してます。

isAssignedErr関数

この関数は引数で受け取る*ast.IfStmtを使って、ifのコードブロックの中でerrという変数に値が代入されているかどうか確認しています。
https://github.com/snkrdunk/empty_err_checker/blob/main/empty_err_checker.go#L117-L133
empty_err_checkerはifの条件でerrのnilチェックをしないでreturn errしてるコードを検知しますが、ifのブロックの中でerrに代入されてる場合もあるので、それを判定するための関数を実装しました。
具体例としてテストコードを貼っておきます。
https://github.com/snkrdunk/empty_err_checker/blob/main/testdata/src/a/a.go#L16-L24

感想

初めて静的解析ツール作成してみましたが、skeletonやinspectorのおかげでそこまで苦労せずに作れた印象でした。
まだまだastパッケージまわり理解できない箇所あるので引き続きツール作成して勉強していきたいです。
あとツール自体は結構前に作っていて、どんな実装か思い出しながら記事書くのが辛かったので、次回からはすぐ記事くようにしたいと思いました。

SODA Engineering Blog
SODA Engineering Blog

Discussion