Goで静的解析してlinterを作る

STORES 予約でwebアプリケーションエンジニアをやっております。ykpythemindです。

本記事は hey アドベントカレンダー2020の11日目です。

概要

heyではOpenAPI(Swagger)の導入が行われています。

STORES 予約チームでも一部で使用し始めており、Stoplight Studioの使用感が素晴らしく、チームでの開発がしやすくなっていることを実感しております。自動生成によりスキーマと実装の差が生じない状態が理想ですが、まだどうしてもツール側が追従できず "運用でカバー" せざるを得ない部分も存在します。私たちが実際に直面した問題としては、サーバサイドの自動生成コードでは型情報の制約が弱く、目視でのコードレビューが必須そうになりそうな点がありました。

今回はそのあたりの運用でカバー用のツールとしてGo用のlinter作成にトライしてみます。 *1

oapi-codegenとその課題点

前提としてサーバサイドの実装としてはGo を用い、ジェネレータとしては deepmap/oapi-codegen を使用します。 *2

future-architect.github.io

こちらの記事に詳しい解説があり参考になります。私もおおむねpros/consに同じことを感じており、とくにリクエスト/レスポンスの定義がBindされないという部分が欠点だと思います。

例として、以下はoapi-codegenで実際に生成したコードです。

// CreateUserRequest defines model for CreateUserRequest.
type CreateUserRequest struct {
    Email *string `json:"email,omitempty"`
    Name  *string `json:"name,omitempty"`
}

// CreateUserResponse defines model for CreateUserResponse.
type CreateUserResponse struct {
    User *User `json:"user,omitempty"`
}

// ...

// User defines model for User.
type User struct {
    Email openapi_types.Email `json:"email"`

    // Unique identifier for the given user.
    Id   int     `json:"id"`
    Name *string `json:"name,omitempty"`
}

// ServerInterface represents all server handlers.
type ServerInterface interface {
    // Create New User
    // (POST /user)
    CreateUser(w http.ResponseWriter, r *http.Request)
    // List Users
    // (GET /users)
    GetUsers(w http.ResponseWriter, r *http.Request)
}

ServerInterfaceが各operationを表しており、このインターフェースを満たす実装をすることで各エンドポイントにルーティングされます。ここで問題なのは、CreateUserRequest CreateUserResponse などの構造体を各handler内でよしなに使用してbindするしかないということです。普通に使用する構造体を間違えてしまいそうです。

oapi-codegenのREADMEでは

This package tries to be too simple rather than too generic, so we've made some design decisions in favor of simplicity, knowing that we can't generate strongly typed Go code for all possible OpenAPI Schemas.

としており、生成コード/実装のシンプルさを重要視している為、このような生成コードになるようです。

ここで思いついたのが、この生成コードのシンプルさを逆手に取り、 CreateUser(w http.ResponseWriter, r *http.Request) handler func内では必ずCreateUserRequest及びCreateUserResponse構造体を使用するというお約束を静的解析できればある程度運用でカバーできるのでは？？ということでした。目視ではカバーしきれなさそうなので、linterで指摘するようにしてしまいましょう。

誤った構造体にbindしてしまう事故 *3 を防いだり、xxxRequest/xxxResponse等の命名規則でスキーマを定義するというお約束を担保することはできそうです。

analysisパッケージを使う

Goには準公式の https://pkg.go.dev/golang.org/x/tools/go/analysis が存在し、これを使うことで静的解析ツールを簡単に作成することができます。

budougumi0617.github.io

こちらの記事が分かりやすいです。簡潔にまとめると、analysis.Pass 構造体や inspector.Inspector を使いASTを探索し、適宜reportすれば良いということになります。GoのASTは

monpoke1.hatenablog.com

こちらを眺めて理解したつもりになっておきましょう。(理解できてません)

実際にlinterを作成していきます。

まずは自動生成コードから ServerInterface の定義を抜き出し、必要なoperation idのリストを作っておきます。

package apirule

import (
    "fmt"
    "go/ast"
    "go/parser"
    "go/token"
    "io/ioutil"
    "os"
    "sync"

    "golang.org/x/tools/go/analysis"
    "golang.org/x/tools/go/analysis/passes/inspect"
    "golang.org/x/tools/go/ast/inspector"
)

var once = new(sync.Once)
var operationIdSet = make(map[string]struct{})

func parseOperationIds() map[string]struct{} {
    // run(pass *analysis.Pass)は複数回呼ばれるので最初の一回だけパースを実行する
    once.Do(func() {
        f, err := os.Open("apispec/api.gen.go") // apispecから自動生成したファイル
        if err != nil {
            panic(err)
        }
        defer f.Close()

        b, err := ioutil.ReadAll(f)
        if err != nil {
            panic(err)
        }

        fset := token.NewFileSet()
        specfile, err := parser.ParseFile(fset, "", b, parser.Mode(0))
        if err != nil {
            panic(err)
        }

        ast.Inspect(specfile, func(n ast.Node) bool {
            if typespec, ok := n.(*ast.TypeSpec); ok {
                if typespec.Name.Name == "ServerInterface" {
                    if iftype, ok := typespec.Type.(*ast.InterfaceType); ok {
                        for _, id := range getMethodNamesOfInterfaceType(iftype) {
                            operationIdSet[id] = struct{}{}
                        }
                    }
                }
            }
            return true
        })
    })

    return operationIdSet
}

