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
diff --git a/build.gradle b/build.gradle
index 7ce43ad..36ca298 100644
--- a/build.gradle
+++ b/build.gradle
@@ -29,6 +29,9 @@ bootJar {
repositories {
mavenCentral()
+
+ // @Nullable アノテーションのために Google の mvn リポジトリを追加する
+ google()
}
// Java のバージョン指定
@@ -37,6 +40,7 @@ targetCompatibility = 1.10
// 依存パッケージ
dependencies {
+ compile "com.android.support:support-annotations:27.1.1"
compile "javax.xml.bind:jaxb-api"
compile "org.springframework.boot:spring-boot-starter-web"
compile "org.springframework.boot:spring-boot-starter-data-jpa"
Message
, MessageForm
それぞれのメンバ変数に @Nullable
アノテーションを追加する。
src/main/java/com/example/messageboard/Message.java
diff --git a/src/main/java/com/example/messageboard/Message.java b/src/main/java/com/example/messageboard/Message.java
index 7d2cea5..cd72399 100644
--- a/src/main/java/com/example/messageboard/Message.java
+++ b/src/main/java/com/example/messageboard/Message.java
@@ -1,5 +1,6 @@
package com.example.messageboard;
+import android.support.annotation.Nullable;
import java.time.ZonedDateTime;
import java.util.UUID;
import javax.persistence.Column;
@@ -21,15 +22,19 @@ public class Message {
@Id
@Column
@GeneratedValue(strategy = GenerationType.AUTO)
+ @Nullable
private UUID id;
@Column(nullable = false)
+ @Nullable
private String message;
@Column(nullable = true)
+ @Nullable
private String command;
@Column(nullable = false)
+ @Nullable
private ZonedDateTime createdAt;
/**
src/main/java/com/example/messageboard/MessageForm.java
diff --git a/src/main/java/com/example/messageboard/MessageForm.java b/src/main/java/com/example/messageboard/MessageForm.java
index 9bd9ec8..1001866 100644
--- a/src/main/java/com/example/messageboard/MessageForm.java
+++ b/src/main/java/com/example/messageboard/MessageForm.java
@@ -1,11 +1,14 @@
package com.example.messageboard;
+import android.support.annotation.Nullable;
+
/**
* メッセージ投稿フォームのパラメタ用POJO.
*/
public class MessageForm {
+ @Nullable
private String message;
/**
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
diff --git a/src/main/java/com/example/messageboard/Message.java b/src/main/java/com/example/messageboard/Message.java
index cd72399..ab74a06 100644
--- a/src/main/java/com/example/messageboard/Message.java
+++ b/src/main/java/com/example/messageboard/Message.java
@@ -51,6 +51,7 @@ public class Message {
*
* @return レコードの主キー.
*/
+ @Nullable
public UUID getId() {
return this.id;
}
@@ -71,6 +72,7 @@ public class Message {
*
* @return レコードのメッセージ本文.
*/
+ @Nullable
public String getMessage() {
return this.message;
}
@@ -92,6 +94,7 @@ public class Message {
*
* @return レコードのコマンド.
*/
+ @Nullable
public String getCommand() {
return this.command;
}
@@ -110,6 +113,7 @@ public class Message {
*
* @return レコードのメッセージ作成日時.
*/
+ @Nullable
public ZonedDateTime getCreatedAt() {
return this.createdAt;
}
src/main/java/com/example/messageboard/MessageForm.java
diff --git a/src/main/java/com/example/messageboard/MessageForm.java b/src/main/java/com/example/messageboard/MessageForm.java
index 1001866..39cd4a8 100644
--- a/src/main/java/com/example/messageboard/MessageForm.java
+++ b/src/main/java/com/example/messageboard/MessageForm.java
@@ -27,6 +27,7 @@ public class MessageForm {
*
* @return 投稿メッセージ本文.
*/
+ @Nullable
public String getMessage() {
return this.message;
}
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
diff --git a/src/main/java/com/example/messageboard/MessageMappingService.java b/src/main/java/com/example/messageboard/MessageMappingService.java
index 08cd0a9..74ede54 100644
--- a/src/main/java/com/example/messageboard/MessageMappingService.java
+++ b/src/main/java/com/example/messageboard/MessageMappingService.java
@@ -73,7 +73,8 @@ public class MessageMappingService {
final MessageForm messageForm,
final ZonedDateTime postedDateTime) {
var message = new Message();
- final var originalMessage = messageForm.getMessage();
+ final var originalMessage =
+ messageForm.getMessage() != null ? messageForm.getMessage() : "";
final var parsedMessage = parseMessage(originalMessage);
message.setMessage(parsedMessage.getMessage());
これで検証を通過する。
なお、実装したソースの中で、動的に読み込まれるテンプレートは解析の対象にならないので注意。 また、いくつかの制限についてはドキュメントを参照のこと。
スレッドセーフ検証
次にスレッドセーフの検証を行ってみる。 Spring の Bean は Singleton であるため、スレッドセーフであることが求められる。 MessageMappingService
のスレッドセーフ性を検証する。
まずは必要な依存性を追加する。
build.gradle
diff --git a/build.gradle b/build.gradle
index 36ca298..f8a3691 100644
--- a/build.gradle
+++ b/build.gradle
@@ -41,6 +41,7 @@ targetCompatibility = 1.10
// 依存パッケージ
dependencies {
compile "com.android.support:support-annotations:27.1.1"
+ compile "com.facebook.infer.annotation:infer-annotation:0.11.2"
compile "javax.xml.bind:jaxb-api"
compile "org.springframework.boot:spring-boot-starter-web"
compile "org.springframework.boot:spring-boot-starter-data-jpa"
次に、対象のクラスに @Threadsafe
アノテーションをつける。
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..4511fe9 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 {
実はこのクラスは 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
diff --git a/src/main/java/com/example/messageboard/MessageMappingService.java b/src/main/java/com/example/messageboard/MessageMappingService.java
index b2e733e..db37217 100644
--- a/src/main/java/com/example/messageboard/MessageMappingService.java
+++ b/src/main/java/com/example/messageboard/MessageMappingService.java
@@ -82,8 +82,10 @@ public class MessageMappingService {
messageForm.getMessage() != null ? messageForm.getMessage() : "";
final var parsedMessage = parseMessage(originalMessage);
- if (!originalMessage.trim().equals("")) {
- this.evilCount++;
+ synchronized (this) {
+ if (!originalMessage.trim().equals("")) {
+ this.evilCount++;
+ }
}
message.setMessage(parsedMessage.getMessage());
邪悪なコードはもとに戻して次に進む。
次回
社内サーバにリモートリポジトリを作るのも一つですが、「開発にまつわる面倒事」をこの際全部、tracpath(トラックパス)に任せてみませんか?
バージョン管理サービス・プロジェクト管理サービスの「tracpath(トラックパス)」では、
ユーザー5名、リポジトリ数3つまで、無料で利用可能です。
さっそく実務でも使って見ましょう。
自らも開発を行う会社が作ったからこそ、開発チームの「作る情熱」を支える、やるべきことに集中出来るサービスになっています。
エンタープライズ利用が前提のASPサービスなので、セキュリティも強固です。