Initialize constructor entry memory state to initializer final memory state

Fixes IDEA-186422 Red code green DFA: reference on non-initialized field

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java

@@ -17,6 +17,7 @@ package com.intellij.codeInspection.dataFlow;
 
 import com.intellij.codeInsight.AnnotationUtil;
 import com.intellij.codeInsight.ExceptionUtil;
+import com.intellij.codeInsight.daemon.impl.UnusedSymbolUtil;
 import com.intellij.codeInspection.dataFlow.inliner.*;
 import com.intellij.codeInspection.dataFlow.instructions.*;
 import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet;
@@ -79,17 +80,14 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
   }
 
   private void buildClassInitializerFlow(PsiClass psiClass, boolean isStatic) {
-    pushUnknown();
-    ConditionalGotoInstruction conditionalGoto = new ConditionalGotoInstruction(null, false, null);
-    addInstruction(conditionalGoto);
     for (PsiElement element : psiClass.getChildren()) {
       if ((element instanceof PsiField || element instanceof PsiClassInitializer) &&
           ((PsiModifierListOwner)element).hasModifierProperty(PsiModifier.STATIC) == isStatic) {
         element.accept(this);
       }
     }
+    addInstruction(new EndOfInitializerInstruction(isStatic));
     addInstruction(new FlushVariableInstruction(null));
-    conditionalGoto.setOffset(getInstructionCount());
   }
 
   @Nullable
