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 614c39494ea1..21aa528fadf3 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/InstanceOfUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/InstanceOfUtils.java @@ -35,6 +35,41 @@ public class InstanceOfUtils { } final PsiClassType rawType = classType.rawType(); final InstanceofChecker checker = new InstanceofChecker(operand, rawType, false); + PsiStatement sibling = PsiTreeUtil.getParentOfType(context, PsiStatement.class); + sibling = PsiTreeUtil.getPrevSiblingOfType(sibling, PsiStatement.class); + while (sibling != null) { + if (sibling instanceof PsiIfStatement) { + final PsiIfStatement ifStatement = (PsiIfStatement)sibling; + final PsiExpression condition = ifStatement.getCondition(); + if (condition != null) { + if (!ControlFlowUtils.statementMayCompleteNormally(ifStatement.getThenBranch())) { + checker.negate = true; + checker.checkExpression(condition); + if (checker.hasAgreeingInstanceof()) { + return null; + } + } + else if (!ControlFlowUtils.statementMayCompleteNormally(ifStatement.getElseBranch())) { + checker.negate = false; + checker.checkExpression(condition); + if (checker.hasAgreeingInstanceof()) { + return null; + } + } + } + } + else if (sibling instanceof PsiAssertStatement) { + final PsiAssertStatement assertStatement = (PsiAssertStatement)sibling; + final PsiExpression condition = assertStatement.getAssertCondition(); + checker.negate = false; + checker.checkExpression(condition); + if (checker.hasAgreeingInstanceof()) { + return null; + } + } + sibling = PsiTreeUtil.getPrevSiblingOfType(sibling, PsiStatement.class); + } + checker.negate = false; PsiElement parent = PsiTreeUtil.getParentOfType(context, PsiIfStatement.class, PsiConditionalExpression.class, PsiPolyadicExpression.class); while (parent != null) { @@ -82,7 +117,7 @@ public class InstanceOfUtils { private final PsiReferenceExpression referenceExpression; private final PsiType castType; private final boolean strict; - private boolean inElse = false; + private boolean negate = false; private PsiInstanceOfExpression conflictingInstanceof = null; private boolean agreeingInstanceof = false; @@ -110,21 +145,21 @@ public class InstanceOfUtils { return; } } - if (!inElse && conflictingInstanceof != null) { + if (!negate && conflictingInstanceof != null) { agreeingInstanceof = false; } } else if (tokenType == JavaTokenType.OROR) { for (PsiExpression operand : expression.getOperands()) { if (operand instanceof PsiPrefixExpression && ((PsiPrefixExpression)operand).getOperationTokenType() == JavaTokenType.EXCL) { - inElse = true; + negate = true; } checkExpression(operand); if (agreeingInstanceof) { return; } } - if (inElse && conflictingInstanceof != null) { + if (negate && conflictingInstanceof != null) { agreeingInstanceof = false; } } @@ -133,9 +168,9 @@ public class InstanceOfUtils { @Override public void visitIfStatement(PsiIfStatement ifStatement) { final PsiStatement branch = ifStatement.getElseBranch(); - inElse = branch != null && + negate = branch != null && PsiTreeUtil.isAncestor(branch, referenceExpression, true); - if (inElse) { + if (negate) { if (branch instanceof PsiBlockStatement) { final PsiBlockStatement blockStatement = (PsiBlockStatement)branch; @@ -162,13 +197,13 @@ public class InstanceOfUtils { @Override public void visitConditionalExpression(PsiConditionalExpression expression) { final PsiExpression elseExpression = expression.getElseExpression(); - inElse = elseExpression != null && PsiTreeUtil.isAncestor(elseExpression, referenceExpression, true); + negate = elseExpression != null && PsiTreeUtil.isAncestor(elseExpression, referenceExpression, true); checkExpression(expression.getCondition()); } private void checkExpression(PsiExpression expression) { expression = PsiUtil.deparenthesizeExpression(expression); - if (inElse) { + if (negate) { if (expression instanceof PsiPrefixExpression) { final PsiPrefixExpression prefixExpression = (PsiPrefixExpression)expression; @@ -202,7 +237,7 @@ public class InstanceOfUtils { agreeingInstanceof = true; conflictingInstanceof = null; } - else if (isConflicting(instanceOfExpression)) { + else if (isConflicting(instanceOfExpression) && conflictingInstanceof == null) { conflictingInstanceof = instanceOfExpression; } } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/bugs/castConflicts/replaceInstanceofInFront.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/bugs/castConflicts/replaceInstanceofInFront.after.java new file mode 100644 index 000000000000..493e7b34b5e0 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/bugs/castConflicts/replaceInstanceofInFront.after.java @@ -0,0 +1,8 @@ +public class Test { + void foo(Object o) { + if (o instanceof Number) { + assert o instanceof Integer; + Integer i = (Integer)o; + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/bugs/castConflicts/replaceInstanceofInFront.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/bugs/castConflicts/replaceInstanceofInFront.java new file mode 100644 index 000000000000..581b710f39ae --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/bugs/castConflicts/replaceInstanceofInFront.java @@ -0,0 +1,8 @@ +public class Test { + void foo(Object o) { + if (o instanceof Number) { + assert o instanceof String; + Integer i = (Integer)o; + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/AssertCheckBefore.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/AssertCheckBefore.java new file mode 100644 index 000000000000..566ae6107a93 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/AssertCheckBefore.java @@ -0,0 +1,10 @@ +class AssertCheckBefore { + void m(Object child, Object parent) { + if (parent instanceof Number) { + if (child instanceof String) { + assert parent instanceof Integer; + Integer attribute = (Integer) parent; + } + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/IfCheckBefore.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/IfCheckBefore.java new file mode 100644 index 000000000000..5dc197d90fbe --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/IfCheckBefore.java @@ -0,0 +1,12 @@ +class IfCheckBefore { + void m(Object child, Object parent) { + if (parent instanceof Number) { + if (child instanceof String) { + if (!(parent instanceof Integer)) { + return; + } + Integer attribute = (Integer) parent; + } + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/IfElseCheckBefore.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/IfElseCheckBefore.java new file mode 100644 index 000000000000..7303650d78d9 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/cast_conflicts_with_instanceof/IfElseCheckBefore.java @@ -0,0 +1,15 @@ +class IfCheckBefore { + void m(Object child, Object parent) { + if (parent instanceof Number) { + if (child instanceof String) { + if ((parent instanceof Integer)) { + System.out.println(parent); + } + else { + return; + } + Integer attribute = (Integer) parent; + } + } + } +} \ 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 369e2f48e1e9..2876ea89c956 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/CastConflictsWithInstanceofInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/CastConflictsWithInstanceofInspectionTest.java @@ -49,6 +49,18 @@ public class CastConflictsWithInstanceofInspectionTest extends LightInspectionTe doTest(); } + public void testIfCheckBefore() { + doTest(); + } + + public void testIfElseCheckBefore() { + doTest(); + } + + public void testAssertCheckBefore() { + doTest(); + } + @Nullable @Override protected InspectionProfileEntry getInspection() { diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/bugs/CastConflictsWithInstanceofFixesTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/bugs/CastConflictsWithInstanceofFixesTest.java index d97b55a40036..26cee5c63a7e 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/bugs/CastConflictsWithInstanceofFixesTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/bugs/CastConflictsWithInstanceofFixesTest.java @@ -49,6 +49,10 @@ public class CastConflictsWithInstanceofFixesTest extends IGQuickFixesTestCase { assertQuickfixNotAvailable("Replace 'E' with 'String' in cast"); } + public void testReplaceInstanceofInFront() { + doTest("replaceInstanceofInFront", "Replace 'String' with 'Integer' in instanceof"); + } + @Override protected String getRelativePath() { return "bugs/castConflicts";