From f188b0bb2b2a5e27361080bf531670793f654184 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 21 Jan 2020 11:53:07 +0700 Subject: [PATCH] CastConflictsWithInstanceofInspection: support while/do-while/for loops (IDEA-229915) GitOrigin-RevId: 68d1ca2c40d05c838b08b21bcbd98277dffd20b3 --- .../siyeh/ig/psiutils/InstanceOfUtils.java | 142 ++++++++---------- .../WhileOrChain.java | 11 ++ ...ConflictsWithInstanceofInspectionTest.java | 1 + 3 files changed, 73 insertions(+), 81 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/WhileOrChain.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/InstanceOfUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/InstanceOfUtils.java index 7aea23a81d56..9f12c30afeef 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/InstanceOfUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/InstanceOfUtils.java @@ -25,6 +25,7 @@ import com.intellij.psi.util.PsiUtil; import com.intellij.util.ObjectUtils; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.List; import java.util.OptionalInt; @@ -83,15 +84,13 @@ public class InstanceOfUtils { sibling = PsiTreeUtil.getPrevSiblingOfType(sibling, PsiStatement.class); } checker.negate = false; - PsiElement parent = PsiTreeUtil.getParentOfType(context, PsiIfStatement.class, PsiConditionalExpression.class, - PsiPolyadicExpression.class); + PsiElement parent = findInterestingParent(context); while (parent != null) { parent.accept(checker); if (checker.hasAgreeingInstanceof()) { return null; } - parent = PsiTreeUtil.getParentOfType(parent, PsiPolyadicExpression.class, PsiIfStatement.class, - PsiConditionalExpression.class); + parent = findInterestingParent(parent); } if (checker.hasAgreeingInstanceof()) { return null; @@ -99,6 +98,13 @@ public class InstanceOfUtils { return checker.getConflictingInstanceof(); } + @Nullable + private static PsiElement findInterestingParent(PsiElement context) { + return PsiTreeUtil.getParentOfType( + context, PsiIfStatement.class, PsiConditionalExpression.class, PsiPolyadicExpression.class, + PsiConditionalLoopStatement.class); + } + private static boolean isInstanceOfAssertionCall(InstanceofChecker checker, PsiMethodCallExpression call) { if (call == null) return false; List contracts = JavaMethodContractUtil.getMethodCallContracts(call); @@ -121,28 +127,17 @@ public class InstanceOfUtils { return checker.hasAgreeingInstanceof(); } - public static boolean hasAgreeingInstanceof( - @NotNull PsiTypeCastExpression expression) { + public static boolean hasAgreeingInstanceof(@NotNull PsiTypeCastExpression expression) { final PsiType castType = expression.getType(); final PsiExpression operand = expression.getOperand(); - if (!(operand instanceof PsiReferenceExpression)) { - return false; - } - final PsiReferenceExpression referenceExpression = - (PsiReferenceExpression)operand; - final InstanceofChecker checker = new InstanceofChecker( - referenceExpression, castType, false); - PsiElement parent = PsiTreeUtil.getParentOfType(expression, - PsiIfStatement.class, - PsiConditionalExpression.class, PsiPolyadicExpression.class); + if (!(operand instanceof PsiReferenceExpression)) return false; + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)operand; + final InstanceofChecker checker = new InstanceofChecker(referenceExpression, castType, false); + PsiElement parent = findInterestingParent(expression); while (parent != null) { parent.accept(checker); - if (checker.hasAgreeingInstanceof()) { - return true; - } - parent = PsiTreeUtil.getParentOfType(parent, - PsiIfStatement.class, - PsiConditionalExpression.class, PsiPolyadicExpression.class); + if (checker.hasAgreeingInstanceof()) return true; + parent = findInterestingParent(parent); } return false; } @@ -165,8 +160,7 @@ public class InstanceOfUtils { } @Override - public void visitReferenceExpression( - PsiReferenceExpression expression) { + public void visitReferenceExpression(PsiReferenceExpression expression) { visitExpression(expression); } @@ -201,39 +195,44 @@ public class InstanceOfUtils { } } + @Override + public void visitForStatement(PsiForStatement statement) { + processConditionalLoop(statement); + } + + @Override + public void visitWhileStatement(PsiWhileStatement statement) { + processConditionalLoop(statement); + } + + @Override + public void visitDoWhileStatement(PsiDoWhileStatement statement) { + processConditionalLoop(statement); + } + + private void processConditionalLoop(PsiConditionalLoopStatement loop) { + PsiStatement body = loop.getBody(); + if (!PsiTreeUtil.isAncestor(body, referenceExpression, true)) return; + if (isReassignedInside(body)) return; + checkExpression(loop.getCondition()); + } + @Override public void visitIfStatement(PsiIfStatement ifStatement) { - final PsiStatement branch = ifStatement.getElseBranch(); - negate = branch != null && - PsiTreeUtil.isAncestor(branch, referenceExpression, true); - if (negate) { - if (branch instanceof PsiBlockStatement) { - final PsiBlockStatement blockStatement = - (PsiBlockStatement)branch; - if (VariableAccessUtils.variableIsAssignedBeforeReference( - referenceExpression, blockStatement)) { - return; - } - } - } - else { - final PsiStatement thenBranch = ifStatement.getThenBranch(); - if (thenBranch instanceof PsiBlockStatement) { - final PsiBlockStatement blockStatement = - (PsiBlockStatement)thenBranch; - if (VariableAccessUtils.variableIsAssignedBeforeReference( - referenceExpression, blockStatement)) { - return; - } - } - } + final PsiStatement elseBranch = ifStatement.getElseBranch(); + negate = PsiTreeUtil.isAncestor(elseBranch, referenceExpression, true); + if (isReassignedInside(negate ? elseBranch : ifStatement.getThenBranch())) return; checkExpression(ifStatement.getCondition()); } + private boolean isReassignedInside(PsiStatement branch) { + return branch instanceof PsiBlockStatement && VariableAccessUtils.variableIsAssignedBeforeReference(referenceExpression, branch); + } + @Override public void visitConditionalExpression(PsiConditionalExpression expression) { final PsiExpression elseExpression = expression.getElseExpression(); - negate = elseExpression != null && PsiTreeUtil.isAncestor(elseExpression, referenceExpression, true); + negate = PsiTreeUtil.isAncestor(elseExpression, referenceExpression, true); checkExpression(expression.getCondition()); } @@ -241,15 +240,10 @@ public class InstanceOfUtils { expression = PsiUtil.deparenthesizeExpression(expression); if (negate) { if (expression instanceof PsiPrefixExpression) { - final PsiPrefixExpression prefixExpression = - (PsiPrefixExpression)expression; - final IElementType tokenType = - prefixExpression.getOperationTokenType(); - if (tokenType != JavaTokenType.EXCL) { - return; - } - expression = PsiUtil.deparenthesizeExpression( - prefixExpression.getOperand()); + final PsiPrefixExpression prefixExpression = (PsiPrefixExpression)expression; + final IElementType tokenType = prefixExpression.getOperationTokenType(); + if (tokenType != JavaTokenType.EXCL) return; + expression = PsiUtil.deparenthesizeExpression(prefixExpression.getOperand()); checkInstanceOfExpression(expression); } } @@ -257,18 +251,14 @@ public class InstanceOfUtils { checkInstanceOfExpression(expression); } if (expression instanceof PsiPolyadicExpression) { - final PsiPolyadicExpression binaryExpression = - (PsiPolyadicExpression)expression; + final PsiPolyadicExpression binaryExpression = (PsiPolyadicExpression)expression; visitPolyadicExpression(binaryExpression); } } private void checkInstanceOfExpression(PsiExpression expression) { - if (!(expression instanceof PsiInstanceOfExpression)) { - return; - } - final PsiInstanceOfExpression instanceOfExpression = - (PsiInstanceOfExpression)expression; + if (!(expression instanceof PsiInstanceOfExpression)) return; + final PsiInstanceOfExpression instanceOfExpression = (PsiInstanceOfExpression)expression; if (isAgreeing(instanceOfExpression)) { agreeingInstanceof = true; conflictingInstanceof = null; @@ -280,40 +270,30 @@ public class InstanceOfUtils { private boolean isConflicting(PsiInstanceOfExpression expression) { final PsiExpression conditionOperand = expression.getOperand(); - if (!EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent( - referenceExpression, conditionOperand)) { + if (!EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(referenceExpression, conditionOperand)) { return false; } final PsiTypeElement typeElement = expression.getCheckType(); - if (typeElement == null) { - return false; - } + if (typeElement == null) return false; final PsiType type = typeElement.getType(); if (strict) { return !castType.equals(type); } - else { - return !castType.isAssignableFrom(type); - } + return !castType.isAssignableFrom(type); } private boolean isAgreeing(PsiInstanceOfExpression expression) { final PsiExpression conditionOperand = expression.getOperand(); - if (!EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent( - referenceExpression, conditionOperand)) { + if (!EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(referenceExpression, conditionOperand)) { return false; } final PsiTypeElement typeElement = expression.getCheckType(); - if (typeElement == null) { - return false; - } + if (typeElement == null) return false; final PsiType type = typeElement.getType(); if (strict) { return castType.equals(type); } - else { - return castType.isAssignableFrom(type); - } + return castType.isAssignableFrom(type); } public boolean hasAgreeingInstanceof() { diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/WhileOrChain.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/WhileOrChain.java new file mode 100644 index 000000000000..c7874d713b2b --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/WhileOrChain.java @@ -0,0 +1,11 @@ +class WhileOrChain { + void test(Object obj) { + while (obj instanceof String || obj instanceof Number) { + if (obj instanceof String || ((Number)obj).intValue() == 0) { + obj = update(); + } else break; + } + } + + native Object update(); +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/CastConflictsWithInstanceofInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/CastConflictsWithInstanceofInspectionTest.java index a0822cc3c695..46cda42b064a 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/CastConflictsWithInstanceofInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/CastConflictsWithInstanceofInspectionTest.java @@ -33,6 +33,7 @@ public class CastConflictsWithInstanceofInspectionTest extends LightJavaInspecti public void testAssertCheckBefore() { doTest(); } public void testAssertionMethodCheckBefore() { doTest(); } public void testCastMethod() { doTest(); } + public void testWhileOrChain() { doTest(); } @Nullable @Override