多くの人が覚えているように、しばらくの間、私はFindBugs Javaバイトコード静的アナライザーを開発しました。 ただし、FindBugsには非常に多くの問題があったため、新しいバイトコードアナライザーを作成する方が簡単だと判断しました。 私は彼をHuntBugsと非常に創造的に呼んだわけではありません。 GitHubで開発が進行中です。 それはまだ開発の初期段階にあり、時にはバグがあり、FindBugs診断の約35％をカバーしますが、同時に独自の興味深いものを追加します。 コマンドmvn one.util:huntbugs-maven-plugin:huntbugsを使用してMavenプロジェクトで試すことができます（レポートはtarget/huntbugs/report.html書き込まれtarget/huntbugs/report.html ）。 または、 one.util.huntbugs.HuntBugsから手動でアセンブルし、 one.util.huntbugs.HuntBugsコマンドラインone.util.huntbugs.HuntBugsを実行することもできone.util.huntbugs.HuntBugs 。このone.util.huntbugs.HuntBugs 、JARファイルまたは.classファイルを含むディレクトリで送信できます。

なんとかして、プロジェクトがいくぶん大きくなったとき、私はそれについてさらに詳しく話します。 この記事では、 IntelliJ IDEA Community Editionで見つかった興味深いHuntBugsを紹介します。 公式サイトからダウンロードし、このIDEの最新バージョンをインストールしてから、 lib/idea.jarにHuntBugsを設定します。このlib/idea.jarには、ほぼすべてのものがあります。 IDEAは非常に優れた静的アナライザーがあり、開発者が明らかにそれを使用するIDEであるため、IDEAで静的分析をテストするのが好きです。 その後に残っているものを見るのは興味深いです。

この記事の形式は、 PVS-Studioの機能とそれほど変わりません：エラー、コードの断片、説明。 もちろん、この記事には最も興味深いものだけが含まれていました。

フィールドはそれ自体に割り当てられます

原則として、 this.field = this.fieldような間違いを犯す人はいません。最新のIDEでさえ、通常は警告しません。 ただし、HuntBugsは少し深く見える場合があります。 コードスニペットは次のとおりです。

  private int myLastOffsetInNewTree; ... private int getNewOffset(ASTNode node){ int optimizedResult = haveNotCalculated; ... if (myLastNode == prev) { ... optimizedResult = myLastOffsetInNewTree; myLastNode = node; myLastOffsetInNewTree = optimizedResult; ... } } 

myLastOffsetInNewTreeフィールドmyLastOffsetInNewTreeローカル変数optimizedResultロードされ、何らかの理由で再びフィールドに保存されますが、この間は変更できませんでした。 最後の接ぎ木は奇妙で、削除する必要があるか、別の意味があります。

整数値は浮動小数点数にキャストされ、丸めメソッドに渡されます

型全体の分数へのタイミングの悪い変換に伴うエラーが発生する場合があります。 これは常にキャッチできるとは限りませんが、 ここでは判明しました ：

  final int width = icon.getIconWidth(); final int height = icon.getIconHeight(); final int x = (int)Math.ceil((actionButton.getWidth() - width) / 2); final int y = (int)Math.ceil((actionButton.getHeight() - height) / 2); 

ここでは、切り上げが2回使用されますが（ Math.ceil ）、Javaでは整数を整数で割ると整数が生成されるため（ Math.ceil ）、整数が引数として指定されます。 2.0除算するか、除算の前に小数に切り替えることを意図していた可能性が(int)Math.ceil現在の動作が(int)Math.ceilな(int)Math.ceil 、 (int)Math.ceilを削除する必要があります：コードのこの部分は使用(int)Math.ceilません。

式の範囲が原因でスイッチブランチに到達できません

