From 3d216927b7239d028d527925ef5a56bd7d008b8d Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 23 Mar 2017 10:43:10 +0700 Subject: [PATCH] DataFlowInspection: do not report some common compile-time flag checks --- .../dataFlow/DataFlowInspectionBase.java | 46 +++++++++++++--- .../dataFlow/fixture/SuppressStaticFlags.java | 54 +++++++++++++++++++ .../DataFlowInspectionTest.java | 2 + 3 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/SuppressStaticFlags.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index 6d3e42b93c6f..41864a95ad7e 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -53,6 +53,8 @@ import com.intellij.refactoring.extractMethod.ExtractMethodUtil; import com.intellij.util.*; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MultiMap; +import com.siyeh.ig.psiutils.ComparisonUtils; +import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.TypeUtils; import one.util.streamex.StreamEx; import org.jdom.Element; @@ -63,6 +65,7 @@ import org.jetbrains.annotations.Nullable; import javax.swing.*; import java.util.*; +@SuppressWarnings("ConditionalExpressionWithIdenticalBranches") public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.DataFlowInspection"); @NonNls private static final String SHORT_NAME = "ConstantConditions"; @@ -777,18 +780,49 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { null; if (!PsiTreeUtil.isAncestor(topExpression, element, false)) return false; - if (isCompileTimeFlagReference(topExpression)) return true; - - Collection refs = PsiTreeUtil.findChildrenOfType(topExpression, PsiReferenceExpression.class); - return ContainerUtil.or(refs, DataFlowInspectionBase::isCompileTimeFlagReference); + return StreamEx.ofTree(topExpression, e -> StreamEx.of(e.getChildren())) + .anyMatch(DataFlowInspectionBase::isCompileTimeFlagCheck); } - private static boolean isCompileTimeFlagReference(PsiElement element) { + private static boolean isCompileTimeFlagCheck(PsiElement element) { + if(element instanceof PsiBinaryExpression) { + PsiBinaryExpression binOp = (PsiBinaryExpression)element; + if(ComparisonUtils.isComparisonOperation(binOp.getOperationTokenType())) { + PsiExpression comparedWith = null; + if(ExpressionUtils.isLiteral(binOp.getROperand())) { + comparedWith = binOp.getLOperand(); + } else if(ExpressionUtils.isLiteral(binOp.getLOperand())) { + comparedWith = binOp.getROperand(); + } + comparedWith = PsiUtil.skipParenthesizedExprDown(comparedWith); + if (isConstantOfType(comparedWith, PsiType.INT, PsiType.LONG)) { + // like "if(DEBUG_LEVEL > 2)" + return true; + } + if(comparedWith instanceof PsiBinaryExpression) { + PsiBinaryExpression subOp = (PsiBinaryExpression)comparedWith; + if(subOp.getOperationTokenType().equals(JavaTokenType.AND)) { + PsiExpression left = PsiUtil.skipParenthesizedExprDown(subOp.getLOperand()); + PsiExpression right = PsiUtil.skipParenthesizedExprDown(subOp.getROperand()); + if(isConstantOfType(left, PsiType.INT, PsiType.LONG) || + isConstantOfType(right, PsiType.INT, PsiType.LONG)) { + // like "if((FLAGS & SOME_FLAG) != 0)" + return true; + } + } + } + } + } + // like "if(DEBUG)" + return isConstantOfType(element, PsiType.BOOLEAN); + } + + private static boolean isConstantOfType(PsiElement element, PsiPrimitiveType... types) { PsiElement resolved = element instanceof PsiReferenceExpression ? ((PsiReferenceExpression)element).resolve() : null; if (!(resolved instanceof PsiField)) return false; PsiField field = (PsiField)resolved; return field.hasModifierProperty(PsiModifier.FINAL) && field.hasModifierProperty(PsiModifier.STATIC) && - PsiType.BOOLEAN.equals(field.getType()); + ArrayUtil.contains(field.getType(), types); } private static boolean isNullLiteralExpression(PsiElement expr) { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/SuppressStaticFlags.java b/java/java-tests/testData/inspection/dataFlow/fixture/SuppressStaticFlags.java new file mode 100644 index 000000000000..6dd1af778cb2 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/SuppressStaticFlags.java @@ -0,0 +1,54 @@ +public class SuppressStaticFlags { + private static final boolean DEBUG = true; + private static final int DEBUG_LEVEL = 2; + + private static final int FLAGS = 0x03; + private static final int FLAG1 = 0x01; + private static final int FLAG2 = 0x02; + + private static final long LONG_FLAGS = 0x03; + private static final long LONG_FLAG1 = 0x01; + private static final long LONG_FLAG2 = 0x02; + + public void testDebug(String s) { + if(DEBUG) { + System.out.println("Debug is on"); + } + if(DEBUG && s.length() == 0) { + System.out.println("String is empty"); + } + } + + public void testDebugLevel(String s) { + if(DEBUG_LEVEL == 3) { + System.out.println("Debug level is high"); + } + if(DEBUG_LEVEL > 1 && s.length() == 0) { + System.out.println("String is empty"); + } + } + + public void testFlags(String s) { + if((FLAGS & 1) == 0) { + System.out.println("No flag"); + } + if(((FLAGS) & (FLAG2)) != 0 && s.length() == 0) { + System.out.println("Flag2"); + } + if((3 & 1) == 2) { + System.out.println("Literals"); + } + } + + public void testLongFlags(String s) { + if((LONG_FLAGS & 1) == 0) { + System.out.println("No flag"); + } + if(((LONG_FLAGS) & (LONG_FLAG2)) != 0 && s == null) { + System.out.println("Flag2"); + } + if((3L & 1L) == 2L) { + System.out.println("Literals"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java index a283eeee1b9e..927b58ab691f 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -55,6 +55,8 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testCanBeNullDoesntImplyIsNull() throws Throwable { doTest(); } public void testAnnReport() throws Throwable { doTest(); } + public void testSuppressStaticFlags() throws Throwable { doTest(); } + public void testBigMethodNotComplex() throws Throwable { doTest(); } public void testBuildRegexpNotComplex() throws Throwable { doTest(); } public void testTernaryInWhileNotComplex() throws Throwable { doTest(); }