KT-897 Don't allow assignment to a property before it is defined

compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetControlFlowGraphTraverser.java

@@ -1,6 +1,5 @@
 package org.jetbrains.jet.lang.cfg;
 
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
@@ -23,24 +22,25 @@ public class JetControlFlowGraphTraverser<D> {
     private final boolean straightDirection;
     private final Map<Instruction, Pair<D, D>> dataMap = Maps.newLinkedHashMap();
 
-    public static <D> JetControlFlowGraphTraverser<D> create(Pseudocode pseudocode, boolean lookInside, boolean straightDirection) {
+    public static <D> JetControlFlowGraphTraverser<D> create(@NotNull Pseudocode pseudocode, boolean lookInside, boolean straightDirection) {
         return new JetControlFlowGraphTraverser<D>(pseudocode, lookInside, straightDirection);
     }
 
-    private JetControlFlowGraphTraverser(Pseudocode pseudocode, boolean lookInside, boolean straightDirection) {
+    private JetControlFlowGraphTraverser(@NotNull Pseudocode pseudocode, boolean lookInside, boolean straightDirection) {
         this.pseudocode = pseudocode;
         this.lookInside = lookInside;
         this.straightDirection = straightDirection;
     }
 
-    private Instruction getStartInstruction(Pseudocode pseudocode) {
+    @NotNull
+    private Instruction getStartInstruction(@NotNull Pseudocode pseudocode) {
         return straightDirection ? pseudocode.getEnterInstruction() : pseudocode.getSinkInstruction();
     }
 
     public void collectInformationFromInstructionGraph(
-            InstructionDataMergeStrategy<D> instructionDataMergeStrategy,
+            @NotNull D initialDataValue,
-            D initialDataValue,
+            @NotNull D initialDataValueForEnterInstruction,
-            D initialDataValueForEnterInstruction) {
+            @NotNull InstructionDataMergeStrategy<D> instructionDataMergeStrategy) {
         initializeDataMap(pseudocode, initialDataValue);
         dataMap.put(getStartInstruction(pseudocode),
                     Pair.create(initialDataValueForEnterInstruction, initialDataValueForEnterInstruction));
@@ -54,8 +54,8 @@ public class JetControlFlowGraphTraverser<D> {
     }
 
     private void initializeDataMap(
-            Pseudocode pseudocode,
+            @NotNull Pseudocode pseudocode,
-            D initialDataValue) {
+            @NotNull D initialDataValue) {
         List<Instruction> instructions = pseudocode.getInstructions();
         Pair<D, D> initialPair = Pair.create(initialDataValue, initialDataValue);
         for (Instruction instruction : instructions) {
@@ -67,9 +67,9 @@ public class JetControlFlowGraphTraverser<D> {
     }
 
     private void traverseSubGraph(
-            Pseudocode pseudocode,
+            @NotNull Pseudocode pseudocode,
-            InstructionDataMergeStrategy<D> instructionDataMergeStrategy,
+            @NotNull InstructionDataMergeStrategy<D> instructionDataMergeStrategy,
-            Collection<Instruction> previousSubGraphInstructions,
+            @NotNull Collection<Instruction> previousSubGraphInstructions,
             boolean[] changed,
             boolean isLocal) {
         List<Instruction> instructions = pseudocode.getInstructions();
@@ -127,13 +127,13 @@ public class JetControlFlowGraphTraverser<D> {
     }
 
     public void traverseAndAnalyzeInstructionGraph(
-            InstructionDataAnalyzeStrategy<D> instructionDataAnalyzeStrategy) {
+            @NotNull InstructionDataAnalyzeStrategy<D> instructionDataAnalyzeStrategy) {
         traverseAndAnalyzeInstructionGraph(pseudocode, instructionDataAnalyzeStrategy);
     }
     
     private void traverseAndAnalyzeInstructionGraph(
-            Pseudocode pseudocode,
+            @NotNull Pseudocode pseudocode,
-            InstructionDataAnalyzeStrategy<D> instructionDataAnalyzeStrategy) {
+            @NotNull InstructionDataAnalyzeStrategy<D> instructionDataAnalyzeStrategy) {
         List<Instruction> instructions = pseudocode.getInstructions();
         if (!straightDirection) {
             instructions = Lists.newArrayList(instructions);
@@ -156,10 +156,10 @@ public class JetControlFlowGraphTraverser<D> {
     }
     
     interface InstructionDataMergeStrategy<D> {
-        Pair<D, D> execute(Instruction instruction, @NotNull Collection<D> incomingEdgesData);
+        Pair<D, D> execute(@NotNull Instruction instruction, @NotNull Collection<D> incomingEdgesData);
     }
 
     interface InstructionDataAnalyzeStrategy<D> {
-        void execute(Instruction instruction, @Nullable D enterData, @Nullable D exitData);
+        void execute(@NotNull Instruction instruction, @Nullable D enterData, @Nullable D exitData);
     }
 }

compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetFlowInformationProvider.java

@@ -202,27 +202,25 @@ public class JetFlowInformationProvider {
         final Collection<VariableDescriptor> declaredVariables = collectDeclaredVariables(subroutine);
         Map<VariableDescriptor, VariableInitializers> initialMapForStartInstruction = prepareInitialMapForStartInstruction(usedVariables, declaredVariables);
 
-        JetControlFlowGraphTraverser.InstructionDataMergeStrategy<Map<VariableDescriptor, VariableInitializers>> variableInitializersMergeStrategy =
+        traverser.collectInformationFromInstructionGraph(Collections.<VariableDescriptor, VariableInitializers>emptyMap(), initialMapForStartInstruction, 
-                new JetControlFlowGraphTraverser.InstructionDataMergeStrategy<Map<VariableDescriptor, VariableInitializers>>() {
+                                                         new JetControlFlowGraphTraverser.InstructionDataMergeStrategy<Map<VariableDescriptor, VariableInitializers>>() {
             @Override
             public Pair<Map<VariableDescriptor, VariableInitializers>, Map<VariableDescriptor, VariableInitializers>> execute(
-                    Instruction instruction,
+                    @NotNull Instruction instruction,
                     @NotNull Collection<Map<VariableDescriptor, VariableInitializers>> incomingEdgesData) {
 
                 Map<VariableDescriptor, VariableInitializers> enterInstructionData = mergeIncomingEdgesData(incomingEdgesData);
                 Map<VariableDescriptor, VariableInitializers> exitInstructionData = addVariableInitializerFromCurrentInstructionIfAny(instruction, enterInstructionData);
                 return Pair.create(enterInstructionData, exitInstructionData);
             }
-        };
+        });
-
-        traverser.collectInformationFromInstructionGraph(variableInitializersMergeStrategy, Collections.<VariableDescriptor, VariableInitializers>emptyMap(), initialMapForStartInstruction);
 
         final Collection<VariableDescriptor> varWithUninitializedErrorGenerated = Sets.newHashSet();
         final Collection<VariableDescriptor> varWithValReassignErrorGenerated = Sets.newHashSet();
         final boolean processClassOrObject = subroutine instanceof JetClassOrObject;
         traverser.traverseAndAnalyzeInstructionGraph(new JetControlFlowGraphTraverser.InstructionDataAnalyzeStrategy<Map<VariableDescriptor, VariableInitializers>>() {
             @Override
-            public void execute(Instruction instruction, @Nullable Map<VariableDescriptor, VariableInitializers> enterData, @Nullable Map<VariableDescriptor, VariableInitializers> exitData) {
+            public void execute(@NotNull Instruction instruction, @Nullable Map<VariableDescriptor, VariableInitializers> enterData, @Nullable Map<VariableDescriptor, VariableInitializers> exitData) {
                 assert enterData != null && exitData != null;
                 VariableDescriptor variableDescriptor = extractVariableDescriptorIfAny(instruction, true);
                 if (variableDescriptor == null) return;
@@ -237,9 +235,12 @@ public class JetFlowInformationProvider {
                     JetElement element = ((WriteValueInstruction) instruction).getlValue();
                     boolean error = checkBackingField(variableDescriptor, element);
                     if (!(element instanceof JetExpression)) return;
-                    if (!error && !processLocalDeclaration) { // error has been generated before while processing outer function of this local declaration
+                    if (!error && !processLocalDeclaration) { // error has been generated before, while processing outer function of this local declaration
                         error = checkValReassignment(variableDescriptor, (JetExpression) element, enterData.get(variableDescriptor), varWithValReassignErrorGenerated);
                     }
+                    if (!error) {
+                        error = checkAssignmentBeforeDeclaration(variableDescriptor, (JetExpression) element, enterData.get(variableDescriptor), exitData.get(variableDescriptor));
+                    }
                     if (!error && processClassOrObject) {
                         checkInitializationUsingBackingField(variableDescriptor, (JetExpression) element, enterData.get(variableDescriptor), exitData.get(variableDescriptor));
                     }
@@ -312,13 +313,21 @@ public class JetFlowInformationProvider {
         return false;
     }
     
-    private void checkInitializationUsingBackingField(@NotNull VariableDescriptor variableDescriptor, @NotNull JetExpression expression, @NotNull VariableInitializers enterInitializers, @NotNull VariableInitializers exitInitializers) {
+    private boolean checkAssignmentBeforeDeclaration(@NotNull VariableDescriptor variableDescriptor, @NotNull JetExpression expression, @NotNull VariableInitializers enterInitializers, @NotNull VariableInitializers exitInitializers) {
+        if (!enterInitializers.isDeclared() && !exitInitializers.isDeclared() && !enterInitializers.isInitialized() && exitInitializers.isInitialized()) {
+            trace.report(Errors.INITIALIZATION_BEFORE_DECLARATION.on(expression, variableDescriptor));
+            return true;
+        }
+        return false;
+    }
+    
+    private boolean checkInitializationUsingBackingField(@NotNull VariableDescriptor variableDescriptor, @NotNull JetExpression expression, @NotNull VariableInitializers enterInitializers, @NotNull VariableInitializers exitInitializers) {
         if (variableDescriptor instanceof PropertyDescriptor && !enterInitializers.isInitialized() && exitInitializers.isInitialized()) {
-            if (!variableDescriptor.isVar()) return;
+            if (!variableDescriptor.isVar()) return false;
-            if (!trace.get(BindingContext.BACKING_FIELD_REQUIRED, (PropertyDescriptor) variableDescriptor)) return;
+            if (!trace.get(BindingContext.BACKING_FIELD_REQUIRED, (PropertyDescriptor) variableDescriptor)) return false;
             PsiElement property = trace.get(BindingContext.DESCRIPTOR_TO_DECLARATION, variableDescriptor);
             assert property instanceof JetProperty;
-            if (((PropertyDescriptor) variableDescriptor).getModality() == Modality.FINAL && ((JetProperty) property).getSetter() == null) return;
+            if (((PropertyDescriptor) variableDescriptor).getModality() == Modality.FINAL && ((JetProperty) property).getSetter() == null) return false;
             JetExpression variable = expression;
             if (expression instanceof JetDotQualifiedExpression) {
                 if (((JetDotQualifiedExpression) expression).getReceiverExpression() instanceof JetThisExpression) {
@@ -334,9 +343,11 @@ public class JetFlowInformationProvider {
                     else {
                         trace.report(Errors.INITIALIZATION_USING_BACKING_FIELD_CUSTOM_SETTER.on(simpleNameExpression, expression, variableDescriptor));
                     }
+                    return true;
                 }
             }
         }
+        return false;
     }
 
     private boolean checkBackingField(@NotNull VariableDescriptor variableDescriptor, @NotNull JetElement element) {
@@ -415,6 +426,20 @@ public class JetFlowInformationProvider {
             VariableInitializers initializationAtThisElement = new VariableInitializers(((WriteValueInstruction) instruction).getElement());
             exitInstructionData.put(variable, initializationAtThisElement);
         }
+        else if (instruction instanceof VariableDeclarationInstruction) {
+            VariableDescriptor variable = extractVariableDescriptorIfAny(instruction, false);
+            VariableInitializers enterInitializers = enterInstructionData.get(variable);
+            if (enterInitializers == null || !enterInitializers.isInitialized()) {
+                JetElement element = ((VariableDeclarationInstruction) instruction).getElement();
+                if (element instanceof JetProperty) {
+                    JetProperty property = (JetProperty) element;
+                    if (property.getInitializer() == null) {
+                        VariableInitializers variableDeclarationInfo = new VariableInitializers(false, true);
+                        exitInstructionData.put(variable, variableDeclarationInfo);
+                    }
+                }
+            }
+        }
         return exitInstructionData;
     }
 
@@ -469,7 +494,7 @@ public class JetFlowInformationProvider {
 
         JetControlFlowGraphTraverser.<Void>create(pseudocode, true, true).traverseAndAnalyzeInstructionGraph(new JetControlFlowGraphTraverser.InstructionDataAnalyzeStrategy<Void>() {
             @Override
-            public void execute(Instruction instruction, Void enterData, Void exitData) {
+            public void execute(@NotNull Instruction instruction, Void enterData, Void exitData) {
                 if (instruction instanceof ReadValueInstruction) {
                     VariableDescriptor variableDescriptor = extractVariableDescriptorIfAny(instruction, false);
                     if (variableDescriptor != null && functionVariables.contains(variableDescriptor)) {
@@ -493,9 +518,9 @@ public class JetFlowInformationProvider {
         JetControlFlowGraphTraverser<Map<VariableDescriptor, VariableStatus>> traverser = JetControlFlowGraphTraverser.create(pseudocode, true, false);
         final Collection<VariableDescriptor> declaredVariables = collectDeclaredVariables(subroutine);        
         Map<VariableDescriptor, VariableStatus> sinkInstructionData = Maps.newHashMap();
-        traverser.collectInformationFromInstructionGraph(new JetControlFlowGraphTraverser.InstructionDataMergeStrategy<Map<VariableDescriptor, VariableStatus>>() {
+        traverser.collectInformationFromInstructionGraph(Collections.<VariableDescriptor, VariableStatus>emptyMap(), sinkInstructionData, new JetControlFlowGraphTraverser.InstructionDataMergeStrategy<Map<VariableDescriptor, VariableStatus>>() {
             @Override
-            public Pair<Map<VariableDescriptor, VariableStatus>, Map<VariableDescriptor, VariableStatus>> execute(Instruction instruction, @NotNull Collection<Map<VariableDescriptor, VariableStatus>> incomingEdgesData) {
+            public Pair<Map<VariableDescriptor, VariableStatus>, Map<VariableDescriptor, VariableStatus>> execute(@NotNull Instruction instruction, @NotNull Collection<Map<VariableDescriptor, VariableStatus>> incomingEdgesData) {
                 Map<VariableDescriptor, VariableStatus> enterResult = Maps.newHashMap();
                 for (Map<VariableDescriptor, VariableStatus> edgeData : incomingEdgesData) {
                     for (Map.Entry<VariableDescriptor, VariableStatus> entry : edgeData.entrySet()) {
@@ -526,10 +551,10 @@ public class JetFlowInformationProvider {
                 }
                 return new Pair<Map<VariableDescriptor, VariableStatus>, Map<VariableDescriptor, VariableStatus>>(enterResult, exitResult);
             }
-        }, Collections.<VariableDescriptor, VariableStatus>emptyMap(), sinkInstructionData);
+        });
         traverser.traverseAndAnalyzeInstructionGraph(new JetControlFlowGraphTraverser.InstructionDataAnalyzeStrategy<Map<VariableDescriptor, VariableStatus>>() {
             @Override
-            public void execute(Instruction instruction, @Nullable Map<VariableDescriptor, VariableStatus> enterData, @Nullable Map<VariableDescriptor, VariableStatus> exitData) {
+            public void execute(@NotNull Instruction instruction, @Nullable Map<VariableDescriptor, VariableStatus> enterData, @Nullable Map<VariableDescriptor, VariableStatus> exitData) {
                 assert enterData != null && exitData != null;
                 VariableDescriptor variableDescriptor = extractVariableDescriptorIfAny(instruction, false);
                 if (variableDescriptor == null || !declaredVariables.contains(variableDescriptor) ||
@@ -630,7 +655,7 @@ public class JetFlowInformationProvider {
         final Set<VariableDescriptor> usedVariables = Sets.newHashSet();
         JetControlFlowGraphTraverser.<Void>create(pseudocode, true, true).traverseAndAnalyzeInstructionGraph(new JetControlFlowGraphTraverser.InstructionDataAnalyzeStrategy<Void>() {
             @Override
-            public void execute(Instruction instruction, Void enterData, Void exitData) {
+            public void execute(@NotNull Instruction instruction, Void enterData, Void exitData) {
                 VariableDescriptor variableDescriptor = extractVariableDescriptorIfAny(instruction, false);
                 if (variableDescriptor != null) {
                     usedVariables.add(variableDescriptor);
@@ -647,7 +672,7 @@ public class JetFlowInformationProvider {
         final Set<VariableDescriptor> declaredVariables = Sets.newHashSet();
         JetControlFlowGraphTraverser.<Void>create(pseudocode, false, true).traverseAndAnalyzeInstructionGraph(new JetControlFlowGraphTraverser.InstructionDataAnalyzeStrategy<Void>() {
             @Override
-            public void execute(Instruction instruction, @Nullable Void enterData, @Nullable Void exitData) {
+            public void execute(@NotNull Instruction instruction, @Nullable Void enterData, @Nullable Void exitData) {
                 if (instruction instanceof VariableDeclarationInstruction) {
                     JetDeclaration variableDeclarationElement = ((VariableDeclarationInstruction) instruction).getVariableDeclarationElement();
                     DeclarationDescriptor descriptor = trace.get(BindingContext.DECLARATION_TO_DESCRIPTOR, variableDeclarationElement);
@@ -667,22 +692,33 @@ public class JetFlowInformationProvider {
     private static class VariableInitializers {
         private Set<JetElement> possibleLocalInitializers = Sets.newHashSet();
         private boolean isInitialized;
+        private boolean isDeclared;
 
         public VariableInitializers(boolean isInitialized) {
+            this(isInitialized, false);
+        }
+        
+        public VariableInitializers(boolean isInitialized, boolean isDeclared) {
             this.isInitialized = isInitialized;
+            this.isDeclared = isDeclared;
         }
 
         public VariableInitializers(JetElement element) {
             isInitialized = true;
+            isDeclared = element instanceof JetProperty;
             possibleLocalInitializers.add(element);
         }
         
         public VariableInitializers(Set<VariableInitializers> edgesData) {
             isInitialized = true;
+            isDeclared = true;
             for (VariableInitializers edgeData : edgesData) {
                 if (!edgeData.isInitialized) {
                     isInitialized = false;
                 }
+                if (!edgeData.isDeclared) {
+                    isDeclared = false;
+                }
                 possibleLocalInitializers.addAll(edgeData.possibleLocalInitializers);
             }
         }
@@ -695,6 +731,10 @@ public class JetFlowInformationProvider {
             return isInitialized;
         }
 
+        public boolean isDeclared() {
+            return isDeclared;
+        }
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;

compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java

@@ -173,6 +173,7 @@ public interface Errors {
     };
 
     PsiElementOnlyDiagnosticFactory1<JetExpression, DeclarationDescriptor> VAL_REASSIGNMENT = PsiElementOnlyDiagnosticFactory1.create(ERROR, "Val can not be reassigned", NAME);
+    PsiElementOnlyDiagnosticFactory1<JetExpression, DeclarationDescriptor> INITIALIZATION_BEFORE_DECLARATION = PsiElementOnlyDiagnosticFactory1.create(ERROR, "Variable cannot be initialized before declaration", NAME);
     SimplePsiElementOnlyDiagnosticFactory<JetExpression> VARIABLE_EXPECTED = new SimplePsiElementOnlyDiagnosticFactory<JetExpression>(ERROR, "Variable expected");
 
     PsiElementOnlyDiagnosticFactory1<JetSimpleNameExpression, DeclarationDescriptor> INITIALIZATION_USING_BACKING_FIELD_CUSTOM_SETTER = PsiElementOnlyDiagnosticFactory1.create(ERROR, "This property has a custom setter, so initialization using backing field required", NAME);

compiler/testData/codegen/regressions/kt640.jet

@@ -1,15 +1,15 @@
 package demo
 
 public open class Identifier<T>(myName : T?, myHasDollar : Boolean) {
+    private val myName : T?
+    private var myHasDollar : Boolean
+    private var myNullable : Boolean = true
+
     {
         $myName = myName
         $myHasDollar = myHasDollar
     }
 
-    private val myName : T?
-    private var myHasDollar : Boolean
-    private var myNullable : Boolean = true
-
     open public fun getName() : T? {
         return myName
     }

compiler/testData/diagnostics/tests/controlFlowAnalysis/kt897.jet

@@ -0,0 +1,20 @@
+//KT-897 Don't allow assignment to a property before it is defined
+
+package kt897
+
+class A() {
+    {
+        <!INITIALIZATION_BEFORE_DECLARATION!>i<!> = 11
+    }
+    val i : Int? = null // must be an error
+
+    {
+        <!INITIALIZATION_BEFORE_DECLARATION!>j<!> = 1
+    }
+    var j : Int = 2
+
+    {
+        <!INITIALIZATION_BEFORE_DECLARATION!>k<!> = 3
+    }
+    val k : Int
+}

testlib/test/test/collections/TestAll.java

@@ -5,9 +5,9 @@ import junit.framework.TestSuite;
 /**
  */
 public class TestAll {
-  public static TestSuite suite() {
+//  public static TestSuite suite() {
-    TestSuite suite = new TestSuite(StandardCollectionTest.class, CollectionTest.class, IoTest.class, ListTest.class, MapTest.class, SetTest.class, OldStdlibTest.class);
+//    TestSuite suite = new TestSuite(StandardCollectionTest.class, CollectionTest.class, IoTest.class, ListTest.class, MapTest.class, SetTest.class, OldStdlibTest.class);
-    suite.addTest(testDslExample.namespace.getSuite());
+//    suite.addTest(testDslExample.namespace.getSuite());
-    return suite;
+//    return suite;
-  }
+//  }
 }