func getMethodNamesOfInterfaceType(iftype *ast.InterfaceType) (list []string) {
    // いくつかのnilチェックを省略しているが、自動生成コードなので必ず存在するはずである
    for _, method := range iftype.Methods.List {
        name := method.Names[0].Name
        list = append(list, name)
    }
    return
}

ServerInterfaceという名前のinterfaceを探し、メソッドリストらしき構造から名前を抜き出しています。ちなみにどう走査すればよいのか分からなかったので ast.Print を用いてプリントデバッグしまくっています。

このoperationIDリストを用い、operationIDの名前を持つHandlerFunc内から operationId + Request operationID + Response の名前を持つstructが使われているかどうかを探します。

func run(pass *analysis.Pass) (interface{}, error) {
    operationIds := parseOperationIds()

    inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)

    // handlerはpackage handler内に書くよという自分たちの都合なので、コードの都合によって書き換える
    if pass.Pkg.Name() != "handler" {
        return nil, nil
    }

    nodeFilter := []ast.Node{
        (*ast.FuncDecl)(nil),
    }

    inspect.Preorder(nodeFilter, func(n ast.Node) {
        fn := n.(*ast.FuncDecl)

        _, ok := operationIds[fn.Name.Name]
        if !ok {
            // func名がoperation Idでなかったらなにもしない
            return
        }
        opId := fn.Name.Name

        if fn.Body == nil {
            return
        }

        if err := analyze(fn.Body, opId); err != nil {
            // 違反を検出したらReport()を使って報告する
            pass.Report(analysis.Diagnostic{
                Pos:     fn.Body.Pos(),
                End:     fn.Body.End(),
                Message: err.Error(),
            })
        }
    })

    return nil, nil
}

// analyze は与えられた関数の中でoperation idと紐づくrequest, responseの型を使用しているかどうか確認し、使用していない場合ルール違反としてエラーを返す.
func analyze(stmt ast.Stmt, opId string) error {
    responseTypeUsed := false
    requestTypeUsed := false

    ast.Inspect(stmt, func(n ast.Node) bool {
        if ident, ok := n.(*ast.Ident); ok {
            if ident.Name == fmt.Sprintf("%sResponse", opId) {
                responseTypeUsed = true
            } else if ident.Name == fmt.Sprintf("%sRequest", opId) {
                requestTypeUsed = true
            }
        }
        return true
    })

    if responseTypeUsed && requestTypeUsed {
        return nil // ok
    }

    err := fmt.Errorf("func %s() で使う必要のあるstructを使っていません.", opId)

    if !responseTypeUsed {
        err = fmt.Errorf("%w %sResponse を使用していません", err, opId)
    }

    if !requestTypeUsed {
        err = fmt.Errorf("%w %sRequest を使用していません", err, opId)
    }

    return err
}

func analyze()内で実際に探しています。超大雑把に書くと、ast.Identのnameを調査します。 *4

それでは作成したlinterで以下のコードを検査してみます。以下の例だと、CreateUserというオペレーションIDに対してCreateUserRequest, CreateUserResponse構造体をそれぞれ使うというルールを守る必要があります

func (h *Handlers) CreateUser(w http.ResponseWriter, r *http.Request) {
    var req server.CreateUserRequest // server packageは自動生成されたAPI定義です

    err := json.NewDecoder(r.Body).Decode(&req)
    if err != nil {
        http.Error(w, err.Error(), http.StatusBadRequest)
        return
    }

    // 本当は以下のようにCreateUserResponseを使ってレスポンスしなければいけない..!
    // res := server.CreateUserResponse{
    //     User: &server.User{},
    // }

    // err = json.NewEncoder(w).Encode(res)
    // if err != nil {
    //     http.Error(w, err.Error(), http.StatusInternalServerError)
    //     return
    // }
}

f:id:ykpythemind:20201209154645p:plain

ちゃんと指摘されましたね！これをCIで実行するようにしておきましょう。

問題点

現時点では諸々問題点があります。

誤検出がある

ast.Identのみを確認すると、以下のような場合も通ってしまいます。

func (h *Handlers) CreateUser(w http.ResponseWriter, r *http.Request) {
    // func内で定義したstruct
    type CreateUserRequest struct{}

    // 変数名
    CreateUserResponse := "hoge"
    w.Write([]byte(CreateUserResponse))
}

厳密には、 resp := server.CreateUserResponse{} のような代入文(AssignStmt) や、 var resp server.CreateUserResponse のような変数宣言(GenDecl) に使われる名前のみを検出しなければなりません。また、実際にresponseWriterを用いてレスポンスとして送られたかどうかも検出することはできません。

複雑なスキーマへの対応

status codeやcontent typeが複数ある場合など、どう担保する？などの課題があります
Get methodや単純にstatus codeのみを返す場合にも決められた名前の構造体を使用しなければなりません。

func (h *Handlers) GetUsers(w http.ResponseWriter, r *http.Request) {
    var _ server.GetUsersRequest // 不要だが使わなければいけない... (blank variableを使ってコンパイルエラーを回避