CIツール導入ガイド 第4回 Infer による静的バグ検証【Java版】

By • 2018-10-23 • DevOps

JavaによるCI導入ガイドの第4回は Infer による静的バグ検証を行う。

目次

1. Infer 紹介

Infer は、ソースコードの静的解析により、ソースコードの問題点を検出するツールである。 Infer では、Java では null pointer exception、マルチスレッドプログラムでのスレッドセーフ、リソースの開放漏れなどを確認できる。

null 参照の発明者アントニー・ホーアが『それは10億ドルにも相当する私の誤りだ』と述べているが、Java プログラマを長年悩ませてきた null pointer exception は数理的に排除できるものであり、実際にそれを実現したプログラミング言語も多くある。つまり null pointer exception は「慎重に」排除するものなどではない。Infer は、null pointer exception になる可能性がある場所を指摘してくれる。開発当初から Infer を導入すれば、ヌルポフリーな開発を実現できる。

Infer は非常に強力で有用なツールであるため、多少導入や使用の点での面倒さはあるが、ぜひとも一度使用してみていただきたい。

2. パッケージ導入

Infer 公式導入ガイド を参考に Infer をインストールする。 mac であれば brew コマンドで一発である。Windows 環境では Docker を使うのが楽なようだ。

3. 検証と修正

Null ポインタ検証

infer run -a checkers --eradicate -- gradle clean build -x test を実行すると、infer の検証結果を出力する。

legend:
  "F" analyzing a file
  "." analyzing a procedure

FFFFFF.............................
Found 5 issues

