コード品質を上げるために凝集度解析ツールをGo言語で自作した話

みなさんこんにちは。Yahoo!広告 ディスプレイ広告エンジニアの安部です。広告配信システムの開発を担当しています。

私がいるチームでは、レガシーシステムのモダン化に取り組んでいます。新しいシステムでは処理速度や書き易さの点からGo言語を採用しています。

新しいシステムでは、品質を担保するためにソフトウェアメトリクスをCIで監視しています。メトリクスの算出にはgolangci-lintを使っています。golangci-lintが算出するメトリクスには循環的複雑度などがあります。

今回は、golangci-lintが計測していない凝集度に着目しました。本記事では凝集度の解説に加え、静的解析ツールの開発と導入など、凝集度をCI監視のメトリクスに加えてクラスの保守性を担保する仕組みづくりへのトライについて紹介します。

凝集度

凝集度とは、クラスが責任範囲に集中しているかを示すソフトウェアメトリクスです。

クラスの責任範囲が少ないほど凝集度は高く（良く）なります。責任範囲が多いほど凝集度は低く（悪く）なり、保守性などの観点から好ましくありません。

凝集度の算出には複数の方法がありますが、今回は LCOM4 を使ってみます。

LCOM4

過去にTypeScriptを使ったプロジェクトで凝集度を計測する基準の1つであるLCOM4をこちらのツールで計測したことがありました。GoでもLCOM4を算出してコード品質の向上に役立てようと考えましたが、GoでLCOM4を算出するツールは存在していないようでした。

LCOM4はクラスごとに値を出力します。凝集度が理想的な状態では値が1になり、責任範囲が多いと1より大きい値が算出されます。例えばクラスが2つの責任範囲を持っていればLCOM4の値は2になります。

Go静的解析ツールの自作

Goは標準のgoパッケージで静的解析の機能を提供しています。標準で付いていてかつドキュメントやサンプルコードが充実しているので、比較的簡単に静的解析ツールが作れそうです。

また、LCOM4はアルゴリズムがシンプルなので実装が比較的容易です。

これらの点を考慮して、LCOM4を評価するためにツールを自作しました。作成したツールはhttps://github.com/yahoojapan/lcom4go に公開してあります。

LCOM4の計算アルゴリズムはこちらです。グラフ理論の連結成分数を数えるアルゴリズムで、基礎的なアルゴリズムです。深さ優先探索でノードを探索していきます。こちらの記事の解説がわかりやすいです。

今回の実装では1つのグラフが1つの構造体に対応しています。グラフの表現は隣接リストです。

type graph struct {
    nodes    []graphNode
    neighbor map[graphNode][]graphNode
}

連結成分を数える処理はcomputeConnectedComponents()で行っています。全ノードをfor rangeでループします。もし現在のノードが未探索ならば、そこからたどれるノードをcollectConnectedNodes()で列挙します。列挙したノードは1つの連結成分なので、componentsに記録します。さらに、visitedにフラグを立てて探索済みにします。

func computeConnectedComponents(g graph) [][]graphNode {
    components := [][]graphNode{}

    visited := make(map[graphNode]bool)
    for _, n := range g.nodes {
        if visited[n] {
            continue
        }

        compo := collectConnectedNodes(g, n)
        for _, m := range compo {
            visited[m] = true
        }
        components = append(components, compo)
    }
    return components
}

また、Goに特有な処理を1つ入れてあります。Goではレシーバーをメンバー変数のように参照できるので、レシーバーをメンバー変数と同等の扱いをしています。例えば、次のようなコードがあったとします。

type myint int

func (a myint) add(b int) int {
  return int(a) + b
}

func (a myint) mul(b int) int {
  return int(a) * b
}

もし、レシーバーをメンバー変数と同等の扱いをしない場合にはmyintのLCOM4値は2です。しかしこの場合はレシーバーをメンバー変数のように使っているので、LCOM4値は1であるのが正しいでしょう。

LCOM4を適用してみた

作成したツールを自分自身のコードに対して適用してみます。

/lcom4go/lcom4.go:43:6: 'github.com/yahoojapan/lcom4go.methodNode' has low cohesion, LCOM4 is 2, pairs of methods: [[typ()] [name() .__receiver__ String()]]
/lcom4go/lcom4.go:37:6: 'github.com/yahoojapan/lcom4go.fieldNode' has low cohesion, LCOM4 is 2, pairs of methods: [[typ()] [name() .__receiver__ String()]]

なんと！　自分自身に対してチェックが通りません…。

警告はLCOM4値が1より大きい場合に出ます。lcom4.go:43:6のstructにおいてtyp()とname() String()の2つがグラフの連結成分として計算された、という警告が出ているようです。

警告が出ている箇所のソースコードを見てみます。

type fieldNode string

