SpotBugsが可変オブジェクトでないものを可変オブジェクトと判定してしまう場合の対処法

こんにちは、エキサイト株式会社の平石です。

今回は、SpotBugsで可変でないオブジェクトが可変であると判定されてしまう問題に対処する方法をご紹介します。

はじめに

SpotBugsは、Javaのプログラムの中のバグや脆弱性（につながると考えられるもの）を検知してくれるプログラムです。

膨大なプログラムをレビューや手動で記述したテストで検証するのは大変ですので、自動で検証をおこなってくれるSpotBugsは有用なツールです。

しかしながら、時にSpotBugsは「バグではない箇所」を「バグを含んでいる」と判定して、警告を出すこともあります。
いわゆる「False Positive」と呼ばれるもので、「バグを正常であると判定して見逃してしまうよりは、怪しい箇所はバグとして判定して警告してしまった方が良い」という考え方から、そのような仕様になっているのだと考えられます。

コード例と発生する警告

今回は、以下のようなコードでSpotBugsが警告を発しました。

public interface NewsArticleService {
    void registerNewsArticle(String title, String body);
}

@Service
@RequiredArgsConstructor
public class NewsArticleServiceImpl implements NewsArticleService {
    private final NewsArticleRepository newsArticleRepository;

    @Override
    public void registerNewsArticle(final String title, final String body) {
        newsArticleRepository.insertNewsArticle(title, body);
    }
}

public interface NewsArticleRepository {
    void insertNewsArticle(String title, String body);
}

@Repository
public class NewsArticleRepositoryImpl implements NewsArticleRepository {

    @Override
    public void insertNewsArticle(final String title, final String body) {
        // ニュース記事をデータベースに登録する処理
    }
}

ニュース記事のService層で、データベースにニュース記事をInsertしにいく処理を呼び出しています。
その処理の詳細を記述したRepositoryはLombokの@RequiredArgsConstructorで自動生成されるコンストラクタでDIしています。

このコードを含んだ状態でSpotBugsを実行すると以下のような警告が出ます。

要するに、「newsArticleRepositoryは可変オブジェクトであり、この参照をそのままNewsArticleServiceに取り込むことで外部からnewsArticleRepositoryを操作できてしまう」という警告です。

しかし、NewsArticleRepositoryクラスにはそのクラスの内部のデータを変更するような機構は定義していません。
ただデータベースにニュース記事を登録するメソッドがあるだけです。
これは、一体どういうことでしょうか？

原因

この問題の原因は、SpotBugsのソースコード内の以下の記述にあるようです。

    private static final List<String> SETTER_LIKE_PREFIXES = Arrays.asList(
            "set", "put", "add", "insert", "delete", "remove", "erase", "clear", "push", "pop",
            "enqueue", "dequeue", "write", "append", "replace");

    〜 略 〜

    public static boolean looksLikeASetter(String methodName, String classSig, String retSig) {
        if (Objects.equals(classSig, retSig)) {
            return false;
        }

        return SETTER_LIKE_PREFIXES.stream().anyMatch(name -> methodName.startsWith(name));
    }

looksLikeASetterメソッドは検証対象のクラス内のメソッドが、SETTER_LIKE_PREFIXESに含まれる文字列で始まっていればtrueを返します。
このメソッドが、オブジェクトが可変かどうかの判定に使われているようです。

確かに、NewsArticleRepositoryクラスはSETTER_LIKE_PREFIXES内に含まれるinsertという文字列で始まるメソッドinsertNewsArticleを持っています。
これが、newsArticleRepositoryが実際には可変オブジェクトではないにも関わらず、可変オブジェクトと判定された理由です。

対処法

前節から、この警告はFalse Positiveであり、実際には何も問題を引き起こさないことがわかりました。
特に何も対処せずに警告を無視することもできますが、毎回SpotBugsの実行でエラーが発生するのは鬱陶しいですし、その他の有用な警告が埋もれてしまう可能性もあります。
そのため、このような警告は抑制するように設定するのが良いでしょう。
その方法はいくつか存在します。

メソッド名を変更する

今回は、メソッド名がinsertという文字列で始まっていることが原因です。そのため、例えばメソッド名をinsertNewsArticleからregisterNewsArticleに変更すれば、警告は発せられなくなります。

Lombokの設定で抑制する

これは、今回のケースでしか使えない方法ですが、Lombokの設定でLombokによって生成されたコードではSpotBugsの警告は発せられないようにすることができます。

lombok.configに以下のような記述を追加します。（lombok.configが存在しない場合には追加してください。）

lombok.extern.findbugs.addSuppressFBWarnings = true

アノテーションを付与する

Lombokを利用していない場合には、こちらの記事で紹介されている方法で一つ一つ抑止することもできます。

tech.excite.co.jp

@SuppressFBWarnings(value = {"EI_EXPOSE_REP2"})

SpotBugsの設定ファイルに記述する

一つ一つアノテーションを付与するのが面倒な場合には、SpotBugsの「フィルタファイル」というファイルに警告を抑止するような設定を記述することもできます。

フィルタファイル — spotbugs 4.8.3 ドキュメント

例えば、コンストラクタではEI_EXPOSE_REP2の警告を発しないようにするためには、適当な名前をつけたXMLファイルに以下のように記述します。

<?xml version="1.0" encoding="utf-8" ?>
<FindBugsFilter ...>
    <Match>
        <Method name="&lt;init&gt;"/>
        <Bug pattern="EI_EXPOSE_REP2" />
    </Match>
</FindBugsFilter>

次に、SpotBugsにこの設定ファイルを認識させます。
Gradleのプラグインを利用している場合には、以下のように記述します。

spotbugs {
    excludeFilter = rootProject.file("path/filename")
}

そもそも可変データの検出を行わないようにする

リスクはありますが、そもそも可変データの検出を行わないようにすることもできます。
社内の内部APIのように使用法を制御できるような場合には、このような設定を検討しても良いでしょう。

以下は、Gradleプラグインで設定する場合を利用している場合の例です。

spotbugs {
    omitVisitors = ["FindReturnRef"]
}

終わりに

今回は、SpotBugsで可変でないオブジェクトが可変であると判定されてしまう問題とその対処法をご紹介しました。

可変オブジェクトの判定の方法はやや乱暴に感じてしまう部分もありますが、GitHubのissueが立ってから2年以上Openのままであることから、良い判定方法は見つかっていないようです。

当分は、紹介したような警告を抑止する対処法でしのぐしかなさそうですね。

では、また次回。

エキサイト TechBlog.