src/main/java/com/example/messageboard/MessageForm.java:7: error: ERADICATE_FIELD_NOT_INITIALIZED
  Field `MessageForm.message` is not initialized in the constructor and is not declared `@Nullable`.
  5.    * メッセージ投稿フォームのパラメタ用POJO.
  6.    */
  7. > public class MessageForm {
  8.
  9.     private String message;

src/main/java/com/example/messageboard/Message.java:18: error: ERADICATE_FIELD_NOT_INITIALIZED
  Field `Message.command` is not initialized in the constructor and is not declared `@Nullable`.
  16.   @Entity
  17.   @Table(name = "messages")
  18. > public class Message {
  19.
  20.     @Id

src/main/java/com/example/messageboard/Message.java:18: error: ERADICATE_FIELD_NOT_INITIALIZED
  Field `Message.createdAt` is not initialized in the constructor and is not declared `@Nullable`.
  16.   @Entity
  17.   @Table(name = "messages")
  18. > public class Message {
  19.
  20.     @Id

src/main/java/com/example/messageboard/Message.java:18: error: ERADICATE_FIELD_NOT_INITIALIZED
  Field `Message.id` is not initialized in the constructor and is not declared `@Nullable`.
  16.   @Entity
  17.   @Table(name = "messages")
  18. > public class Message {
  19.
  20.     @Id

src/main/java/com/example/messageboard/Message.java:18: error: ERADICATE_FIELD_NOT_INITIALIZED
  Field `Message.message` is not initialized in the constructor and is not declared `@Nullable`.
  16.   @Entity
  17.   @Table(name = "messages")
  18. > public class Message {
  19.
  20.     @Id

コンストラクタで初期化されないメンバ変数があるため、null が入っている可能性があると指摘している。 この実装では、指摘されたすべてのパラメタに null が入る可能性があるため、@Nullable アノテーションを付与して null が入る可能性があることを Infer に伝える。

まず、@Nullable アノテーションのための依存性を追加する。

build.gradle

Message, MessageForm それぞれのメンバ変数に @Nullable アノテーションを追加する。

src/main/java/com/example/messageboard/Message.java

src/main/java/com/example/messageboard/MessageForm.java

Found 6 source files to analyze in /Users/yuki/work-local/java-ci-test/java-ci/sample/infer-out
Starting analysis...

legend:
  "F" analyzing a file
  "." analyzing a procedure

FFFFFF.............................
Found 5 issues

src/main/java/com/example/messageboard/MessageForm.java:30: error: ERADICATE_RETURN_NOT_NULLABLE
  Method `getMessage()` may return null but it is not annotated with `@Nullable`. (Origin: field MessageForm.message at line 31).
  28.      * @return 投稿メッセージ本文.
  29.      */
  30. >   public String getMessage() {
  31.       return this.message;
  32.     }

src/main/java/com/example/messageboard/Message.java:54: error: ERADICATE_RETURN_NOT_NULLABLE
  Method `getId()` may return null but it is not annotated with `@Nullable`. (Origin: field Message.id at line 55).
  52.      * @return レコードの主キー.
  53.      */
  54. >   public UUID getId() {
  55.       return this.id;
  56.     }

src/main/java/com/example/messageboard/Message.java:74: error: ERADICATE_RETURN_NOT_NULLABLE
  Method `getMessage()` may return null but it is not annotated with `@Nullable`. (Origin: field Message.message at line 75).
  72.      * @return レコードのメッセージ本文.
  73.      */
  74. >   public String getMessage() {
  75.       return this.message;
  76.     }

src/main/java/com/example/messageboard/Message.java:95: error: ERADICATE_RETURN_NOT_NULLABLE
  Method `getCommand()` may return null but it is not annotated with `@Nullable`. (Origin: field Message.command at line 96).
  93.      * @return レコードのコマンド.
  94.      */
  95. >   public String getCommand() {
  96.       return this.command;
  97.     }

src/main/java/com/example/messageboard/Message.java:113: error: ERADICATE_RETURN_NOT_NULLABLE
  Method `getCreatedAt()` may return null but it is not annotated with `@Nullable`. (Origin: field Message.createdAt at line 114).
  111.      * @return レコードのメッセージ作成日時.
  112.      */
  113. >   public ZonedDateTime getCreatedAt() {
  114.       return this.createdAt;
  115.     }


Summary of the reports

  ERADICATE_RETURN_NOT_NULLABLE: 5

次は getter が Nullable でないと指摘されたので、アノテーションを付与する。

src/main/java/com/example/messageboard/Message.java

src/main/java/com/example/messageboard/MessageForm.java

Found 6 source files to analyze in /Users/yuki/work-local/java-ci-test/java-ci/sample/infer-out
Starting analysis...

legend:
  "F" analyzing a file
  "." analyzing a procedure

FFFFFF.............................
Found 1 issue

src/main/java/com/example/messageboard/MessageMappingService.java:77: error: ERADICATE_PARAMETER_NOT_NULLABLE
  `MessageMappingService.parseMessage(...)` needs a non-null value in parameter 1 but argument `originalMessage` can be null. (Origin: call to getMessage() at line 76).
  75.       var message = new Message();
  76.       final var originalMessage = messageForm.getMessage();
  77. >     final var parsedMessage = parseMessage(originalMessage);
  78.
  79.       message.setMessage(parsedMessage.getMessage());

Summary of the reports

  ERADICATE_PARAMETER_NOT_NULLABLE: 1

すると、どうもnullチェックが抜けていたところが見つかったようである。 ここでは、空文字列に置き換えて問題ないので、そのように修正する。

src/main/java/com/example/messageboard/MessageMappingService.java

これで検証を通過する。

なお、実装したソースの中で、動的に読み込まれるテンプレートは解析の対象にならないので注意。 また、いくつかの制限についてはドキュメントを参照のこと。

スレッドセーフ検証

次にスレッドセーフの検証を行ってみる。 Spring の Bean は Singleton であるため、スレッドセーフであることが求められる。 MessageMappingService のスレッドセーフ性を検証する。

まずは必要な依存性を追加する。

build.gradle

次に、対象のクラスに @Threadsafe アノテーションをつける。

src/main/java/com/example/messageboard/MessageMappingService.java

実はこのクラスは Immutable に作ってあるので、テストは問題なく通過する。 ここで、このクラスに邪悪で無意味なコードを追加して、スレッドセーフの検証を行ってみよう。

src/main/java/com/example/messageboard/MessageMappingService.java

diff --git a/src/main/java/com/example/messageboard/MessageMappingService.java b/src/main/java/com/example/messageboard/MessageMappingService.java
index 74ede54..b2e733e 100644
--- a/src/main/java/com/example/messageboard/MessageMappingService.java
+++ b/src/main/java/com/example/messageboard/MessageMappingService.java
@@ -1,5 +1,6 @@
 package com.example.messageboard;
  
+import com.facebook.infer.annotation.ThreadSafe;
 import java.time.ZonedDateTime;
 import java.util.Optional;
 import java.util.regex.Pattern;
@@ -10,6 +11,7 @@ import org.springframework.stereotype.Service;
  * メッセージフォームオブジェクトをDB用のエンティティに変換するサービス.
  */
 @Service
+@ThreadSafe
 public class MessageMappingService {
 
   protected final class ParseResult {
@@ -37,6 +39,8 @@ public class MessageMappingService {
  
   // コンパイル済み正規表現パターン.
   private final Pattern commandPattern;
+
+  private int evilCount;
  
   /**
    * 文字列からコマンドを分割する.
@@ -60,6 +64,7 @@ public class MessageMappingService {
   public MessageMappingService() {
     this.commandPattern = Pattern.compile(
         commandPatternSource, Pattern.MULTILINE);
+    this.evilCount = 0;
   }
  
   /**
@@ -77,6 +82,10 @@ public class MessageMappingService {
         messageForm.getMessage() != null ? messageForm.getMessage() : "";
     final var parsedMessage = parseMessage(originalMessage);
  
+    if (!originalMessage.trim().equals("")) {
+      this.evilCount++;
+    }
+
     message.setMessage(parsedMessage.getMessage());
     message.setCommand(parsedMessage.getCommand());
     message.setCreatedAt(postedDateTime);

このソースを検証すると、以下のように指摘される。

Found 6 source files to analyze in /Users/yuki/work-local/java-ci-test/java-ci/sample/infer-out
Starting analysis...

legend:
  "F" analyzing a file
  "." analyzing a procedure

FFFFFF..............................
Found 1 issue

src/main/java/com/example/messageboard/MessageMappingService.java:86: error: THREAD_SAFETY_VIOLATION
  Unprotected write. Non-private method `Message MessageMappingService.map(MessageForm,ZonedDateTime)` writes to field `this.com.example.messageboard.MessageMappingService.evilCount` outside of synchronization.
 Reporting because the current class is annotated `@ThreadSafe`, so we assume that this method can run in parallel with other non-private methods in the class (including itself).
  84.
  85.       if (!originalMessage.trim().equals("")) {
  86. >       this.evilCount++;
  87.       }
  88.

Summary of the reports

  THREAD_SAFETY_VIOLATION: 1

例えば以下のように修正すれば、チェックを通過できる。

src/main/java/com/example/messageboard/MessageMappingService.java

邪悪なコードはもとに戻して次に進む。

次回

第5回 End to End (e2e) テスト

本ブログは、Git / Subversionのクラウド型ホスティングサービス「tracpath(トラックパス)」を提供している株式会社オープングルーヴが運営しています。

エンタープライズ向け Git / Subversion 導入や DevOps による開発の効率化を検討している法人様必見！

「tracpath(トラックパス)」は、企業内の情報システム部門や、ソフトウェア開発・アプリケーション開発チームに対して、開発の効率化を支援し、品質向上を実現します。
さらに、システム運用の効率化・自動化支援サービスも提供しています。
”つくる情熱を支えるサービス”を提供し、まるで専属のインフラエンジニアのように、あなたのチームを支えていきます。

クラウド型バグ管理・バージョン管理ソフトの「tracpath(トラックパス)」を提供しているオープングルーヴが運営しています

You Might Also Like

No Comments

    Leave a Reply

    このサイトはスパムを低減するために Akismet を使っています。コメントデータの処理方法の詳細はこちらをご覧ください。