IDEA-160635 'Condition always true' inspection sometimes incorrect when inner class access final field

This commit is contained in:
peter
2016-09-07 16:25:08 +02:00
parent 5a84e9452d
commit e9e9dbfffe
4 changed files with 50 additions and 20 deletions
@@ -20,7 +20,6 @@ import com.intellij.codeInsight.NullableNotNullManager;
import com.intellij.codeInspection.dataFlow.instructions.Instruction;
import com.intellij.codeInspection.dataFlow.instructions.MethodCallInstruction;
import com.intellij.codeInspection.dataFlow.instructions.ReturnInstruction;
import com.intellij.codeInspection.dataFlow.value.DfaValueFactory;
import com.intellij.lang.java.JavaLanguage;
import com.intellij.openapi.util.Ref;
import com.intellij.patterns.PsiJavaPatterns;
@@ -32,6 +31,7 @@ import com.intellij.psi.util.*;
import com.intellij.util.ArrayUtil;
import com.intellij.util.NullableFunction;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.JBIterable;
import com.intellij.util.containers.MultiMap;
import com.intellij.util.containers.Stack;
import org.jetbrains.annotations.NotNull;
@@ -220,21 +220,34 @@ public class DfaPsiUtil {
PsiCall call = ((MethodCallInstruction)instruction).getCallExpression();
if (call == null) return false;
if (call instanceof PsiMethodCallExpression &&
DfaValueFactory.isEffectivelyUnqualified(((PsiMethodCallExpression)call).getMethodExpression())) {
if (call instanceof PsiNewExpression && canAccessFields((PsiExpression)call)) {
return true;
}
if (call instanceof PsiMethodCallExpression) {
PsiExpression qualifier = ((PsiMethodCallExpression)call).getMethodExpression().getQualifierExpression();
if (qualifier == null || canAccessFields(qualifier)) {
return true;
}
}
PsiExpressionList argumentList = call.getArgumentList();
if (argumentList != null) {
for (PsiExpression expression : argumentList.getExpressions()) {
if (expression instanceof PsiThisExpression) return true;
if (canAccessFields(expression)) return true;
}
}
return false;
}
private boolean canAccessFields(PsiExpression expression) {
PsiClass type = PsiUtil.resolveClassInClassTypeOnly(expression.getType());
JBIterable<PsiClass> typeContainers =
JBIterable.generate(type, PsiClass::getContainingClass).takeWhile(c -> !c.hasModifierProperty(PsiModifier.STATIC));
return typeContainers.contains(containingClass);
}
@NotNull
@Override
protected DfaInstructionState[] acceptInstruction(@NotNull InstructionVisitor visitor, @NotNull DfaInstructionState instructionState) {
@@ -189,30 +189,26 @@ public class DfaVariableValue extends DfaValue {
}
if (var instanceof PsiField && DfaPsiUtil.isFinalField((PsiVariable)var) && myFactory.isHonorFieldInitializers()) {
PsiExpression initializer = ((PsiField)var).getInitializer();
if (initializer != null) {
return getFieldInitializerNullness(initializer);
}
List<PsiExpression> initializers = DfaPsiUtil.findAllConstructorInitializers((PsiField)var);
if (initializers.isEmpty()) {
return defaultNullability;
}
boolean hasUnknowns = false;
for (PsiExpression expression : initializers) {
Nullness nullness = getFieldInitializerNullness(expression);
if (nullness == Nullness.NULLABLE) {
if (getFieldInitializerNullness(expression) == Nullness.NULLABLE) {
return Nullness.NULLABLE;
}
if (nullness == Nullness.UNKNOWN) {
hasUnknowns = true;
}
}
if (hasUnknowns) {
if (DfaPsiUtil.isInitializedNotNull((PsiField)var)) {
return Nullness.NOT_NULL;
}
return defaultNullability;
if (DfaPsiUtil.isInitializedNotNull((PsiField)var)) {
return Nullness.NOT_NULL;
}
return Nullness.NOT_NULL;
return defaultNullability;
}
return defaultNullability;
@@ -223,11 +219,11 @@ public class DfaVariableValue extends DfaValue {
if (expression instanceof PsiNewExpression || expression instanceof PsiLiteralExpression || expression instanceof PsiPolyadicExpression) return Nullness.NOT_NULL;
if (expression instanceof PsiReferenceExpression) {
PsiElement target = ((PsiReferenceExpression)expression).resolve();
return DfaPsiUtil.getElementNullability(null, (PsiModifierListOwner)target);
return DfaPsiUtil.getElementNullability(expression.getType(), (PsiModifierListOwner)target);
}
if (expression instanceof PsiMethodCallExpression) {
PsiMethod method = ((PsiMethodCallExpression)expression).resolveMethod();
return method != null ? DfaPsiUtil.getElementNullability(null, method) : Nullness.UNKNOWN;
return method != null ? DfaPsiUtil.getElementNullability(expression.getType(), method) : Nullness.UNKNOWN;
}
return Nullness.UNKNOWN;
}
@@ -0,0 +1,20 @@
class FailingNonNull {
private final String nullable;
public FailingNonNull() {
Inner inner = new Inner();
inner.doNullableStuff();
nullable = "now non-null";
}
private class Inner{
public void doNullableStuff() {
if (nullable !=null) { //Condition 'nullable!=null' is always 'true'
System.out.println(nullable.length());
}
}
}
public static void main(String[] args) {
new FailingNonNull();
}
}
@@ -193,6 +193,7 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
public void testRememberLocalTransientFieldState() { doTest(); }
public void testFinalFieldDuringInitialization() { doTest(); }
public void testFinalFieldDuringSuperInitialization() { doTest(); }
public void testFinalFieldInCallBeforeInitialization() { doTest(); }
public void testFinalFieldInConstructorAnonymous() { doTest(); }
public void testFinalFieldNotDuringInitialization() {