diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/OperatorPrecedence.java b/core/src/main/java/com/google/errorprone/bugpatterns/OperatorPrecedence.java index 713c946d07cd..1b706d8d27df 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/OperatorPrecedence.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/OperatorPrecedence.java @@ -19,6 +19,7 @@ import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.util.ASTHelpers.getStartPosition; +import static com.google.errorprone.util.ASTHelpers.getType; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; @@ -26,13 +27,17 @@ import com.google.errorprone.BugPattern.StandardTags; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher; +import com.google.errorprone.bugpatterns.BugChecker.ConditionalExpressionTreeMatcher; import com.google.errorprone.fixes.SuggestedFix; import com.google.errorprone.matchers.Description; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.BinaryTree; +import com.sun.source.tree.ConditionalExpressionTree; +import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.ParenthesizedTree; import com.sun.source.tree.Tree; import com.sun.source.tree.Tree.Kind; +import com.sun.tools.javac.code.TypeTag; import com.sun.tools.javac.tree.JCTree.JCBinary; import com.sun.tools.javac.tree.TreeInfo; @@ -41,7 +46,8 @@ summary = "Use grouping parenthesis to make the operator precedence explicit", severity = WARNING, tags = StandardTags.STYLE) -public class OperatorPrecedence extends BugChecker implements BinaryTreeMatcher { +public class OperatorPrecedence extends BugChecker + implements BinaryTreeMatcher, ConditionalExpressionTreeMatcher { private static final ImmutableSet CONDITIONAL = Sets.immutableEnumSet(Kind.AND, Kind.OR, Kind.XOR, Kind.CONDITIONAL_AND, Kind.CONDITIONAL_OR); @@ -71,6 +77,22 @@ public Description matchBinary(BinaryTree tree, VisitorState state) { return createAppropriateFix(tree, state); } + @Override + public Description matchConditionalExpression( + ConditionalExpressionTree tree, VisitorState state) { + ExpressionTree condition = tree.getCondition(); + if (!(condition instanceof BinaryTree)) { + return NO_MATCH; + } + if (!CONDITIONAL.contains(condition.getKind())) { + return NO_MATCH; + } + if (!state.getTypes().unboxedTypeOrType(getType(tree)).hasTag(TypeTag.BOOLEAN)) { + return NO_MATCH; + } + return basicFix((BinaryTree) condition); + } + private static boolean isConfusing(Kind thisKind, Kind parentKind) { if (CONDITIONAL.contains(thisKind) && CONDITIONAL.contains(parentKind)) { return true; diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/OperatorPrecedenceTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/OperatorPrecedenceTest.java index 5fcb78bb0cef..4e6a25f52165 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/OperatorPrecedenceTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/OperatorPrecedenceTest.java @@ -191,4 +191,42 @@ public void aLotOfParenthesis() { "}") .doTest(); } + + @Test + public void conditionalBoolean() { + helper + .addInputLines( + "Test.java", // + "class Test {", + " void f(boolean a, boolean b, boolean c, boolean d) {", + " boolean g = a || b ? c : d;", + " g = a && b ? c : d;", + " g = a == b ? c : d;", + " }", + "}") + .addOutputLines( + "Test.java", // + "class Test {", + " void f(boolean a, boolean b, boolean c, boolean d) {", + " boolean g = (a || b) ? c : d;", + " g = (a && b) ? c : d;", + " g = a == b ? c : d;", + " }", + "}") + .doTest(); + } + + @Test + public void conditionOtherType() { + helper + .addInputLines( + "Test.java", // + "class Test {", + " void f(boolean a, boolean b, String c, String d) {", + " String g = a || b ? c : d;", + " }", + "}") + .expectUnchanged() + .doTest(); + } }