From 201c7b55127cdf6069221eea938f2f2b3f8ba046 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 8 Nov 2013 17:27:01 +0100 Subject: [PATCH] IDEA-75717 ("Referenced checked for null not used inside if" false positive) --- .../siyeh/InspectionGadgetsBundle.properties | 3 +- .../VariableNotUsedInsideIfInspection.java | 45 +++++++++++-------- .../VariableNotUsedInsideIf.java | 25 +++++++++-- .../variable_not_used_inside_if/expected.xml | 21 +++++++++ 4 files changed, 72 insertions(+), 22 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index cc8ab962b254..36709b62b934 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -1731,7 +1731,8 @@ field.may.be.final.display.name=Field may be 'final' field.may.be.final.problem.descriptor=Field #ref may be 'final' #loc cast.that.loses.precision.option=Ignore casts from int to char variable.not.used.inside.if.display.name=Reference checked for 'null' is not used inside 'if' -variable.not.used.inside.if.problem.descriptor=#ref is not used inside if #loc +variable.not.used.inside.if.problem.descriptor=#ref checked for 'null' is not used inside 'if' #loc +variable.not.used.inside.conditional.problem.descriptor=#ref checked for 'null' is not used inside conditional #loc if.may.be.conditional.display.name='if' statement could be replaced with conditional expression if.may.be.conditional.problem.descriptor=#ref could be replaced with conditional expression #loc if.may.be.conditional.quickfix=Replace with conditional expression diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/VariableNotUsedInsideIfInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/VariableNotUsedInsideIfInspection.java index f6dc0e40fbc0..bae56e94b550 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/VariableNotUsedInsideIfInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/VariableNotUsedInsideIfInspection.java @@ -31,15 +31,19 @@ public class VariableNotUsedInsideIfInspection extends BaseInspection { @Nls @NotNull public String getDisplayName() { - return InspectionGadgetsBundle.message( - "variable.not.used.inside.if.display.name"); + return InspectionGadgetsBundle.message("variable.not.used.inside.if.display.name"); } @Override @NotNull protected String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "variable.not.used.inside.if.problem.descriptor"); + final boolean isIf = ((Boolean)infos[0]).booleanValue(); + if (isIf) { + return InspectionGadgetsBundle.message("variable.not.used.inside.if.problem.descriptor"); + } + else { + return InspectionGadgetsBundle.message("variable.not.used.inside.conditional.problem.descriptor"); + } } @Override @@ -63,10 +67,14 @@ public class VariableNotUsedInsideIfInspection extends BaseInspection { } final IElementType tokenType = binaryExpression.getOperationTokenType(); if (tokenType == JavaTokenType.EQEQ) { - checkVariableUsage(referenceExpression, expression.getThenExpression(), expression.getElseExpression()); + if (checkVariableUsage(referenceExpression, expression.getThenExpression(), expression.getElseExpression())) { + registerError(referenceExpression, Boolean.FALSE); + } } else if (tokenType == JavaTokenType.NE) { - checkVariableUsage(referenceExpression, expression.getElseExpression(), expression.getThenExpression()); + if (checkVariableUsage(referenceExpression, expression.getElseExpression(), expression.getThenExpression())) { + registerError(referenceExpression, Boolean.FALSE); + } } } @@ -84,29 +92,30 @@ public class VariableNotUsedInsideIfInspection extends BaseInspection { } final IElementType tokenType = binaryExpression.getOperationTokenType(); if (tokenType == JavaTokenType.EQEQ) { - checkVariableUsage(referenceExpression, statement.getThenBranch(), statement.getElseBranch()); + if (checkVariableUsage(referenceExpression, statement.getThenBranch(), statement.getElseBranch())) { + registerError(referenceExpression, Boolean.TRUE); + } } else if (tokenType == JavaTokenType.NE) { - checkVariableUsage(referenceExpression, statement.getElseBranch(), statement.getThenBranch()); + if (checkVariableUsage(referenceExpression, statement.getElseBranch(), statement.getThenBranch())) { + registerError(referenceExpression, Boolean.TRUE); + } } } - private void checkVariableUsage(PsiReferenceExpression referenceExpression, PsiElement thenContext, PsiElement elseContext) { - if (thenContext == null) { - return; - } + private boolean checkVariableUsage(PsiReferenceExpression referenceExpression, PsiElement thenContext, PsiElement elseContext) { final PsiElement target = referenceExpression.resolve(); if (!(target instanceof PsiVariable)) { - return; + return false; } final PsiVariable variable = (PsiVariable)target; - if (contextExits(thenContext) || VariableAccessUtils.variableIsAssigned(variable, thenContext)) { - return; + if (thenContext != null && (contextExits(thenContext) || VariableAccessUtils.variableIsAssigned(variable, thenContext))) { + return false; } - if (elseContext != null && (contextExits(elseContext) || VariableAccessUtils.variableIsUsed(variable, elseContext))) { - return; + if (elseContext == null || VariableAccessUtils.variableIsUsed(variable, elseContext)) { + return false; } - registerError(referenceExpression); + return true; } private static PsiReferenceExpression extractVariableReference(PsiBinaryExpression expression) { diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/variable_not_used_inside_if/VariableNotUsedInsideIf.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/variable_not_used_inside_if/VariableNotUsedInsideIf.java index e8dcfc55877e..c6d13025555d 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/variable_not_used_inside_if/VariableNotUsedInsideIf.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/variable_not_used_inside_if/VariableNotUsedInsideIf.java @@ -27,10 +27,10 @@ public class VariableNotUsedInsideIf { } void bat(String s) { - if (s == null) { + if (s != null) { System.out.println(); } - if (s != null) { + if (s == null) { } else { @@ -38,8 +38,27 @@ public class VariableNotUsedInsideIf { } void money(String s) { - if ((s == null)) { + if (((s) != (null))) { System.out.println(); } } + + void x(Integer x){ + if (x != null) { + System.out.println(); + } + } + + int x(Integer x, int y){ + if (x != null) return y;//oops, wrong one + return y; + } + + int conditional(Integer x) { + return x == null ? 1 : someValue(); + } + + private int someValue() { + return 0; + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/variable_not_used_inside_if/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/variable_not_used_inside_if/expected.xml index a6d23f928e87..603a54209ebd 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/variable_not_used_inside_if/expected.xml +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/variable_not_used_inside_if/expected.xml @@ -20,4 +20,25 @@ Reference checked for 'null' is not used inside 'if' <code>s</code> is not used inside if #loc + + + VariableNotUsedInsideIf.java + 47 + Reference checked for 'null' is not used inside 'if' + <code>x</code> checked for 'null' is not used inside 'if' #loc + + + + VariableNotUsedInsideIf.java + 53 + Reference checked for 'null' is not used inside 'if' + <code>x</code> checked for 'null' is not used inside 'if' #loc + + + + VariableNotUsedInsideIf.java + 58 + Reference checked for 'null' is not used inside 'if' + <code>x</code> checked for 'null' is not used inside conditional #loc + \ No newline at end of file