From 04bd75efdde3cbcdc13d309b1ef31fd09ed3c394 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Thu, 28 Mar 2013 11:37:31 +0100 Subject: [PATCH] teach "Duplicate condition in 'if' statement" inspection about polyadic expressions --- .../DuplicateConditionInspection.java | 73 ++++++++----------- .../DuplicateCondition.html | 3 + .../DuplicateCondition.java | 12 +++ .../duplicate_condition/expected.xml | 37 ++++++++++ .../DuplicateConditionInspectionTest.java | 10 +++ 5 files changed, 91 insertions(+), 44 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/duplicate_condition/DuplicateCondition.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/duplicate_condition/expected.xml create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/DuplicateConditionInspectionTest.java diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/DuplicateConditionInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/DuplicateConditionInspection.java index 0962eb7784ab..905868060d5c 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/DuplicateConditionInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/DuplicateConditionInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2003-2007 Dave Griffith, Bas Leijdekkers + * Copyright 2003-2013 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -42,30 +42,25 @@ public class DuplicateConditionInspection extends BaseInspection { @NotNull public String getDisplayName() { - return InspectionGadgetsBundle.message( - "duplicate.condition.display.name"); + return InspectionGadgetsBundle.message("duplicate.condition.display.name"); } @NotNull protected String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "duplicate.condition.problem.descriptor"); + return InspectionGadgetsBundle.message("duplicate.condition.problem.descriptor"); } @Nullable public JComponent createOptionsPanel() { - return new SingleCheckboxOptionsPanel( - InspectionGadgetsBundle.message( - "duplicate.condition.ignore.method.calls.option"), - this, "ignoreMethodCalls"); + return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message("duplicate.condition.ignore.method.calls.option"), + this, "ignoreMethodCalls"); } public BaseInspectionVisitor buildVisitor() { return new DuplicateConditionVisitor(); } - private class DuplicateConditionVisitor - extends BaseInspectionVisitor { + private class DuplicateConditionVisitor extends BaseInspectionVisitor { @Override public void visitIfStatement(@NotNull PsiIfStatement statement) { @@ -84,8 +79,7 @@ public class DuplicateConditionInspection extends BaseInspection { if (numConditions < 2) { return; } - final PsiExpression[] conditionArray = - conditions.toArray(new PsiExpression[numConditions]); + final PsiExpression[] conditionArray = conditions.toArray(new PsiExpression[numConditions]); final boolean[] matched = new boolean[conditionArray.length]; Arrays.fill(matched, false); for (int i = 0; i < conditionArray.length; i++) { @@ -98,60 +92,51 @@ public class DuplicateConditionInspection extends BaseInspection { continue; } final PsiExpression testCondition = conditionArray[j]; - final boolean areEquivalent = - EquivalenceChecker.expressionsAreEquivalent( - condition, testCondition); + final boolean areEquivalent = EquivalenceChecker.expressionsAreEquivalent(condition, testCondition); if (areEquivalent) { + if (!ignoreMethodCalls || !containsMethodCallExpression(testCondition)) { + registerError(testCondition); + if (!matched[i]) { + registerError(condition); + } + } matched[i] = true; matched[j] = true; - if (ignoreMethodCalls && - containsMethodCallExpression(testCondition)) { - break; - } - registerError(testCondition); - if (!matched[i]) { - registerError(condition); - } } } } } - private void collectConditionsForIfStatement( - PsiIfStatement statement, Set conditions, int depth) { - if (depth > LIMIT_DEPTH) return; - + private void collectConditionsForIfStatement(PsiIfStatement statement, Set conditions, int depth) { + if (depth > LIMIT_DEPTH) { + return; + } final PsiExpression condition = statement.getCondition(); collectConditionsForExpression(condition, conditions); final PsiStatement branch = statement.getElseBranch(); if (branch instanceof PsiIfStatement) { - collectConditionsForIfStatement((PsiIfStatement)branch, - conditions, depth + 1); + collectConditionsForIfStatement((PsiIfStatement)branch, conditions, depth + 1); } } - private void collectConditionsForExpression( - PsiExpression condition, Set conditions) { + private void collectConditionsForExpression(PsiExpression condition, Set conditions) { if (condition == null) { return; } if (condition instanceof PsiParenthesizedExpression) { - final PsiParenthesizedExpression parenthesizedExpression = - (PsiParenthesizedExpression)condition; - final PsiExpression contents = - parenthesizedExpression.getExpression(); + final PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression)condition; + final PsiExpression contents = parenthesizedExpression.getExpression(); collectConditionsForExpression(contents, conditions); return; } - if (condition instanceof PsiBinaryExpression) { - final PsiBinaryExpression binaryExpression = - (PsiBinaryExpression)condition; - final IElementType tokenType = binaryExpression.getOperationTokenType(); + if (condition instanceof PsiPolyadicExpression) { + final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)condition; + final IElementType tokenType = polyadicExpression.getOperationTokenType(); if (JavaTokenType.OROR.equals(tokenType)) { - final PsiExpression lhs = binaryExpression.getLOperand(); - collectConditionsForExpression(lhs, conditions); - final PsiExpression rhs = binaryExpression.getROperand(); - collectConditionsForExpression(rhs, conditions); + final PsiExpression[] operands = polyadicExpression.getOperands(); + for (PsiExpression operand : operands) { + collectConditionsForExpression(operand, conditions); + } return; } } diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/DuplicateCondition.html b/plugins/InspectionGadgets/src/inspectionDescriptions/DuplicateCondition.html index 242fdae23897..f6fa1cc1b5a3 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/DuplicateCondition.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/DuplicateCondition.html @@ -5,6 +5,9 @@ Reports on any duplicate conditions among different branches of an desired semantics, duplicate conditions usually represent programmer oversight.

+Use the checkbox below to let this inspection ignore conditions containing method calls. Some method calls may return a different value +on an identical invocation. +

Powered by InspectionGadgets \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/duplicate_condition/DuplicateCondition.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/duplicate_condition/DuplicateCondition.java new file mode 100644 index 000000000000..ef047398e14f --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/duplicate_condition/DuplicateCondition.java @@ -0,0 +1,12 @@ +package com.siyeh.igtest.controlflow.duplicate_condition; + +public class DuplicateCondition { + + void x(boolean b) { + if (b || b || b ) { + + } else if (b) { + + } else if (b) {} + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/duplicate_condition/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/duplicate_condition/expected.xml new file mode 100644 index 000000000000..47f1e352b5e6 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/duplicate_condition/expected.xml @@ -0,0 +1,37 @@ + + + + DuplicateCondition.java + 6 + Duplicate condition in 'if' statement + Duplicate condition <code>b</code> #loc + + + + DuplicateCondition.java + 6 + Duplicate condition in 'if' statement + Duplicate condition <code>b</code> #loc + + + + DuplicateCondition.java + 8 + Duplicate condition in 'if' statement + Duplicate condition <code>b</code> #loc + + + + DuplicateCondition.java + 6 + Duplicate condition in 'if' statement + Duplicate condition <code>b</code> #loc + + + + DuplicateCondition.java + 10 + Duplicate condition in 'if' statement + Duplicate condition <code>b</code> #loc + + \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/DuplicateConditionInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/DuplicateConditionInspectionTest.java new file mode 100644 index 000000000000..aaa379cf6ad1 --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/controlflow/DuplicateConditionInspectionTest.java @@ -0,0 +1,10 @@ +package com.siyeh.ig.controlflow; + +import com.siyeh.ig.IGInspectionTestCase; + +public class DuplicateConditionInspectionTest extends IGInspectionTestCase { + + public void test() throws Exception { + doTest("com/siyeh/igtest/controlflow/duplicate_condition", new DuplicateConditionInspection()); + } +} \ No newline at end of file