@@ -97,9 +95,16 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
     myCurrentFlow = new ControlFlow(myFactory);
     try {
       if(myCodeFragment instanceof PsiClass) {
-        // if(unknown) { staticInitializer(); } if(unknown) { instanceInitializer(); }
+        // if(unknown) { staticInitializer(); } else { instanceInitializer(); }
+        pushUnknown();
+        ConditionalGotoInstruction conditionalGoto = new ConditionalGotoInstruction(null, false, null);
+        addInstruction(conditionalGoto);
         buildClassInitializerFlow((PsiClass)myCodeFragment, true);
+        GotoInstruction unconditionalGoto = new GotoInstruction(null);
+        addInstruction(unconditionalGoto);
+        conditionalGoto.setOffset(getInstructionCount());
         buildClassInitializerFlow((PsiClass)myCodeFragment, false);
+        unconditionalGoto.setOffset(getInstructionCount());
       } else {
         myCodeFragment.accept(this);
       }
@@ -280,7 +285,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
     if (initializer != null) {
       initializeVariable(field, initializer);
     }
-    else if (!field.hasModifierProperty(PsiModifier.FINAL)) {
+    else if (!field.hasModifierProperty(PsiModifier.FINAL) && !UnusedSymbolUtil.isImplicitWrite(field)) {
       // initialize with default value
       DfaVariableValue dfaVariable = myFactory.getVarFactory().createVariableValue(field);
       addInstruction(new PushInstruction(dfaVariable, null, true));

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java

@@ -24,6 +24,7 @@ import com.intellij.openapi.util.Pair;
 import com.intellij.openapi.util.WriteExternalException;
 import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.psi.*;
+import com.intellij.psi.impl.source.resolve.JavaResolveUtil;
 import com.intellij.psi.tree.IElementType;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiTypesUtil;
@@ -88,12 +89,46 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
       @Override
       public void visitClass(PsiClass aClass) {
         if (aClass instanceof PsiTypeParameter) return;
-        analyzeCodeBlock(aClass, holder);
+        if (PsiUtil.isLocalOrAnonymousClass(aClass) && !(aClass instanceof PsiEnumConstantInitializer)) return;
+
+        final StandardDataFlowRunner runner = new StandardDataFlowRunner(TREAT_UNKNOWN_MEMBERS_AS_NULLABLE, aClass);
+        DataFlowInstructionVisitor visitor =
+          analyzeDfaWithNestedClosures(aClass, holder, runner, Collections.singletonList(runner.createMemoryState()));
+        List<DfaMemoryState> states = visitor.getEndOfInitializerStates();
+        for (PsiMethod method : aClass.getConstructors()) {
+          List<DfaMemoryState> initialStates;
+          PsiMethodCallExpression call = ConstructorUtil.findThisOrSuperCallInConstructor(method);
+          if (ConstructorUtil.isChainedConstructorCall(call) || (call == null && hasImplicitImpureSuperCall(aClass, method))) {
+            initialStates = Collections.singletonList(runner.createMemoryState());
+          } else {
+            initialStates = StreamEx.of(states).map(DfaMemoryState::createCopy).toList();
+          }
+          analyzeMethod(method, runner, initialStates);
+        }
+      }
+
+      private boolean hasImplicitImpureSuperCall(PsiClass aClass, PsiMethod constructor) {
+        PsiClass superClass = aClass.getSuperClass();
+        if (superClass == null) return false;
+        PsiElement superCtor = JavaResolveUtil.resolveImaginarySuperCallInThisPlace(constructor, constructor.getProject(), superClass);
+        if (!(superCtor instanceof PsiMethod)) return false;
+        return !ControlFlowAnalyzer.isPure((PsiMethod)superCtor);
       }
 
       @Override
       public void visitMethod(PsiMethod method) {
-        analyzeCodeBlock(method.getBody(), holder);
+        if (method.isConstructor()) return;
+        final StandardDataFlowRunner runner = new StandardDataFlowRunner(TREAT_UNKNOWN_MEMBERS_AS_NULLABLE, method.getBody());
+        analyzeMethod(method, runner, Collections.singletonList(runner.createMemoryState()));
+      }
+
+      private void analyzeMethod(PsiMethod method, StandardDataFlowRunner runner, List<DfaMemoryState> initialStates) {
+        PsiCodeBlock scope = method.getBody();
+        if (scope == null) return;
+        PsiClass containingClass = PsiTreeUtil.getParentOfType(method, PsiClass.class);
+        if (containingClass != null && PsiUtil.isLocalOrAnonymousClass(containingClass) && !(containingClass instanceof PsiEnumConstantInitializer)) return;
+
+        analyzeDfaWithNestedClosures(scope, holder, runner, initialStates);
         analyzeNullLiteralMethodArguments(method, holder);
       }
 
@@ -161,20 +196,10 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
     }
   }
 
-  private void analyzeCodeBlock(@Nullable final PsiElement scope, ProblemsHolder holder) {
-    if (scope == null) return;
-
-    PsiClass containingClass = PsiTreeUtil.getNonStrictParentOfType(scope, PsiClass.class);
-    if (containingClass != null && PsiUtil.isLocalOrAnonymousClass(containingClass) && !(containingClass instanceof PsiEnumConstantInitializer)) return;
-
-    final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner(TREAT_UNKNOWN_MEMBERS_AS_NULLABLE, scope);
-    analyzeDfaWithNestedClosures(scope, holder, dfaRunner, Collections.singletonList(dfaRunner.createMemoryState()));
-  }
-
-  private void analyzeDfaWithNestedClosures(PsiElement scope,
-                                            ProblemsHolder holder,
-                                            StandardDataFlowRunner dfaRunner,
-                                            Collection<? extends DfaMemoryState> initialStates) {
+  private DataFlowInstructionVisitor analyzeDfaWithNestedClosures(PsiElement scope,
+                                                                  ProblemsHolder holder,
+                                                                  StandardDataFlowRunner dfaRunner,
+                                                                  Collection<? extends DfaMemoryState> initialStates) {
     final DataFlowInstructionVisitor visitor = new DataFlowInstructionVisitor();
     final RunnerResult rc = dfaRunner.analyzeMethod(scope, visitor, IGNORE_ASSERT_STATEMENTS, initialStates);
     if (rc == RunnerResult.OK) {
@@ -195,6 +220,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
         holder.registerProblem(name, message, ProblemHighlightType.WEAK_WARNING);
       }
     }
+    return visitor;
   }
 
   @NotNull

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java

@@ -6,6 +6,8 @@ import com.intellij.codeInspection.dataFlow.value.*;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.util.Pair;
 import com.intellij.psi.*;
+import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.psi.util.PsiTypesUtil;
 import com.intellij.util.ObjectUtils;
 import com.intellij.util.ThreeState;
 import com.intellij.util.containers.ContainerUtil;
@@ -33,6 +35,7 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
   private final Set<PsiElement> myArgumentMutabilityViolation = new HashSet<>();
   private final Map<PsiExpression, Boolean> mySameValueAssigned = new HashMap<>();
   private boolean myAlwaysReturnsNotNull = true;
+  private final List<DfaMemoryState> myEndOfInitializerStates = new ArrayList<>();
 
   @Override
   public DfaInstructionState[] visitAssign(AssignInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
@@ -43,14 +46,16 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
           LOG.debug("Non-physical element in assignment instruction: " + left.getParent().getText(), new Throwable());
         }
       } else {
-        DfaValue dest = memState.peek();
+        DfaValue value = memState.peek();
         // Reporting of floating zero is skipped, because this produces false-positives on the code like
         // if(x == -0.0) x = 0.0;
-        if (dest instanceof DfaVariableValue || (dest instanceof DfaConstValue && !isFloatingZero(((DfaConstValue)dest).getValue()))) {
+        if (value instanceof DfaVariableValue || (value instanceof DfaConstValue && !isFloatingZero(((DfaConstValue)value).getValue()))) {
           DfaMemoryState copy = memState.createCopy();
           copy.pop();
-          DfaValue src = copy.peek();
-          boolean sameValue = !copy.applyCondition(runner.getFactory().createCondition(dest, DfaRelationValue.RelationType.NE, src));
+          DfaValue target = copy.peek();
+          boolean sameValue =
+            !isAssignmentToDefaultValueInConstructor(instruction, runner, target) &&
+            !copy.applyCondition(runner.getFactory().createCondition(value, DfaRelationValue.RelationType.NE, target));
           mySameValueAssigned.merge(left, sameValue, Boolean::logicalAnd);
         }
         else {
@@ -61,6 +66,29 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
     return super.visitAssign(instruction, runner, memState);
   }
 
+  private static boolean isAssignmentToDefaultValueInConstructor(AssignInstruction instruction, DataFlowRunner runner, DfaValue target) {
+    if (!(target instanceof DfaVariableValue)) return false;
+    DfaVariableValue var = (DfaVariableValue)target;
+    if (var.getQualifier() != null || !(var.getPsiVariable() instanceof PsiField)) return false;
+
+    // chained assignment like this.a = this.b = 0; is also supported
+    PsiExpression rExpression = instruction.getRExpression();
+    while (rExpression instanceof PsiAssignmentExpression &&
+           ((PsiAssignmentExpression)rExpression).getOperationTokenType().equals(JavaTokenType.EQ)) {
+      rExpression = ((PsiAssignmentExpression)rExpression).getRExpression();
+    }
+    if (rExpression == null) return false;
+    DfaValue dest = runner.getFactory().createValue(rExpression);
+    if (!(dest instanceof DfaConstValue)) return false;
+    Object value = ((DfaConstValue)dest).getValue();
+
+    PsiType type = var.getVariableType();
+    boolean isDefaultValue = Objects.equals(PsiTypesUtil.getDefaultValue(type), value) || Long.valueOf(0L).equals(value) && PsiType.INT.equals(type);
+    if (!isDefaultValue) return false;
+    PsiMethod method = PsiTreeUtil.getParentOfType(rExpression, PsiMethod.class);
+    return method != null && method.isConstructor();
+  }
+
   private static boolean isFloatingZero(Object value) {
     if (value instanceof Double) {
       return ((Double)value).doubleValue() == 0.0;
@@ -111,6 +139,10 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
     return receiver ? myReceiverMutabilityViolation : myArgumentMutabilityViolation;
   }
 
+  public List<DfaMemoryState> getEndOfInitializerStates() {
+    return myEndOfInitializerStates;
+  }
+
   Stream<PsiArrayAccessExpression> outOfBoundsArrayAccesses() {
     return StreamEx.ofKeys(myOutOfBoundsArrayAccesses, ThreeState.YES::equals);
   }
@@ -207,6 +239,14 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
     return super.visitPush(instruction, runner, memState);
   }
 
+  @Override
+  public DfaInstructionState[] visitEndOfInitializer(EndOfInitializerInstruction instruction, DataFlowRunner runner, DfaMemoryState state) {
+    if (!instruction.isStatic()) {
+      myEndOfInitializerStates.add(state.createCopy());
+    }
+    return super.visitEndOfInitializer(instruction, runner, state);
+  }
+
   public List<Pair<PsiReferenceExpression, DfaConstValue>> getConstantReferenceValues() {
     List<Pair<PsiReferenceExpression, DfaConstValue>> result = ContainerUtil.newArrayList();
     for (PushInstruction instruction : myPossibleVariableValues.keySet()) {

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java

@@ -76,6 +76,10 @@ public abstract class InstructionVisitor {
       .toArray(DfaInstructionState.EMPTY_ARRAY);
   }
 
+  public DfaInstructionState[] visitEndOfInitializer(EndOfInitializerInstruction instruction, DataFlowRunner runner, DfaMemoryState state) {
+    return nextInstruction(instruction, runner, state);
+  }
+
   protected static DfaInstructionState[] nextInstruction(Instruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
     return new DfaInstructionState[]{new DfaInstructionState(runner.getInstruction(instruction.getIndex() + 1), memState)};
   }

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/EndOfInitializerInstruction.java

@@ -0,0 +1,27 @@
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.intellij.codeInspection.dataFlow.instructions;
+
+import com.intellij.codeInspection.dataFlow.DataFlowRunner;
+import com.intellij.codeInspection.dataFlow.DfaInstructionState;
+import com.intellij.codeInspection.dataFlow.DfaMemoryState;
+import com.intellij.codeInspection.dataFlow.InstructionVisitor;
+
+/**
+ * Marks end of static or instance initializer
+ */
+public class EndOfInitializerInstruction extends Instruction {
+  private final boolean myStatic;
+
+  public EndOfInitializerInstruction(boolean isStatic) {
+    myStatic = isStatic;
+  }
+
+  public boolean isStatic() {
+    return myStatic;
+  }
+
+  @Override
+  public DfaInstructionState[] accept(DataFlowRunner runner, DfaMemoryState stateBefore, InstructionVisitor visitor) {
+    return visitor.visitEndOfInitializer(this, runner, stateBefore);
+  }
+}

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java

@@ -305,8 +305,8 @@ public class DfaValueFactory {
 
     FieldChecker(PsiElement context) {
       PsiMethod method = context instanceof PsiClass ? null : PsiTreeUtil.getParentOfType(context, PsiMethod.class);
-      myClass = method == null ? null : method.getContainingClass();
-      if (myClass == null) {
+      myClass = method != null ? method.getContainingClass() : context instanceof PsiClass ? (PsiClass)context : null;
+      if (method == null || myClass == null) {
         myTrustDirectFieldInitializers = myTrustFieldInitializersInConstructors = myCanInstantiateItself = false;
         return;
       }

java/java-tests/testData/inspection/dataFlow/fixture/MergedInitializerAndConstructor.java

@@ -0,0 +1,84 @@
+import java.util.*;
+import org.jetbrains.annotations.*;
+
+class MergedInitializerAndConstructor {
+  static class Test1 {
+    private Collection<Object> collection2 = null;
+
+    public Test1() {
+      collection2.<warning descr="Method invocation 'add' may produce 'java.lang.NullPointerException'">add</warning>("");
+    }
+  }
+
+  static class Test2 {
+    private Collection<Object> collection2 = null;
+
+    public Test2() {
+      collection2.add("");
+    }
+
+    {
+      collection2.<warning descr="Method invocation 'add' may produce 'java.lang.NullPointerException'">add</warning>(""); //<- warning here
+    }
+  }
+
+  static class Test3 {
+    private Collection<Object> collection2 = null;
+
+    public Test3(String s) {
+      super();
+      collection2.<warning descr="Method invocation 'add' may produce 'java.lang.NullPointerException'">add</warning>(s);
+    }
+  }
+
+  static class Test4 {
+    private Collection<Object> collection2 = null;
+
+    @Contract(pure = true)
+    public Test4() {
+      collection2 = new ArrayList<>();
+    }
+
+    public Test4(String s) {
+      this();
+      collection2.add(s);
+    }
+  }
+
+  static class Super {
+    Super() {
+      init();
+    }
+
+    void init() {}
+  }
+
+  static class Test5 extends Super {
+    private Collection<Object> collection2 = null;
+
+    public Test5() {
+      // implicit super initializes collection2
+      collection2.add("foo");
+    }
+
+    public Test5(String s) {
+      super();
+      collection2.add(s);
+    }
+
+    void init() {
+      collection2 = new ArrayList<>();
+    }
+  }
+
+  static class DoNotWarnOnDefaultInit {
+    private int x;
+    private @Nullable Object val;
+
+    DoNotWarnOnDefaultInit() {
+      // Do not issue "Variable is already assigned" here
+      x = 0;
+      val = null;
+    }
+  }
+}

java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java

@@ -591,4 +591,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
     myFixture.addClass("package org.junit; public class Assume { public static void assumeNotNull(Object... objects) {}}");
     doTest();
   }
+  public void testMergedInitializerAndConstructor() { doTest(); }
 }