func (f fieldNode) typ() int       { return field }
func (f fieldNode) name() string   { return string(f) }
func (f fieldNode) String() string { return fmt.Sprintf(".%s", string(f)) }

図にすると次になります。name()とString()はレシーバーを介して1つの連結成分にカウントされます。

typ()というメソッドはどのメンバー変数も参照していません。ですが、これはGoでよくある書き方です。

例えば、net/net.goのtimeoutErrorでもこのような書き方が見られます。

type timeoutError struct{}

func (e *timeoutError) Error() string   { return "i/o timeout" }
func (e *timeoutError) Timeout() bool   { return true }
func (e *timeoutError) Temporary() bool { return true }

図から明らかなようにtimeoutErrorのLCOM4値は3です。

メソッドはメンバー変数を参照していませんが、特に問題がある実装ではありません。このように特に問題がある実装ではなかったとしてもLCOM4値が1より大きくなってしまう場合があることが分かりました。

プロジェクトコードに適用してみた

チームで開発しているコードに適用したところ、最大で6のLCOM4値が出ました。ちょっと大きい気がしますね。6を出したstructはData Transfer Objectでした。凝集度は低いですが、設計で意図した構造になっているので問題ありません。

また、Goの標準ライブラリに適用するとLCOM4>1が多数検出されました。上記のパターン以外にもLCOM4値は高いが設計は問題ないパターンが多数あるようです。

参考までにnet/httpにツールを適用した結果を掲載しておきます。

