From 41c9b7e414da37ed7a08ded115a801f94d0c9f55 Mon Sep 17 00:00:00 2001 From: svtk Date: Tue, 1 Nov 2011 18:59:53 +0400 Subject: [PATCH] Added checks for an initialization of properties using backing fields, not using setters --- .../lang/cfg/JetFlowInformationProvider.java | 162 +++++++++--- .../jet/lang/diagnostics/Errors.java | 3 +- .../jet/lang/resolve/BodyResolver.java | 18 +- .../jet/lang/resolve/TopDownAnalyzer.java | 2 +- .../full/FlowAnalyseVariables.jet | 106 -------- .../UninitializedOrReassignedVariables.jet | 250 ++++++++++++++++++ 6 files changed, 375 insertions(+), 166 deletions(-) delete mode 100644 compiler/testData/checkerWithErrorTypes/full/FlowAnalyseVariables.jet create mode 100644 compiler/testData/checkerWithErrorTypes/full/UninitializedOrReassignedVariables.jet diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetFlowInformationProvider.java b/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetFlowInformationProvider.java index 87436be68f5..eb1686c2921 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetFlowInformationProvider.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/cfg/JetFlowInformationProvider.java @@ -3,18 +3,18 @@ package org.jetbrains.jet.lang.cfg; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import com.intellij.openapi.util.Pair; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.cfg.pseudocode.*; -import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor; -import org.jetbrains.jet.lang.descriptors.LocalVariableDescriptor; -import org.jetbrains.jet.lang.descriptors.VariableDescriptor; +import org.jetbrains.jet.lang.descriptors.*; import org.jetbrains.jet.lang.descriptors.VariableDescriptor; import org.jetbrains.jet.lang.diagnostics.Errors; import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.resolve.BindingContext; import org.jetbrains.jet.lang.resolve.BindingTrace; import org.jetbrains.jet.lang.types.JetStandardClasses; +import org.jetbrains.jet.lexer.JetTokens; import java.util.*; @@ -259,13 +259,16 @@ public class JetFlowInformationProvider { // } // } - public void markUninitializedVariables(@NotNull JetElement subroutine, List initializedVariables) { - Pseudocode pseudocode = pseudocodeMap.get(subroutine); + public void markUninitializedVariables( + @NotNull JetElement subroutine, + final boolean inAnonymousInitializers) { + final Pseudocode pseudocode = pseudocodeMap.get(subroutine); assert pseudocode != null; - JetControlFlowGraphTraverser.InstructionsMergeStrategy> instructionsMergeStrategy = new JetControlFlowGraphTraverser.InstructionsMergeStrategy>() { + JetControlFlowGraphTraverser.InstructionsMergeStrategy> instructionsMergeStrategy = + new JetControlFlowGraphTraverser.InstructionsMergeStrategy>() { @Override - public Map execute( + public Pair, Map> execute( Instruction instruction, Collection> incomingEdgesData) { @@ -274,7 +277,7 @@ public class JetFlowInformationProvider { variablesInScope.addAll(edgePointsMap.keySet()); } - Map pointsMap = Maps.newHashMap(); + Map enterInstructionPointsMap = Maps.newHashMap(); for (VariableDescriptor variable : variablesInScope) { Set edgesDataForVariable = Sets.newHashSet(); for (Map edgePointsMap : incomingEdgesData) { @@ -283,43 +286,64 @@ public class JetFlowInformationProvider { edgesDataForVariable.add(points); } } - pointsMap.put(variable, new InitializationPoints(edgesDataForVariable)); + enterInstructionPointsMap.put(variable, new InitializationPoints(edgesDataForVariable)); } + Map exitInstructionPointsMap = Maps.newHashMap(enterInstructionPointsMap); if (instruction instanceof WriteValueInstruction) { - VariableDescriptor variableDescriptor = extractVariableDescriptorIfAny(instruction); - InitializationPoints initializationAtThisPoint = new InitializationPoints(((WriteValueInstruction) instruction).getElement()); - pointsMap.put(variableDescriptor, initializationAtThisPoint); + VariableDescriptor variable = extractVariableDescriptorIfAny(instruction); + InitializationPoints initializationAtThisPoint = new InitializationPoints(((WriteValueInstruction) instruction).getElement()); + exitInstructionPointsMap.put(variable, initializationAtThisPoint); } - return pointsMap; + return Pair.create(enterInstructionPointsMap, exitInstructionPointsMap); } }; - Set localVariables = collectAllLocalVariables(pseudocode); + Collection usedVariables = collectUsedVariables(pseudocode); + Collection declaredVariables = collectDeclaredVariables(subroutine); + Map initialMapForStartInstruction = Maps.newHashMap(); - InitializationPoints initialPointsForLocalVariable = new InitializationPoints(true); - for (VariableDescriptor variable : localVariables) { - initialMapForStartInstruction.put(variable, initialPointsForLocalVariable); + InitializationPoints initialPointsForDeclaredVariable = new InitializationPoints(true); + InitializationPoints initialPointsForExternalVariable = new InitializationPoints(false); + + for (VariableDescriptor variable : usedVariables) { + if (declaredVariables.contains(variable)) { + initialMapForStartInstruction.put(variable, initialPointsForDeclaredVariable); + } + else { + initialMapForStartInstruction.put(variable, initialPointsForExternalVariable); + } } - InitializationPoints initialPointsForParameter = new InitializationPoints(false); - for (VariableDescriptor initializedVariable : initializedVariables) { - initialMapForStartInstruction.put(initializedVariable, initialPointsForParameter); - } - final Map> dataMap = - JetControlFlowGraphTraverser.traverseInstructionGraphUntilFactsStabilization(pseudocode, instructionsMergeStrategy, Collections.emptyMap(), initialMapForStartInstruction, true); + final Map, Map>> dataMap = + JetControlFlowGraphTraverser.traverseInstructionGraphUntilFactsStabilization(pseudocode, + instructionsMergeStrategy, + Collections.emptyMap(), + initialMapForStartInstruction, + true); JetControlFlowGraphTraverser.traverseAndAnalyzeInstructionGraph(pseudocode, new JetControlFlowGraphTraverser.InstructionDataAnalyzeStrategy() { @Override public void execute(Instruction instruction) { - Map variablesData = dataMap.get(instruction); + Pair, Map> variablesData = dataMap.get(instruction); + Map enterData = variablesData.getFirst(); + Map exitData = variablesData.getSecond(); if (instruction instanceof ReadValueInstruction) { VariableDescriptor variableDescriptor = extractVariableDescriptorIfAny(instruction); JetElement element = ((ReadValueInstruction) instruction).getElement(); - if (element instanceof JetSimpleNameExpression && variableDescriptor instanceof LocalVariableDescriptor) { - InitializationPoints initializationPoints = variablesData.get(variableDescriptor); - assert initializationPoints != null; - if (initializationPoints.canBeUninitialized) { + if (element instanceof JetSimpleNameExpression && variableDescriptor != null && + (inAnonymousInitializers || variableDescriptor instanceof LocalVariableDescriptor)) { + + InitializationPoints exitInitializationPoints = exitData.get(variableDescriptor); + assert exitInitializationPoints != null; + + boolean canBeUnitialized = exitInitializationPoints.canBeUninitialized(); + if (variableDescriptor instanceof PropertyDescriptor) { + if (!trace.get(BindingContext.BACKING_FIELD_REQUIRED, (PropertyDescriptor) variableDescriptor)) { + canBeUnitialized = false; + } + } + if (canBeUnitialized) { trace.report(Errors.UNINITIALIZED_VARIABLE.on((JetSimpleNameExpression) element, variableDescriptor)); } } @@ -328,15 +352,43 @@ public class JetFlowInformationProvider { JetElement element = ((WriteValueInstruction) instruction).getlValue(); VariableDescriptor variableDescriptor = extractVariableDescriptorIfAny(instruction); if (element instanceof JetSimpleNameExpression && variableDescriptor != null) { - InitializationPoints initializationPoints = variablesData.get(variableDescriptor); - assert initializationPoints != null; - if (initializationPoints.hasPossibleInitializers() && !variableDescriptor.isVar()) { - trace.report(Errors.VAL_REASSIGNMENT.on((JetSimpleNameExpression) element, variableDescriptor)); + InitializationPoints enterInitializationPoints = enterData.get(variableDescriptor); + assert enterInitializationPoints != null; + InitializationPoints exitInitializationPoints = exitData.get(variableDescriptor); + assert exitInitializationPoints != null; + Set possiblePoints = enterInitializationPoints.getPossiblePoints(); + boolean hasAnotherInitializer = !possiblePoints.isEmpty() || !enterInitializationPoints.canBeUninitialized; + if (possiblePoints.size() == 1) { + JetElement initializer = possiblePoints.iterator().next(); + if (initializer == element.getParent()) { + hasAnotherInitializer = false; + } + } + JetSimpleNameExpression expression = (JetSimpleNameExpression) element; + if (hasAnotherInitializer && !variableDescriptor.isVar()) { + trace.report(Errors.VAL_REASSIGNMENT.on(expression, variableDescriptor)); + } + if (inAnonymousInitializers && variableDescriptor instanceof PropertyDescriptor && + enterInitializationPoints.canBeUninitialized && !exitInitializationPoints.canBeUninitialized) { + if (expression.getReferencedNameElementType() != JetTokens.FIELD_IDENTIFIER) { + trace.report(Errors.INITIALIZATION_USING_BACKING_FIELD.on(expression, variableDescriptor)); + } } } } } }); + + if (inAnonymousInitializers) { + Map lastInfo = dataMap.get(pseudocode.getExitInstruction()).getFirst(); + for (Map.Entry entry : lastInfo.entrySet()) { + VariableDescriptor variable = entry.getKey(); + if (variable instanceof PropertyDescriptor) { + InitializationPoints initializationPoints = entry.getValue(); + trace.record(BindingContext.IS_INITIALIZED, (PropertyDescriptor) variable, !initializationPoints.canBeUninitialized); + } + } + } } public void markNotOnlyInvokedFunctionVariables(@NotNull JetElement subroutine, List variables) { @@ -395,19 +447,43 @@ public class JetFlowInformationProvider { return variableDescriptor; } - private Set collectAllLocalVariables(Pseudocode pseudocode) { - final Set localVariables = Sets.newHashSet(); - JetControlFlowGraphTraverser.InstructionDataAnalyzeStrategy analyzeStrategy = new JetControlFlowGraphTraverser.InstructionDataAnalyzeStrategy() { + private Collection collectUsedVariables(Pseudocode pseudocode) { + final Set usedVariables = Sets.newHashSet(); + JetControlFlowGraphTraverser.traverseAndAnalyzeInstructionGraph(pseudocode, new JetControlFlowGraphTraverser.InstructionDataAnalyzeStrategy() { @Override public void execute(Instruction instruction) { VariableDescriptor variableDescriptor = extractVariableDescriptorIfAny(instruction); if (variableDescriptor != null) { - localVariables.add(variableDescriptor); + usedVariables.add(variableDescriptor); } } - }; - JetControlFlowGraphTraverser.traverseAndAnalyzeInstructionGraph(pseudocode, analyzeStrategy); - return localVariables; + }); + return usedVariables; + } + + private Collection collectDeclaredVariables(JetElement element) { + final Set concernedVariables = Sets.newHashSet(); + element.accept(new JetTreeVisitor() { + @Override + public Void visitProperty(JetProperty property, Void data) { + DeclarationDescriptor descriptor = trace.get(BindingContext.DECLARATION_TO_DESCRIPTOR, property); + assert descriptor instanceof VariableDescriptor; + concernedVariables.add((VariableDescriptor) descriptor); + return null; + } + + @Override + public Void visitForExpression(JetForExpression expression, Void data) { + JetParameter loopParameter = expression.getLoopParameter(); + if (loopParameter != null) { + DeclarationDescriptor descriptor = trace.get(BindingContext.DECLARATION_TO_DESCRIPTOR, loopParameter); + assert descriptor instanceof VariableDescriptor; + concernedVariables.add((VariableDescriptor) descriptor); + } + return null; + } + }, null); + return concernedVariables; } private static class InitializationPoints { @@ -433,8 +509,12 @@ public class JetFlowInformationProvider { } } - public boolean hasPossibleInitializers() { - return !possiblePoints.isEmpty(); + public Set getPossiblePoints() { + return possiblePoints; + } + + public boolean canBeUninitialized() { + return canBeUninitialized; } @Override diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java index 9fdd098d97a..11680129eb7 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java @@ -144,8 +144,9 @@ public interface Errors { PsiElementOnlyDiagnosticFactory1 UNINITIALIZED_VARIABLE = PsiElementOnlyDiagnosticFactory1.create(ERROR, "Variable ''{0}'' must be initialized", NAME); PsiElementOnlyDiagnosticFactory1 VAL_REASSIGNMENT = PsiElementOnlyDiagnosticFactory1.create(ERROR, "Val can not be reassigned", NAME); + PsiElementOnlyDiagnosticFactory1 INITIALIZATION_USING_BACKING_FIELD = PsiElementOnlyDiagnosticFactory1.create(ERROR, "Initialization using backing field required", NAME); - PsiElementOnlyDiagnosticFactory1 FUNCTION_PARAMETERS_OF_INLINE_FUNCTION = PsiElementOnlyDiagnosticFactory1.create(ERROR, "Funciton parameters of inline funciton can only be invoked", NAME); + PsiElementOnlyDiagnosticFactory1 FUNCTION_PARAMETERS_OF_INLINE_FUNCTION = PsiElementOnlyDiagnosticFactory1.create(ERROR, "Function parameters of inline function can only be invoked", NAME); SimpleDiagnosticFactory UNREACHABLE_CODE = SimpleDiagnosticFactory.create(ERROR, "Unreachable code"); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BodyResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BodyResolver.java index 7bc2afd02c8..bc2a5cd90ab 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BodyResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/BodyResolver.java @@ -296,7 +296,7 @@ public class BodyResolver { ConstructorDescriptor primaryConstructor = classDescriptor.getUnsubstitutedPrimaryConstructor(); assert primaryConstructor != null; final JetScope scopeForConstructor = getInnerScopeForConstructor(primaryConstructor, classDescriptor.getScopeForMemberResolution(), true); - ExpressionTypingServices typeInferrer = context.getSemanticServices().getTypeInferrerServices(createFieldAssignTrackingTrace()); // TODO : flow + ExpressionTypingServices typeInferrer = context.getSemanticServices().getTypeInferrerServices(traceForConstructors); for (JetClassInitializer anonymousInitializer : anonymousInitializers) { typeInferrer.getType(scopeForConstructor, anonymousInitializer.getBody(), NO_EXPECTED_TYPE); } @@ -506,22 +506,6 @@ public class BodyResolver { }); } - private ObservableBindingTrace createFieldAssignTrackingTrace() { - return new ObservableBindingTrace(traceForConstructors).addHandler(BindingContext.VARIABLE_ASSIGNMENT, new ObservableBindingTrace.RecordHandler() { - @Override - public void handleRecord(WritableSlice jetExpressionBooleanWritableSlice, JetExpression expression, DeclarationDescriptor descriptor) { - if (expression instanceof JetSimpleNameExpression) { - JetSimpleNameExpression variable = (JetSimpleNameExpression) expression; - if (variable.getReferencedNameElementType() == JetTokens.FIELD_IDENTIFIER) { - if (descriptor instanceof PropertyDescriptor) { - traceForMembers.record(BindingContext.IS_INITIALIZED, (PropertyDescriptor) descriptor); - } - } - } - } - }); - } - private void resolvePropertyInitializer(JetProperty property, PropertyDescriptor propertyDescriptor, JetExpression initializer, JetScope scope) { //JetFlowInformationProvider flowInformationProvider = context.getClassDescriptorResolver().computeFlowData(property, initializer); // TODO : flow JET-15 ExpressionTypingServices typeInferrer = context.getSemanticServices().getTypeInferrerServices(traceForConstructors); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java index 3b43c0bbe42..37b9a59d796 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java @@ -42,8 +42,8 @@ public class TopDownAnalyzer { new DelegationResolver(context).process(); new OverrideResolver(context).process(); new BodyResolver(context).resolveBehaviorDeclarationBodies(); - new DeclarationsChecker(context).process(); new ControlFlowAnalyzer(context, flowDataTraceFactory, declaredLocally).process(); + new DeclarationsChecker(context).process(); } public static void processStandardLibraryNamespace( diff --git a/compiler/testData/checkerWithErrorTypes/full/FlowAnalyseVariables.jet b/compiler/testData/checkerWithErrorTypes/full/FlowAnalyseVariables.jet deleted file mode 100644 index 0a92f3741a7..00000000000 --- a/compiler/testData/checkerWithErrorTypes/full/FlowAnalyseVariables.jet +++ /dev/null @@ -1,106 +0,0 @@ -namespace uninitialized_vals { - -fun t1(b : Boolean) { - val v : Int - if (v == 0) {} - - var u: String - if (b) { - u = "s" - } - doSmth(u) - - var r: String - if (b) { - r = "s" - } - else { - r = "tg" - } - doSmth(r) - - var t: String - if (b) - doSmth(t) - else - t = "ss" - doSmth(t) - - val i = 3 - doSmth(i) - if (b) { - return; - } - doSmth(i) - if (i is Int) { - return; - } -} - -fun t2() { - val s = "ss" - - for (i in 0..2) { - doSmth(s) - } -} - -fun doSmth(s: String) {} -fun doSmth(i: Int) {} - -class A() {} - -fun t4(a: A, val b: A, var c: A) { - a = A() - b = A() - c = A() -} -} - -namespace reassigned_vals { - -fun t1() { - val a : Int = 1 - a = 2 - - var b : Int = 1 - b = 3 -} - -abstract enum class ProtocolState { - WAITING { - override fun signal() = ProtocolState.TALKING - } - - TALKING { - override fun signal() = ProtocolState.WAITING - } - - abstract fun signal() : ProtocolState -} - -fun t2() { - val x: ProtocolState = ProtocolState.WAITING - x = x.signal() - x = x.signal() -} - -fun t3() { - val x = 1 - x += 2 - val y = 3 - y *= 4 - var z = 5 - z -= y -} - -fun t4() { - for (i in 0..2) { - i += 1 - fun t5() { - i += 3 - } - } -} - -} \ No newline at end of file diff --git a/compiler/testData/checkerWithErrorTypes/full/UninitializedOrReassignedVariables.jet b/compiler/testData/checkerWithErrorTypes/full/UninitializedOrReassignedVariables.jet new file mode 100644 index 00000000000..dab85511537 --- /dev/null +++ b/compiler/testData/checkerWithErrorTypes/full/UninitializedOrReassignedVariables.jet @@ -0,0 +1,250 @@ +namespace uninitialized_reassigned_variables + +fun doSmth(s: String) {} +fun doSmth(i: Int) {} + +// ------------------------------------------------ +// uninitialized variables + +fun t1(b : Boolean) { + val v : Int + if (v == 0) {} + + var u: String + if (b) { + u = "s" + } + doSmth(u) + + var r: String + if (b) { + r = "s" + } + else { + r = "tg" + } + doSmth(r) + + var t: String + if (b) + doSmth(t) + else + t = "ss" + doSmth(t) + + val i = 3 + doSmth(i) + if (b) { + return; + } + doSmth(i) + if (i is Int) { + return; + } +} + +fun t2() { + val s = "ss" + + for (i in 0..2) { + doSmth(s) + } +} + +class A() {} + +fun t4(a: A, val b: A, var c: A) { + a = A() + b = A() + c = A() +} + +// ------------------------------------------------ +// reassigned vals + +fun t1() { + val a : Int = 1 + a = 2 + + var b : Int = 1 + b = 3 +} + +abstract enum class ProtocolState { + WAITING { + override fun signal() = ProtocolState.TALKING + } + + TALKING { + override fun signal() = ProtocolState.WAITING + } + + abstract fun signal() : ProtocolState +} + +fun t2() { + val x: ProtocolState = ProtocolState.WAITING + x = x.signal() + x = x.signal() +} + +fun t3() { + val x = 1 + x += 2 + val y = 3 + y *= 4 + var z = 5 + z -= y +} + +fun t4() { + for (i in 0..2) { + i += 1 + fun t5() { + i += 3 + } + } +} + +// ------------------------------------------------ +// backing fields + +var x = 10 +val y = 10 +val z = 10 + +class AnonymousInitializers(var a: String, val b: String) { + { + $a = "30" + a = "s" + + $b = "3" + b = "tt" + } + + val i: Int + { + $i = 121 + } + + { + x = 11 + $y = 11 + z = 10 + } + + val j: Int + get() = 20 + + { + $i = 13 + $j = 30 + j = 34 + } + + val k: String + { + if (1 < 3) { + k = "a" + } + else { + $k = "b" + } + } + + val l: String + { + if (1 < 3) { + $l = "a" + } + else { + $l = "b" + } + } + + val o: String + { + if (1 < 3) { + $o = "a" + } + } + + var m: Int = 30 + + { + $m = 400 + } + + val n: Int + + { + while (n == 0) { + } + $n = 10 + while (n == 0) { + } + } + + var p = 1 + { + p++ + } +} + +fun reassignFunParams(val a: Int) { + a = 1 +} + +open class Open(a: Int, w: Int) {} + +class LocalValsVsProperties(val a: Int, w: Int) : Open(a, w) { + val x : Int + val y : Int + { + $x = 1 + val b = x + } + val b = a + + fun foo() { + val r : Int + doSmth(x) + doSmth(y) + doSmth(r) + doSmth(a) + } + var xx = w + var yy : Int + { + w += 1 + $yy = w + } +} + +class Outer() { + val a : Int + var b : Int + + { + $a = 1 + $b = 1 + } + + class Inner() { + { + a++ + b++ + } + } + + fun foo() { + a++ + b++ + } +} + +class ForwardAccessToBackingField() { //kt-147 + val a = $a // error + val b = $c // error + val c = 1 +} \ No newline at end of file