From c2779b626ca8fcc1e36d189496cbfb09fe72865d Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 17 Jun 2011 11:39:48 +0200 Subject: [PATCH] "Pointless null check" inspection improvements --- .../PointlessNullCheckInspection.java | 118 +++++++++++++----- .../PointlessNullCheck.java | 14 ++- .../pointless_null_check/expected.xml | 18 +++ 3 files changed, 117 insertions(+), 33 deletions(-) diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessNullCheckInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessNullCheckInspection.java index 81eeba4f0916..461073801222 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessNullCheckInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessNullCheckInspection.java @@ -4,11 +4,13 @@ import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.IncorrectOperationException; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; +import com.siyeh.ig.psiutils.ParenthesesUtils; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -62,17 +64,20 @@ public class PointlessNullCheckInspection extends BaseInspection { public void doFix(Project project, ProblemDescriptor descriptor) throws IncorrectOperationException { final PsiElement element = descriptor.getPsiElement(); - final PsiElement parent = element.getParent(); - if (!(parent instanceof PsiBinaryExpression)) { + final PsiBinaryExpression binaryExpression = + PsiTreeUtil.getParentOfType(element, + PsiBinaryExpression.class); + if (binaryExpression == null) { return; } - final PsiBinaryExpression binaryExpression = - (PsiBinaryExpression) parent; final PsiExpression lhs = binaryExpression.getLOperand(); final PsiExpression rhs = binaryExpression.getROperand(); - if (lhs instanceof PsiInstanceOfExpression) { + if (rhs == null) { + return; + } + if (PsiTreeUtil.isAncestor(rhs, element, false)) { replaceExpression(binaryExpression, lhs.getText()); - } else if (rhs instanceof PsiInstanceOfExpression) { + } else if (PsiTreeUtil.isAncestor(lhs, element, false)) { replaceExpression(binaryExpression, rhs.getText()); } } @@ -84,42 +89,89 @@ public class PointlessNullCheckInspection extends BaseInspection { @Override public void visitBinaryExpression(PsiBinaryExpression expression) { super.visitBinaryExpression(expression); - if (!expression.getOperationTokenType().equals( - JavaTokenType.ANDAND)) { - return; - } - final PsiExpression lhs = expression.getLOperand(); - final PsiExpression rhs = expression.getROperand(); - + final IElementType operationTokenType = + expression.getOperationTokenType(); + final PsiExpression lhs = ParenthesesUtils.stripParentheses( + expression.getLOperand()); + final PsiExpression rhs = ParenthesesUtils.stripParentheses( + expression.getROperand()); final PsiBinaryExpression binaryExpression; final PsiInstanceOfExpression instanceofExpression; - if (lhs instanceof PsiBinaryExpression && - rhs instanceof PsiInstanceOfExpression) { - binaryExpression = (PsiBinaryExpression) lhs; - instanceofExpression = (PsiInstanceOfExpression) rhs; - } else if (rhs instanceof PsiBinaryExpression && - lhs instanceof PsiInstanceOfExpression) { - binaryExpression = (PsiBinaryExpression) rhs; - instanceofExpression = (PsiInstanceOfExpression) lhs; + if (operationTokenType.equals(JavaTokenType.ANDAND)) { + if (lhs instanceof PsiBinaryExpression && + rhs instanceof PsiInstanceOfExpression) { + binaryExpression = (PsiBinaryExpression) lhs; + instanceofExpression = (PsiInstanceOfExpression) rhs; + } else if (rhs instanceof PsiBinaryExpression && + lhs instanceof PsiInstanceOfExpression) { + binaryExpression = (PsiBinaryExpression) rhs; + instanceofExpression = (PsiInstanceOfExpression) lhs; + } else { + return; + } + final IElementType tokenType = + binaryExpression.getOperationTokenType(); + if (!tokenType.equals(JavaTokenType.NE)) { + return; + } + + } else if (operationTokenType.equals(JavaTokenType.OROR)) { + if (lhs instanceof PsiBinaryExpression && + rhs instanceof PsiPrefixExpression) { + final PsiPrefixExpression prefixExpression = + (PsiPrefixExpression) rhs; + final IElementType prefixTokenType = + prefixExpression.getOperationTokenType(); + if (!JavaTokenType.EXCL.equals(prefixTokenType)) { + return; + } + final PsiExpression operand = + ParenthesesUtils.stripParentheses( + prefixExpression.getOperand()); + if (!(operand instanceof PsiInstanceOfExpression)) { + return; + } + binaryExpression = (PsiBinaryExpression) lhs; + instanceofExpression = (PsiInstanceOfExpression) operand; + } else if (rhs instanceof PsiBinaryExpression && + lhs instanceof PsiPrefixExpression) { + final PsiPrefixExpression prefixExpression = + (PsiPrefixExpression) lhs; + final IElementType prefixTokenType = + prefixExpression.getOperationTokenType(); + if (!JavaTokenType.EXCL.equals(prefixTokenType)) { + return; + } + final PsiExpression operand = + ParenthesesUtils.stripParentheses( + prefixExpression.getOperand()); + if (!(operand instanceof PsiInstanceOfExpression)) { + return; + } + binaryExpression = (PsiBinaryExpression) rhs; + instanceofExpression = (PsiInstanceOfExpression) operand; + } else { + return; + } + final IElementType tokenType = + binaryExpression.getOperationTokenType(); + if (!tokenType.equals(JavaTokenType.EQEQ)) { + return; + } } else { return; } - final IElementType tokenType = - binaryExpression.getOperationTokenType(); - if (!tokenType.equals(JavaTokenType.NE)) { - return; - } final PsiReferenceExpression referenceExpression1 = - getReferenceFromNotNullCheck(binaryExpression); + getReferenceFromNullCheck(binaryExpression); if (referenceExpression1 == null) { return; } final PsiExpression operand = - instanceofExpression.getOperand(); + ParenthesesUtils.stripParentheses( + instanceofExpression.getOperand()); if (!(operand instanceof PsiReferenceExpression)) { return; } - final PsiReferenceExpression referenceExpression2 = (PsiReferenceExpression) operand; final PsiElement target1 = referenceExpression1.resolve(); @@ -131,10 +183,12 @@ public class PointlessNullCheckInspection extends BaseInspection { } @Nullable - private static PsiReferenceExpression getReferenceFromNotNullCheck( + private static PsiReferenceExpression getReferenceFromNullCheck( PsiBinaryExpression expression) { - final PsiExpression lhs = expression.getLOperand(); - final PsiExpression rhs = expression.getROperand(); + final PsiExpression lhs = ParenthesesUtils.stripParentheses( + expression.getLOperand()); + final PsiExpression rhs = ParenthesesUtils.stripParentheses( + expression.getROperand()); if (lhs instanceof PsiReferenceExpression) { if (!(rhs instanceof PsiLiteralExpression && PsiType.NULL.equals(rhs.getType()))) { diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_null_check/PointlessNullCheck.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_null_check/PointlessNullCheck.java index 4e45ac25e7d5..f5b5e0489384 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_null_check/PointlessNullCheck.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_null_check/PointlessNullCheck.java @@ -18,7 +18,19 @@ public class PointlessNullCheck { if (arg instanceof String && arg != null) { System.out.println("this should trigger a warning"); } - } + + if ((arg instanceof String) && (arg != null)) { + System.out.println("this should trigger a warning"); + } + + if (arg == null || !(arg instanceof String)) { + System.out.println("this should trigger a warning"); + } + + if (((arg) != (null)) && ((arg) instanceof String)) { + System.out.println("this should trigger a warning"); + } + } String arg1 = "foo"; diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_null_check/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_null_check/expected.xml index 9a12347e5875..cf13386cc9df 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_null_check/expected.xml +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_null_check/expected.xml @@ -24,4 +24,22 @@ Pointless null check Pointless null check can be removed + + PointlessNullCheck.java + 22 + Pointless null check + Pointless null check can be removed + + + PointlessNullCheck.java + 26 + Pointless null check + Pointless null check can be removed + + + PointlessNullCheck.java + 30 + Pointless null check + Pointless null check can be removed +