golang/1.17.3/go/src/net/http/h2_bundle.go:3480:6: 'net/http.http2httpError' has low cohesion, LCOM4 is 3, pairs of methods: [[Error() .msg] [Timeout() .timeout] [Temporary()]]
golang/1.17.3/go/src/net/http/server.go:3363:6: 'net/http.timeoutWriter' has low cohesion, LCOM4 is 3, pairs of methods: [[Push() .w] [Header() .h] [Write() .mu .timedOut .wroteHeader writeHeaderLocked() .wbuf WriteHeader() .req .code]]
golang/1.17.3/go/src/net/http/h2_bundle.go:6693:6: 'net/http.http2Transport' has low cohesion, LCOM4 is 2, pairs of methods: [[dialTLSWithContext() dialTLS() dialClientConn() .DialTLS newTLSConfig() newClientConn() .TLSClientConfig NewClientConn() .__receiver__ idleConnTimeout() vlogf() maxHeaderListSize() .AllowHTTP disableKeepAlives() initConnPool() .t1 RoundTripOpt() logf() .MaxHeaderListSize connPool() .ConnPool .connPoolOrDef disableCompression() expectContinueTimeout() RoundTrip() .connPoolOnce CloseIdleConnections() .DisableCompression] [pingTimeout() .PingTimeout]]
golang/1.17.3/go/src/net/http/transport.go:2498:6: 'net/http.httpError' has low cohesion, LCOM4 is 3, pairs of methods: [[Error() .err] [Timeout() .timeout] [Temporary()]]
golang/1.17.3/go/src/net/http/socks_bundle.go:241:6: 'net/http.socksAddr' has low cohesion, LCOM4 is 2, pairs of methods: [[Network()] [String() .__receiver__ .Port .IP .Name]]
golang/1.17.3/go/src/net/http/h2_bundle.go:6920:6: 'net/http.http2clientStream' has low cohesion, LCOM4 is 2, pairs of methods: [[get1xxTraceFunc() .trace writeRequest() encodeAndWriteHeaders() doRequest() .cc .ctx .reqCancel .__receiver__ .isHead .requestedGzip .on100 .reqBodyContentLength .sentEndStream .abort .abortErr writeRequestBody() .respHeaderRecv .peerClosed .sentHeaders .ID cleanupWriteRequest() abortStream() abortStreamLocked() abortRequestBodyWrite() awaitFlowControl() frameScratchBufferLen() .reqBody .reqBodyClosed .bufPipe .donec .abortOnce .flow] [copyTrailers() .trailer .resTrailer]]
golang/1.17.3/go/src/net/http/transport.go:2848:6: 'net/http.fakeLocker' has low cohesion, LCOM4 is 2, pairs of methods: [[Lock()] [Unlock()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:7033:6: 'net/http.http2noCachedConnError' has low cohesion, LCOM4 is 2, pairs of methods: [[IsHTTP2NoCachedConnError()] [Error()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:2561:6: 'net/http.http2PushPromiseFrame' has low cohesion, LCOM4 is 2, pairs of methods: [[HeaderBlockFragment() checkValid() .headerFragBuf] [HeadersEnded() .http2FrameHeader]]
golang/1.17.3/go/src/net/http/h2_bundle.go:2014:6: 'net/http.http2SettingsFrame' has low cohesion, LCOM4 is 2, pairs of methods: [[IsAck() .http2FrameHeader] [Value() checkValid() NumSettings() Setting() ForeachSetting() .p HasDuplicates()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:9623:6: 'net/http.http2writeSettingsAck' has low cohesion, LCOM4 is 2, pairs of methods: [[writeFrame()] [staysWithinBuffer()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:3755:6: 'net/http.http2Server' has low cohesion, LCOM4 is 4, pairs of methods: [[initialConnRecvWindowSize() .MaxUploadBufferPerConnection] [initialStreamRecvWindowSize() .MaxUploadBufferPerStream] [maxReadFrameSize() .MaxReadFrameSize ServeConn() .__receiver__ maxConcurrentStreams() .state .NewWriteScheduler .PermitProhibitedCipherSuites .MaxConcurrentStreams] [maxQueuedControlFrames()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:9351:6: 'net/http.http2missingBody' has low cohesion, LCOM4 is 2, pairs of methods: [[Close()] [Read()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:6862:6: 'net/http.http2ClientConn' has low cohesion, LCOM4 is 2, pairs of methods: [[healthCheck() .t Ping() closeForLostPing() .__receiver__ idleStateLocked() responseHeaderTimeout() forgetStreamID() logf() vlogf() .mu .pings .wmu .fr .bw .readerDone .readerErr closeForError() closeIfIdle() RoundTrip() readLoop() ReserveNewRequest() idleState() .singleUse .nextStreamID .streams .streamsReserved .maxConcurrentStreams .goAway .closed .closing .doNotReuse .pendingRequests tooIdleLocked() canTakeNewRequestLocked() .lastActive .idleTimer .idleTimeout .lastIdle .cond .tconn SetDoNotReuse() setGoAway() CanTakeNewRequest() isDoNotReuseAndIdle() Shutdown() sendGoAway() decrStreamReservations() writeStreamReset() writeHeaders() Close() onIdleTimeout() .tlsState addStreamLocked() awaitOpenSlotForStreamLocked() decrStreamReservationsLocked() .goAwayDebug .werr .initialWindowSize .flow .inflow] [encodeHeaders() .hbuf .peerMaxHeaderListSize writeHeader() encodeTrailers() .henc]]
golang/1.17.3/go/src/net/http/transport.go:2839:6: 'net/http.tlsHandshakeTimeoutError' has low cohesion, LCOM4 is 3, pairs of methods: [[Timeout()] [Temporary()] [Error()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:9544:6: 'net/http.http2flushFrameWriter' has low cohesion, LCOM4 is 2, pairs of methods: [[writeFrame()] [staysWithinBuffer()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:2286:6: 'net/http.http2HeadersFrame' has low cohesion, LCOM4 is 2, pairs of methods: [[HeaderBlockFragment() checkValid() .headerFragBuf] [HeadersEnded() .http2FrameHeader StreamEnded() HasPriority()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:9780:6: 'net/http.http2write100ContinueHeadersFrame' has low cohesion, LCOM4 is 2, pairs of methods: [[writeFrame() .streamID] [staysWithinBuffer()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:9801:6: 'net/http.http2writeWindowUpdate' has low cohesion, LCOM4 is 2, pairs of methods: [[staysWithinBuffer()] [writeFrame() .streamID .n]]
golang/1.17.3/go/src/net/http/h2_bundle.go:1894:6: 'net/http.http2DataFrame' has low cohesion, LCOM4 is 2, pairs of methods: [[StreamEnded() .http2FrameHeader] [Data() checkValid() .data]]
golang/1.17.3/go/src/net/http/h2_bundle.go:9660:6: 'net/http.http2writeResHeaders' has low cohesion, LCOM4 is 2, pairs of methods: [[staysWithinBuffer()] [writeFrame() .httpResCode .h .trailers .contentType .contentLength .date writeHeaderBlock() .streamID .endStream]]
golang/1.17.3/go/src/net/http/h2_bundle.go:10547:6: 'net/http.http2randomWriteScheduler' has low cohesion, LCOM4 is 3, pairs of methods: [[OpenStream()] [CloseStream() .sq .queuePool Push() Pop() .zero] [AdjustStream()]]
golang/1.17.3/go/src/net/http/h2_bundle.go:9732:6: 'net/http.http2writePushPromise' has low cohesion, LCOM4 is 2, pairs of methods: [[staysWithinBuffer()] [writeFrame() .method .url .h writeHeaderBlock() .streamID .promisedID]]
golang/1.17.3/go/src/net/http/h2_bundle.go:2521:6: 'net/http.http2ContinuationFrame' has low cohesion, LCOM4 is 2, pairs of methods: [[HeaderBlockFragment() checkValid() .headerFragBuf] [HeadersEnded() .http2FrameHeader]]
golang/1.17.3/go/src/net/http/h2_bundle.go:1218:6: 'net/http.http2StreamError' has low cohesion, LCOM4 is 2, pairs of methods: [[Error() .Cause .StreamID .Code writeFrame()] [staysWithinBuffer()]]
golang/1.17.3/go/src/net/http/server.go:2527:6: 'Server' has low cohesion, LCOM4 is 3, pairs of methods: [[newConn() .__receiver__ Serve() onceSetNextProtoDefaults() ListenAndServe() setupHTTP2_Serve() trackListener() .BaseContext getDoneChan() logf() .ConnContext ServeTLS() setupHTTP2_ServeTLS() onceSetNextProtoDefaults_Serve() .TLSNextProto .nextProtoErr shuttingDown() .Addr .nextProtoOnce .mu .listeners getDoneChanLocked() .ErrorLog .TLSConfig ListenAndServeTLS() shouldConfigureHTTP2ForServe() doKeepAlives() .inShutdown Close() Shutdown() RegisterOnShutdown() numListeners() closeIdleConns() trackConn() closeListenersLocked() .doneChan closeDoneChanLocked() .disableKeepAlives .activeConn .onShutdown SetKeepAlivesEnabled()] [maxHeaderBytes() .MaxHeaderBytes initialReadLimitSize()] [idleTimeout() .IdleTimeout .ReadTimeout readHeaderTimeout() .ReadHeaderTimeout]]
golang/1.17.3/go/src/net/http/h2_bundle.go:9564:6: 'net/http.http2writeGoAway' has low cohesion, LCOM4 is 2, pairs of methods: [[writeFrame() .maxStreamID .code] [staysWithinBuffer()]]
golang/1.17.3/go/src/net/http/server.go:3456:6: 'net/http.initALPNRequest' has low cohesion, LCOM4 is 2, pairs of methods: [[BaseContext() .ctx] [ServeHTTP() .c .h]]
golang/1.17.3/go/src/net/http/h2_bundle.go:9597:6: 'net/http.http2handlerPanicRST' has low cohesion, LCOM4 is 2, pairs of methods: [[writeFrame() .StreamID] [staysWithinBuffer()]]
golang/1.17.3/go/src/net/http/http.go:106:6: 'net/http.noBody' has low cohesion, LCOM4 is 3, pairs of methods: [[Read()] [Close()] [WriteTo()]]
golang/1.17.3/go/src/net/http/transport.go:454:6: 'net/http.transportRequest' has low cohesion, LCOM4 is 3, pairs of methods: [[extraHeaders() .extra] [setError() .mu .err] [logf() .Request]]
golang/1.17.3/go/src/net/http/h2_bundle.go:1468:6: 'net/http.http2FrameHeader' has low cohesion, LCOM4 is 3, pairs of methods: [[Header() .__receiver__] [String() writeDebug() .Type .Flags .StreamID .Length] [checkValid() .valid invalidate()]]
golang/1.17.3/go/src/net/http/server.go:2527:6: 'Server' has low cohesion, LCOM4 is 3, pairs of methods: [[newConn() .__receiver__ Serve() onceSetNextProtoDefaults() ListenAndServe() setupHTTP2_Serve() trackListener() .BaseContext getDoneChan() logf() .ConnContext ServeTLS() setupHTTP2_ServeTLS() onceSetNextProtoDefaults_Serve() .TLSNextProto .nextProtoErr shuttingDown() .Addr .nextProtoOnce .mu .listeners getDoneChanLocked() .ErrorLog .TLSConfig ListenAndServeTLS() shouldConfigureHTTP2ForServe() doKeepAlives() .inShutdown Close() Shutdown() RegisterOnShutdown() numListeners() closeIdleConns() trackConn() ExportAllConnsIdle() ExportAllConnsByState() closeListenersLocked() .doneChan closeDoneChanLocked() .disableKeepAlives .activeConn .onShutdown SetKeepAlivesEnabled()] [maxHeaderBytes() .MaxHeaderBytes initialReadLimitSize()] [idleTimeout() .IdleTimeout .ReadTimeout readHeaderTimeout() .ReadHeaderTimeout]]

おわりに

当初は、凝集度をCI監視のメトリクスに加えようと考えていました。しかし、全ての構造体で凝集度1にはできないため、CIへの導入は難しいことがわかりました。

自動化は諦めて、手動で実行した結果を目視でチェックすることにしました。

今回は残念ながら期待値通りにはならない結果でしたが、ただその過程の中で対象システム内の凝集度1になっていない構造体の全てに対してチーム内で議論を行うなど、コード品質に対して向き合うきっかけの一つになりました。

さいごにもう一度今回作成したツールのリンクを紹介しますので、この記事を読まれて凝集度に対する関心感じていただけた方はぜひ一度お試しください。https://github.com/yahoojapan/lcom4go