diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/PointlessNullCheckInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/PointlessNullCheckInspection.java index ecb70eaa9ebe..80d66685ee19 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/PointlessNullCheckInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/PointlessNullCheckInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2013 Jetbrains s.r.o. + * Copyright 2011-2016 Jetbrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,7 @@ import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.PsiReplacementUtil; import com.siyeh.ig.psiutils.ParenthesesUtils; +import com.siyeh.ig.psiutils.VariableAccessUtils; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -111,69 +112,124 @@ public class PointlessNullCheckInspection extends BaseInspection { private static class PointlessNullCheckVisitor extends BaseInspectionVisitor { @Override - public void visitBinaryExpression(PsiBinaryExpression expression) { - super.visitBinaryExpression(expression); + public void visitPolyadicExpression(PsiPolyadicExpression expression) { + super.visitPolyadicExpression(expression); final IElementType operationTokenType = expression.getOperationTokenType(); - final PsiExpression lhs = ParenthesesUtils.stripParentheses(expression.getLOperand()); - final PsiExpression rhs = ParenthesesUtils.stripParentheses(expression.getROperand()); - final PsiBinaryExpression binaryExpression; - final PsiExpression possibleInstanceofExpression; if (operationTokenType.equals(JavaTokenType.ANDAND)) { - if (lhs instanceof PsiBinaryExpression) { - binaryExpression = (PsiBinaryExpression)lhs; - possibleInstanceofExpression = rhs; - } - else if (rhs instanceof PsiBinaryExpression) { - binaryExpression = (PsiBinaryExpression)rhs; - possibleInstanceofExpression = lhs; - } - else { - return; - } - final IElementType tokenType = binaryExpression.getOperationTokenType(); - if (!tokenType.equals(JavaTokenType.NE)) { - return; + final PsiExpression[] operands = expression.getOperands(); + for (int i = 0; i < operands.length - 1; i++) { + for (int j = i + 1; j < operands.length; j++) { + if (checkAndedExpressions(operands, i, j)) { + 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[] operands = expression.getOperands(); + for (int i = 0; i < operands.length - 1; i++) { + for (int j = i + 1; j < operands.length; j++) { + if (checkOrredExpressions(operands, i, j)) { + return; + } } - binaryExpression = (PsiBinaryExpression)lhs; - possibleInstanceofExpression = ParenthesesUtils.stripParentheses(prefixExpression.getOperand()); - } - else if (rhs instanceof PsiBinaryExpression && lhs instanceof PsiPrefixExpression) { - final PsiPrefixExpression prefixExpression = (PsiPrefixExpression)lhs; - final IElementType prefixTokenType = prefixExpression.getOperationTokenType(); - if (!JavaTokenType.EXCL.equals(prefixTokenType)) { - return; - } - binaryExpression = (PsiBinaryExpression)rhs; - possibleInstanceofExpression = ParenthesesUtils.stripParentheses(prefixExpression.getOperand()); - } - else { - return; - } - final IElementType tokenType = binaryExpression.getOperationTokenType(); - if (!tokenType.equals(JavaTokenType.EQEQ)) { - return; } } + } + + public boolean checkOrredExpressions(PsiExpression[] operands, int i, int j) { + final PsiExpression lhs = ParenthesesUtils.stripParentheses(operands[i]); + final PsiExpression rhs = ParenthesesUtils.stripParentheses(operands[j]); + final PsiBinaryExpression binaryExpression; + final PsiPrefixExpression prefixExpression; + final boolean checkRef; + if (lhs instanceof PsiBinaryExpression && rhs instanceof PsiPrefixExpression) { + prefixExpression = (PsiPrefixExpression)rhs; + binaryExpression = (PsiBinaryExpression)lhs; + checkRef = true; + } + else if (rhs instanceof PsiBinaryExpression && lhs instanceof PsiPrefixExpression) { + prefixExpression = (PsiPrefixExpression)lhs; + binaryExpression = (PsiBinaryExpression)rhs; + checkRef = false; + } else { - return; + return false; } - final PsiReferenceExpression referenceExpression1 = getReferenceFromNullCheck(binaryExpression); - if (referenceExpression1 == null) { - return; + final IElementType prefixTokenType = prefixExpression.getOperationTokenType(); + if (!JavaTokenType.EXCL.equals(prefixTokenType)) { + return false; } - final PsiReferenceExpression referenceExpression2 = getReferenceFromInstanceofExpression(possibleInstanceofExpression); - if (!referencesEqual(referenceExpression1, referenceExpression2)) { - return; + final PsiExpression possibleInstanceofExpression = ParenthesesUtils.stripParentheses(prefixExpression.getOperand()); + final IElementType tokenType = binaryExpression.getOperationTokenType(); + if (!tokenType.equals(JavaTokenType.EQEQ)) { + return false; + } + final PsiVariable variable = checkExpressions(binaryExpression, possibleInstanceofExpression); + if (variable == null || checkRef && isVariableUsed(operands, i, j, variable)) { + return false; } registerError(binaryExpression, binaryExpression); + return true; + } + + public boolean checkAndedExpressions(PsiExpression[] operands, int i, int j) { + final PsiExpression lhs = ParenthesesUtils.stripParentheses(operands[i]); + final PsiExpression rhs = ParenthesesUtils.stripParentheses(operands[j]); + final PsiBinaryExpression binaryExpression; + final PsiExpression possibleInstanceofExpression; + final boolean checkRef; + if (lhs instanceof PsiBinaryExpression) { + binaryExpression = (PsiBinaryExpression)lhs; + possibleInstanceofExpression = rhs; + checkRef = true; + } + else if (rhs instanceof PsiBinaryExpression) { + binaryExpression = (PsiBinaryExpression)rhs; + possibleInstanceofExpression = lhs; + checkRef = false; + } + else { + return false; + } + final IElementType tokenType = binaryExpression.getOperationTokenType(); + if (!tokenType.equals(JavaTokenType.NE)) { + return false; + } + final PsiVariable variable = checkExpressions(binaryExpression, possibleInstanceofExpression); + if (variable == null || checkRef && isVariableUsed(operands, i, j, variable)) { + return false; + } + registerError(binaryExpression, binaryExpression); + return true; + } + + private static boolean isVariableUsed(PsiExpression[] operands, int i, int j, PsiVariable variable) { + i++; + while (i < j) { + if (VariableAccessUtils.variableIsUsed(variable, operands[i])) { + return true; + } + i++; + } + return false; + } + + public static PsiVariable checkExpressions(PsiBinaryExpression binaryExpression, PsiExpression possibleInstanceofExpression) { + final PsiReferenceExpression referenceExpression1 = getReferenceFromNullCheck(binaryExpression); + if (referenceExpression1 == null) { + return null; + } + final PsiElement target1 = referenceExpression1.resolve(); + if (!(target1 instanceof PsiVariable)) { + return null; + } + final PsiVariable variable = (PsiVariable)target1; + final PsiReferenceExpression referenceExpression2 = getReferenceFromInstanceofExpression(possibleInstanceofExpression); + if (referenceExpression2 == null || !referenceExpression2.isReferenceTo(variable)) { + return null; + } + return variable; } @Nullable @@ -222,8 +278,14 @@ public class PointlessNullCheckInspection extends BaseInspection { if (referenceExpression == null) { return null; } + final PsiElement target = referenceExpression.resolve(); + if (!(target instanceof PsiVariable)) { + return null; + } + final PsiVariable variable = (PsiVariable)target; for (int i = 1, operandsLength = operands.length; i < operandsLength; i++) { - if (!referencesEqual(referenceExpression, getReferenceFromInstanceofExpression(operands[i]))) { + final PsiReferenceExpression reference2 = getReferenceFromInstanceofExpression(operands[i]); + if (reference2 == null || !reference2.isReferenceTo(variable)) { return null; } } @@ -233,16 +295,4 @@ public class PointlessNullCheckInspection extends BaseInspection { } } } - - private static boolean referencesEqual(PsiReferenceExpression reference1, PsiReferenceExpression reference2) { - if (reference1 == null || reference2 == null) { - return false; - } - final PsiElement target1 = reference1.resolve(); - if (target1 == null) { - return false; - } - final PsiElement target2 = reference2.resolve(); - return target1.equals(target2); - } } 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 41251edb3493..fc2062da2981 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 @@ -33,6 +33,10 @@ public class PointlessNullCheck { if (arg != null && (arg instanceof String || arg instanceof Integer)) { System.out.println("this should trigger a warning"); } + + if (arg instanceof String && arg.equals(arg) && arg != null) { + System.out.println("warning"); + } } String arg1 = "foo"; @@ -64,5 +68,9 @@ public class PointlessNullCheck { if (this.arg1 != null && arg1 instanceof String) { System.out.println("this should not trigger a warning"); } + + if (arg1 != null && arg1.equals(arg1) && arg1 instanceof String) { + System.out.println("no warning"); + } } }