build constructor dfa to check final field not-nullability (IDEA-114828)

This commit is contained in:
peter
2013-10-17 21:54:34 +02:00
parent f9b7944bdb
commit 99e97f3207
12 changed files with 159 additions and 37 deletions
@@ -151,7 +151,7 @@ public class GuessManagerImpl extends GuessManager {
scope = file;
}
DataFlowRunner runner = new DataFlowRunner() {
DataFlowRunner runner = new DataFlowRunner(scope) {
@Override
protected DfaMemoryState createMemoryState() {
return new ExpressionTypeMemoryState(getFactory());
@@ -148,7 +148,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
PsiClass containingClass = PsiTreeUtil.getParentOfType(scope, PsiClass.class);
if (containingClass != null && PsiUtil.isLocalOrAnonymousClass(containingClass)) return;
final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner() {
final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner(scope) {
@Override
protected boolean shouldCheckTimeLimit() {
if (!onTheFly) return false;
@@ -30,6 +30,7 @@ import com.intellij.codeInspection.dataFlow.value.DfaVariableValue;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.openapi.util.Condition;
import com.intellij.openapi.util.Key;
import com.intellij.openapi.util.Pair;
import com.intellij.psi.*;
@@ -51,13 +52,20 @@ public class DataFlowRunner {
private Instruction[] myInstructions;
private final MultiMap<PsiElement, DfaMemoryState> myNestedClosures = new MultiMap<PsiElement, DfaMemoryState>();
private DfaVariableValue[] myFields;
private final DfaValueFactory myValueFactory = new DfaValueFactory();
private final DfaValueFactory myValueFactory;
// Maximum allowed attempts to process instruction. Fail as too complex to process if certain instruction
// is executed more than this limit times.
public static final int MAX_STATES_PER_BRANCH = 300;
protected DataFlowRunner() {
protected DataFlowRunner(PsiElement block) {
PsiElement parentConstructor = PsiTreeUtil.findFirstParent(block, new Condition<PsiElement>() {
@Override
public boolean value(PsiElement psiElement) {
return psiElement instanceof PsiMethod && ((PsiMethod)psiElement).isConstructor();
}
});
myValueFactory = new DfaValueFactory(parentConstructor == null);
}
public DfaValueFactory getFactory() {
@@ -207,7 +215,7 @@ public class DataFlowRunner {
return !ApplicationManager.getApplication().isUnitTestMode();
}
private DfaInstructionState[] acceptInstruction(InstructionVisitor visitor, DfaInstructionState instructionState) {
protected DfaInstructionState[] acceptInstruction(InstructionVisitor visitor, DfaInstructionState instructionState) {
Instruction instruction = instructionState.getInstruction();
if (instruction instanceof MethodCallInstruction) {
PsiCallExpression anchor = ((MethodCallInstruction)instruction).getCallExpression();
@@ -16,6 +16,8 @@
package com.intellij.codeInspection.dataFlow;
import com.intellij.codeInsight.NullableNotNullManager;
import com.intellij.codeInspection.dataFlow.instructions.Instruction;
import com.intellij.codeInspection.dataFlow.instructions.ReturnInstruction;
import com.intellij.openapi.util.Ref;
import com.intellij.psi.*;
import com.intellij.psi.search.LocalSearchScope;
@@ -23,6 +25,7 @@ import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.CachedValueProvider;
import com.intellij.psi.util.CachedValuesManager;
import com.intellij.psi.util.PsiModificationTracker;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.NullableFunction;
import com.intellij.util.containers.ContainerUtil;
@@ -31,10 +34,7 @@ import com.intellij.util.containers.Stack;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.*;
public class DfaPsiUtil {
@@ -93,6 +93,70 @@ public class DfaPsiUtil {
return Nullness.UNKNOWN;
}
public static boolean isInitializedNotNull(PsiField field) {
PsiClass containingClass = field.getContainingClass();
if (containingClass == null) return false;
PsiMethod[] constructors = containingClass.getConstructors();
if (constructors.length == 0) return false;
for (PsiMethod method : constructors) {
if (!getNotNullInitializedFields(method, containingClass).contains(field)) {
return false;
}
}
return true;
}
private static Set<PsiField> getNotNullInitializedFields(final PsiMethod constructor, PsiClass containingClass) {
final PsiCodeBlock body = constructor.getBody();
if (body == null) return Collections.emptySet();
final PsiField[] fields = containingClass.getFields();
return CachedValuesManager.getCachedValue(constructor, new CachedValueProvider<Set<PsiField>>() {
@Nullable
@Override
public Result<Set<PsiField>> compute() {
final Map<PsiField, Boolean> map = ContainerUtil.newHashMap();
final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner(body) {
boolean shouldCheck;
@Override
protected void prepareAnalysis(@NotNull PsiElement psiBlock, Iterable<DfaMemoryState> initialStates) {
super.prepareAnalysis(psiBlock, initialStates);
shouldCheck = psiBlock == body;
}
@Override
protected DfaInstructionState[] acceptInstruction(InstructionVisitor visitor, DfaInstructionState instructionState) {
if (shouldCheck) {
Instruction instruction = instructionState.getInstruction();
if (instruction instanceof ReturnInstruction && !((ReturnInstruction)instruction).isViaException()) {
for (PsiField field : fields) {
if (!instructionState.getMemoryState().isNotNull(getFactory().getVarFactory().createVariableValue(field, false))) {
map.put(field, false);
} else if (!map.containsKey(field)) {
map.put(field, true);
}
}
}
}
return super.acceptInstruction(visitor, instructionState);
}
};
final RunnerResult rc = dfaRunner.analyzeMethod(body, new StandardInstructionVisitor());
Set<PsiField> notNullFields = ContainerUtil.newHashSet();
if (rc == RunnerResult.OK) {
for (PsiField field : map.keySet()) {
if (map.get(field)) {
notNullFields.add(field);
}
}
}
return Result.create(notNullFields, constructor, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT);
}
});
}
public static List<PsiExpression> findAllConstructorInitializers(PsiField field) {
final List<PsiExpression> result = ContainerUtil.createLockFreeCopyOnWriteList();
ContainerUtil.addIfNotNull(result, field.getInitializer());
@@ -62,7 +62,7 @@ public class DfaUtil {
}
else {
final ValuableInstructionVisitor visitor = new ValuableInstructionVisitor(context);
RunnerResult runnerResult = new ValuableDataFlowRunner().analyzeMethod(codeBlock, visitor);
RunnerResult runnerResult = new ValuableDataFlowRunner(codeBlock).analyzeMethod(codeBlock, visitor);
if (runnerResult == RunnerResult.OK) {
result = visitor.myValues;
}
@@ -90,7 +90,7 @@ public class DfaUtil {
return Nullness.UNKNOWN;
}
final ValuableInstructionVisitor visitor = new ValuableInstructionVisitor(context);
RunnerResult result = new ValuableDataFlowRunner().analyzeMethod(codeBlock, visitor);
RunnerResult result = new ValuableDataFlowRunner(codeBlock).analyzeMethod(codeBlock, visitor);
if (result != RunnerResult.OK) {
return Nullness.UNKNOWN;
}
@@ -27,7 +27,6 @@ package com.intellij.codeInspection.dataFlow;
import com.intellij.codeInsight.NullableNotNullManager;
import com.intellij.codeInspection.dataFlow.instructions.InstanceofInstruction;
import com.intellij.codeInspection.dataFlow.instructions.Instruction;
import com.intellij.openapi.util.Pair;
import com.intellij.psi.CommonClassNames;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiMethod;
@@ -44,6 +43,10 @@ public class StandardDataFlowRunner extends DataFlowRunner {
private boolean myInNotNullMethod = false;
private boolean myIsInMethod = false;
public StandardDataFlowRunner(PsiElement block) {
super(block);
}
@Override
protected void prepareAnalysis(@NotNull PsiElement psiBlock, Iterable<DfaMemoryState> initialStates) {
myIsInMethod = psiBlock.getParent() instanceof PsiMethod;
@@ -78,14 +81,6 @@ public class StandardDataFlowRunner extends DataFlowRunner {
return myIsInMethod;
}
public boolean problemsDetected(StandardInstructionVisitor visitor) {
final Pair<Set<Instruction>, Set<Instruction>> constConditions = getConstConditionalExpressions();
return !constConditions.getFirst().isEmpty()
|| !constConditions.getSecond().isEmpty()
|| !myCCEInstructions.isEmpty()
|| !getRedundantInstanceofs(this, visitor).isEmpty();
}
@NotNull public static Set<Instruction> getRedundantInstanceofs(final DataFlowRunner runner, StandardInstructionVisitor visitor) {
HashSet<Instruction> result = new HashSet<Instruction>(1);
for (Instruction instruction : runner.getInstructions()) {
@@ -20,6 +20,7 @@ import com.intellij.codeInspection.dataFlow.value.DfaPsiType;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
import com.intellij.codeInspection.dataFlow.value.DfaValueFactory;
import com.intellij.codeInspection.dataFlow.value.DfaVariableValue;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiExpression;
import org.jetbrains.annotations.Nullable;
@@ -30,6 +31,10 @@ import java.util.Set;
*/
public class ValuableDataFlowRunner extends DataFlowRunner {
protected ValuableDataFlowRunner(PsiElement block) {
super(block);
}
@Override
protected DfaMemoryState createMemoryState() {
return new MyDfaMemoryState(getFactory());
@@ -42,8 +42,10 @@ public class DfaValueFactory {
private final Map<Pair<DfaPsiType, DfaPsiType>, Boolean> myAssignableCache = ContainerUtil.newHashMap();
private final Map<Pair<DfaPsiType, DfaPsiType>, Boolean> myConvertibleCache = ContainerUtil.newHashMap();
private final Map<PsiType, DfaPsiType> myDfaTypes = ContainerUtil.newHashMap();
private final boolean myHonorFieldInitializers;
public DfaValueFactory() {
public DfaValueFactory(boolean honorFieldInitializers) {
myHonorFieldInitializers = honorFieldInitializers;
myValues.add(null);
myVarFactory = new DfaVariableValue.Factory(this);
myConstFactory = new DfaConstValue.Factory(this);
@@ -52,6 +54,10 @@ public class DfaValueFactory {
myRelationFactory = new DfaRelationValue.Factory(this);
}
public boolean isHonorFieldInitializers() {
return myHonorFieldInitializers;
}
public DfaValue createTypeValue(@Nullable PsiType type, Nullness nullability) {
if (type == null) return DfaUnknownValue.getInstance();
return getTypeFactory().createTypeValue(internType(type), nullability);
@@ -159,7 +159,7 @@ public class DfaVariableValue extends DfaValue {
return nullability;
}
if (var instanceof PsiField && DfaPsiUtil.isFinalField((PsiVariable)var)) {
if (var instanceof PsiField && DfaPsiUtil.isFinalField((PsiVariable)var) && myFactory.isHonorFieldInitializers()) {
List<PsiExpression> initializers = DfaPsiUtil.findAllConstructorInitializers((PsiField)var);
if (initializers.isEmpty()) {
return Nullness.UNKNOWN;
@@ -168,11 +168,13 @@ public class DfaVariableValue extends DfaValue {
boolean hasUnknowns = false;
for (PsiExpression expression : initializers) {
if (!(expression instanceof PsiReferenceExpression)) {
return Nullness.UNKNOWN;
hasUnknowns = true;
continue;
}
PsiElement target = ((PsiReferenceExpression)expression).resolve();
if (!(target instanceof PsiParameter)) {
return Nullness.UNKNOWN;
hasUnknowns = true;
continue;
}
if (NullableNotNullManager.isNullable((PsiParameter)target)) {
return Nullness.NULLABLE;
@@ -181,7 +183,15 @@ public class DfaVariableValue extends DfaValue {
hasUnknowns = true;
}
}
return hasUnknowns ? Nullness.UNKNOWN : Nullness.NOT_NULL;
if (hasUnknowns) {
if (DfaPsiUtil.isInitializedNotNull((PsiField)var)) {
return Nullness.NOT_NULL;
}
return Nullness.UNKNOWN;
}
return Nullness.NOT_NULL;
}
return Nullness.UNKNOWN;
@@ -40,14 +40,16 @@ import com.intellij.openapi.editor.markup.TextAttributes;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Comparing;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.util.Pass;
import com.intellij.openapi.util.TextRange;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.openapi.wm.WindowManager;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.codeStyle.CodeStyleSettingsManager;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.codeStyle.VariableKind;
import com.intellij.psi.controlFlow.ControlFlowUtil;
import com.intellij.psi.impl.source.codeStyle.JavaCodeStyleManagerImpl;
import com.intellij.psi.search.GlobalSearchScope;
@@ -327,7 +329,6 @@ public class ExtractMethodProcessor implements MatchProvider {
}
private boolean isNotNull(PsiVariable outputVariable) {
final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner();
final PsiCodeBlock block = myElementFactory.createCodeBlock();
for (PsiElement element : myElements) {
block.add(element);
@@ -335,18 +336,15 @@ public class ExtractMethodProcessor implements MatchProvider {
final PsiIfStatement statementFromText = (PsiIfStatement)myElementFactory.createStatementFromText("if (" + outputVariable.getName() + " == null);", null);
block.add(statementFromText);
final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner(block);
final StandardInstructionVisitor visitor = new StandardInstructionVisitor();
final RunnerResult rc = dfaRunner.analyzeMethod(block, visitor);
if (rc == RunnerResult.OK) {
if (dfaRunner.problemsDetected(visitor)) {
final Pair<Set<Instruction>,Set<Instruction>>
conditionalExpressions = dfaRunner.getConstConditionalExpressions();
final Set<Instruction> falseSet = conditionalExpressions.getSecond();
for (Instruction instruction : falseSet) {
if (instruction instanceof BranchingInstruction) {
if (((BranchingInstruction)instruction).getPsiAnchor().getText().equals(statementFromText.getCondition().getText())) {
return true;
}
final Set<Instruction> falseSet = dfaRunner.getConstConditionalExpressions().getSecond();
for (Instruction instruction : falseSet) {
if (instruction instanceof BranchingInstruction) {
if (((BranchingInstruction)instruction).getPsiAnchor().getText().equals(statementFromText.getCondition().getText())) {
return true;
}
}
}
@@ -0,0 +1,35 @@
import java.lang.IllegalArgumentException;
import java.lang.Object;
public class Doo {
private final Object myA;
private final Object myB;
private final Object myC;
public Doo(Object myA, Object myB, Object c) {
if (myB == null) {
// assert myA != null;
throw new IllegalArgumentException();
}
assert c != null;
this.myA = myA;
this.myB = myB;
myC = c;
}
int bar() {
return myC.hashCode();
}
int foo() {
if (<warning descr="Condition 'myB == null' is always 'false'">myB == null</warning>) {
return 2;
}
if (<warning descr="Condition 'myC != null' is always 'true'">myC != null</warning>) {
return 3;
}
return myA.hashCode();
}
}
@@ -91,6 +91,7 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase {
public void testNotNullPrimitive() throws Throwable { doTest(); }
public void testBoxing128() throws Throwable { doTest(); }
public void testFinalFieldsInitializedByAnnotatedParameters() throws Throwable { doTest(); }
public void testFinalFieldsInitializedNotNull() throws Throwable { doTest(); }
public void testMultiCatch() throws Throwable { doTest(); }
public void testContinueFlushesLoopVariable() throws Throwable { doTest(); }