非常に興味深いコードです 。明らかに誰かがかつて自動的に生成したもので、今ではこれが正しいかどうか、そして何があるべきかを誰も理解していません。

  int state = getState() & 0xF; tokenType = fixWrongTokenTypes(tokenType, state); if (...) { // TODO: do not know when this happens! switch (state) { case __XmlLexer.DOCTYPE: tokenType = XmlTokenType.XML_DECL_START; break; } } 

__XmlLexer.DOCTYPEの値は24ですが、上記ではstate = getState() & 0xFが実行されるため、 state値は0〜15のみであり、スイッチブランチは失敗することが保証されています。 おそらく、元のレクサーファイルが再度変更されたときに、定数が異なる値で再生成され、このファイルを再生成するのを忘れていた可能性があります。 なんらかの方法で、コードは非常に疑わしいものであり、コメントからも明らかです。

クラスリテラルではなくgetClass（）での同期

MatcherImplクラスのこのフラグメントは、 getClass()同期されます。 そして、これは、実際にMatcherのサブクラスを持つ、公開の非最終クラスで行われます。 その結果、このコードがサブクラスから実行されると、 MatcherImpl.classではなく、 MatcherImpl.classに従って同期が行われMatcher.class 。 問題は、同じクラスにMatcherImpl.classによる明示的な同期がMatcherImpl.class 、両方のクリティカルセクション（相互に排他的ではない場合があります）が同じ静的フィールドlastMatchData更新するという事実によって悪化します。 その結果、同期のポイント全体が失われます。 通常、 synchronized(getClass())は間違っています。明示的なクラスリテラルsynchronized(MatcherImpl.class) 。

スローされるのではなく作成およびドロップされた例外

Javaでよくある間違い：例外オブジェクトが作成されますが、スローされません。 たとえば、 ここに ：

 public void remove() { new OperationNotSupportedException(); } 

IDEA自体もそのような状況について警告しています。 別の同様の場所 。

不変ループ条件

ここに別の自動生成されたファイルがあります 。 原則として、ここではすべてがおそらく正常であり、何も編集することはできませんが、手書きのコードではこれは疑わしく見えます。

 boolean r = ...; while (r) { if (!value(b, l + 1)) break; if (!empty_element_parsed_guard_(b, "json", c)) break; c = current_position_(b); } 

ここに、ローカル変数r条件があり、その値がループ内で変化しないループがあります。そのため、ループにまったく入らないか、条件で終了することはありません（ breakによってのみ）。 これが本当に暗示されている場合、そのような場合、 if(r) { while(true) { ... } }を記述して、無限ループを作成する意図を強調する方がよいでしょう。

スイッチ演算子には同じブランチがあります

switch重複ブランチは妥当なように見える場合がありますが、次のように大きなコード部分がある場合は、詳しく見てください。

 switch ((((PsiWildcardType)x).isExtends() ? 0 : 1) + (((PsiWildcardType)y).isExtends() ? 0 : 2)) { case 0: /* ? super T1, ? super T2 */ if (constraints != null && xType != null && yType != null) { constraints.add(new Subtype(yType, xType)); } return balance(xType, yType, balancer, constraints); case 1: /* ? extends T1, ? super T2 */ if (constraints != null && xType != null && yType != null) { constraints.add(new Subtype(xType, yType)); } return balance(xType, yType, balancer, constraints); case 2: /* ? super T1, ? extends T2*/ return null; case 3: /* ? extends T1, ? extends T2*/ if (constraints != null && xType != null && yType != null) { constraints.add(new Subtype(xType, yType)); } return balance(xType, yType, balancer, constraints); } 

すぐに目立つわけではありませんが、 case 1とcase 3まったく同じです（ケース0とは異なります）。 そのcase 1 、コードを読みやすく、保守しやすくするために、 case 1とcase 3を組み合わせる方が賢明かもしれません。

同じ条件が繰り返しチェックされます

このコードでは 、何らかの理由で、同じ条件が2回チェックされます。

 if (offsetToScroll < 0) { if (offsetToScroll < 0) { ... } } 

内部チェックを削除する必要があるだけの場合もあれば、他の何かをチェックしたい場合もあります。 同様のケースがもう1つあります。 または、 別の興味深いケースがあります。

  return o instanceof PsiElement && ((PsiElement)o).isValid() && ((PsiElement)o).isPhysical() || o instanceof ProjectRootModificationTracker || o instanceof PsiModificationTracker || o == PsiModificationTracker.MODIFICATION_COUNT || o == PsiModificationTracker.OUT_OF_CODE_BLOCK_MODIFICATION_COUNT || // <<< o == PsiModificationTracker.OUT_OF_CODE_BLOCK_MODIFICATION_COUNT || // <<< o == PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT; 

そして、 ここで繰り返される条件はそれほど近くなく、それらに気付くのはさらに困難です：

 return SUPPORTED_TYPES.contains(token) || StdArrangementTokens.Regexp.NAME.equals(token) || StdArrangementTokens.Regexp.XML_NAMESPACE.equals(token) || KEEP.equals(token) || BY_NAME.equals(token) || SUPPORTED_TYPES.contains(token); 

条件SUPPORTED_TYPES.contains(token) 2回チェックされます。 もちろん、HuntBugsはこれらの条件の間で何も変わらないことを注意深く監視します。 token中間条件の下で再割り当てされた場合、そのような構造は存在する権利を持ちます。

数値比較は常に真または偽です

ここでは、実際のエラーよりも単なる冗長なチェックである可能性が高くなります。

 int size = myPanels.length; final Dimension preferredSize = super.getPreferredSize(); if (size >= 0 && size <= 20) { return preferredSize; } 

size変数には配列の長さが含まれますが、負の値にはできません。 size >= 0チェックする理由は不明です。 エラーがなくても、読者を混乱させるため、このようなチェックは削除すべきだと思います。 知られていない、おそらく作者がsize > 0暗示しているsize > 0 、これはエラーです。

プライベートメソッドのチェーンが呼び出されることはありません

通常、IDEは呼び出されないプライベートメソッドを簡単に見つけ、それらを削除することを提案します。 しかし、 そのような場合は常に決定されるとは限りません。

 @Nullable private static JsonSchemaObject getChild(JsonSchemaObject current, String name) { JsonSchemaObject schema = current.getProperties().get(name); if (schema != null) return schema; schema = getChildFromList(name, current.getAnyOf()); // <<< if (schema != null) return schema; ... } @Nullable private static JsonSchemaObject getChildFromList(String name, List<JsonSchemaObject> of) { if (of == null) return null; JsonSchemaObject schema; for (JsonSchemaObject object : of) { schema = getChild(object, name); // <<< if (schema != null) return schema; } return null; } 

これら2つのプライベートメソッドは互いに再帰的に呼び出しますが、外部からそれらを呼び出すことはありません。 HuntBugsはこの状況を認識しており、両方の方法は実際には使用されていないと述べています。

役に立たないString.substring（0）

正直なところ、私は製品コードでそのような診断を見ることを期待していませんでした、それはあまりにも簡単です。 しかし、いや、 些細なエラーがあります：

 String str = (String)value; if (str.startsWith("\"")) { str = str.substring(0); str = StringUtil.trimEnd(str, "\""); } 

どうやら、著者は文字列の最初の文字を削除するつもりでしたが、何らかの理由でsubstring(1)ではなく、 substring(0)書きました（この呼び出しは単に元の文字列を返します）。 IDEA自体も問題箇所を強調表示する場合、これは（ドロップされた例外に加えて）2番目のケースです。

longに昇格した整数乗算の結果

この警告は常に本当の危険につながるわけではありませんが、それでも例を示したいと思います 。

 final long length = myIndexStream.length(); long totalCount = length / INDEX_ENTRY_SIZE; // INDEX_ENTRY_SIZE = 26 for(int i=0; i<totalCount; i++) { final long indexOffset = length - (i + 1) * INDEX_ENTRY_SIZE; 

第一に、ループ変数がlongではなくint型であるということはすでに疑わしい（おそらく、このような状況では別個の診断を行う必要がある）。 totalCountが2 ³¹を超える場合、サイクルは終了しません。 しかし、大丈夫、これは52ギガバイトより長い長さインデックスでのみ可能です。 ただし、このコードの問題はすでに2ギガバイトを超える長さで始まります。 iおよびINDEX_ENTRY_SIZEはint型であるため、32ビットの符号付き整数で乗算が実行され、オーバーフローが成功します。 この後、乗算の結果はlongなり、減算を実行した後、変位は長さより長くなる可能性があります。 おそらく、このような大きなキャッシュはこれまで存在していませんでしたが、表示されると不快になります。 簡単な修正方法は、 longループ変数を宣言することです。

Kotlinはどうですか？

IntelliJ IDEAの一部はKotlinで記述されており、これもJavaバイトコードにコンパイルされていることが知られています。 静的バイトコードアナライザーは、任意の言語を正式に分析できますが、実際、アナライザーがJavaで記述されている場合、他の言語では多くの誤検知が発生します。 多くの場合、言語コンパイラが特定のコンストラクト（たとえば、暗黙的なチェック）を生成するために使用されます。 ただし、場合によっては、このような誤検知がコンパイラコードジェネレーターを詳しく調べる機会になることがあります。 たとえば、クラスはcom.intellij.configurationStore.FileBasedStorageKtです。 クラス自体に行があります：

 private val XML_PROLOG = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>".toByteArray() 

java.lang.Stringクラスでは、ご存じのようにtoByteArray()メソッドはそうではありません。 これはKotlinの拡張メソッドであり、インラインメソッド（コンパイラは使用場所に直接埋め込みます）を使用して、デフォルトでString.getBytes(Charsets.UTF_8)実行します。 この行がKotlinでコンパイルされたものを見てみましょう。 バイトコードを直接表示しませんが、より理解しやすいJavaコードに変換します。

 String str = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"; Charset charset = null; int mask = 1; Object obj = null; //         —    //  - nop -   if(obj != null) { throw new UnsupportedOperationException("Super calls with default arguments not supported in this target, function: toByteArray"); } if(mask & 1 == 0) { charset = kotlin.text.Charsets.UTF_8; } //   - nop XML_PROLOG = kotlin.jvm.internal.Intrinsics.checkExpressionValueIsNotNull(((String)str).getBytes(charset), "(this as java.lang.String).getBytes(charset)"); 

ラインが信じられないほど成長したことがわかります。 mask変数は、デフォルトパラメータの転送に関連付けられています（Dmitry ZhemerovがJPointでこれについて話しました- スライド40以下を参照してください。明らかに、ここには多くの余分なものがあり、HuntBugsはobj != null間違いなく誓いobj != null mask & 1コードの作成者は、おそらく時間をかけて非難することはありませんが、おそらく、Kotlinコンパイラーはより賢くなり、ガーベッジの生成が少なくなります。

おわりに

ここでは、静的解析の重要性についてプレーンテキストを書くことができます。Andrey2008は、彼の記事の後に同僚と書いていますが、すべてをすでに知っています。 興味深いことに、静的解析を使用して開発されたコードでさえ、新しいツールでチェックするだけで多くの不審な場所を見つけることができました。 もちろん、すべてが記事に含まれているわけではありません。 誤検知に加えて、多くの重要なメッセージがありますが、退屈です。 パフォーマンスに関する多くの投稿。 たとえば、ループ内の文字列連結は59個です。 または、 values() -18個で高速になった場合、 keySet() + get()でMap値をバイパスします。 多数の潜在的なマルチスレッドの問題。 volatileフィールドへの非アトミック更新が50だとしましょう。 または疑わしいユースnotify() wait() / notify() -8個です。

静的解析を使用して、常に注目してください！