From bc4ec97043a22e455e31c017d5e0982deab6dc1e Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Mon, 21 Mar 2016 13:26:53 +0300 Subject: [PATCH] Captured value initialization is no more allowed #KT-10445 Fixed --- .../cfg/ControlFlowInformationProvider.java | 26 ++++++- .../jetbrains/kotlin/diagnostics/Errors.java | 1 + .../rendering/DefaultErrorMessages.java | 1 + .../tests/controlFlowAnalysis/initInLambda.kt | 77 +++++++++++++++++++ .../controlFlowAnalysis/initInLambda.txt | 26 +++++++ ...nLambda.kt => initializationInLocalFun.kt} | 0 ...ambda.txt => initializationInLocalFun.txt} | 0 .../checkers/DiagnosticsTestGenerated.java | 12 ++- .../kotlin/idea/j2k/J2kPostProcessings.kt | 29 ++++--- .../quickfix/ChangeVariableMutabilityFix.kt | 10 ++- .../kotlin/idea/quickfix/QuickFixRegistrar.kt | 1 + .../capturedValInitialization.kt | 9 +++ .../capturedValInitialization.kt.after | 9 +++ .../idea/quickfix/QuickFixTestGenerated.java | 6 ++ 14 files changed, 188 insertions(+), 19 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/initInLambda.kt create mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/initInLambda.txt rename compiler/testData/diagnostics/tests/controlFlowAnalysis/{initializationInLambda.kt => initializationInLocalFun.kt} (100%) rename compiler/testData/diagnostics/tests/controlFlowAnalysis/{initializationInLambda.txt => initializationInLocalFun.txt} (100%) create mode 100644 idea/testData/quickfix/variables/changeMutability/capturedValInitialization.kt create mode 100644 idea/testData/quickfix/variables/changeMutability/capturedValInitialization.kt.after diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java index 9b329a1f7a1..b3dce48caf5 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java @@ -391,6 +391,22 @@ public class ControlFlowInformationProvider { } } + private boolean isCapturedWrite( + @NotNull VariableDescriptor variableDescriptor, + @NotNull WriteValueInstruction writeValueInstruction + ) { + DeclarationDescriptor containingDeclarationDescriptor = variableDescriptor.getContainingDeclaration(); + if (containingDeclarationDescriptor instanceof ClassDescriptor) return false; + KtElement ownerElement = writeValueInstruction.getOwner().getCorrespondingElement(); + DeclarationDescriptor writeOwnerDescriptor = trace.get(BindingContext.DECLARATION_TO_DESCRIPTOR, ownerElement); + if (containingDeclarationDescriptor.equals(writeOwnerDescriptor)) return false; + if (writeOwnerDescriptor instanceof ClassDescriptor) { + ClassDescriptor writeOwnerClass = (ClassDescriptor) writeOwnerDescriptor; + if (containingDeclarationDescriptor.equals(writeOwnerClass.getUnsubstitutedPrimaryConstructor())) return false; + } + return true; + } + private boolean checkValReassignment( @NotNull VariableInitContext ctxt, @NotNull KtExpression expression, @@ -434,7 +450,8 @@ public class ControlFlowInformationProvider { } boolean isThisOrNoDispatchReceiver = PseudocodeUtil.isThisOrNoDispatchReceiver(writeValueInstruction, trace.getBindingContext()); - if ((mayBeInitializedNotHere || !hasBackingField || !isThisOrNoDispatchReceiver) && !variableDescriptor.isVar()) { + boolean captured = isCapturedWrite(variableDescriptor, writeValueInstruction); + if ((mayBeInitializedNotHere || !hasBackingField || !isThisOrNoDispatchReceiver || captured) && !variableDescriptor.isVar()) { boolean hasReassignMethodReturningUnit = false; KtSimpleNameExpression operationReference = null; PsiElement parent = expression.getParent(); @@ -465,7 +482,12 @@ public class ControlFlowInformationProvider { } if (!hasReassignMethodReturningUnit) { if (!isThisOrNoDispatchReceiver || !varWithValReassignErrorGenerated.contains(variableDescriptor)) { - report(Errors.VAL_REASSIGNMENT.on(expression, variableDescriptor), ctxt); + if (captured && !mayBeInitializedNotHere && hasBackingField && isThisOrNoDispatchReceiver) { + report(Errors.CAPTURED_VAL_INITIALIZATION.on(expression, variableDescriptor), ctxt); + } + else { + report(Errors.VAL_REASSIGNMENT.on(expression, variableDescriptor), ctxt); + } } if (isThisOrNoDispatchReceiver) { // try to get rid of repeating VAL_REASSIGNMENT diagnostic only for vars with no receiver diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 2aafc9a6577..afc6aacc356 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -645,6 +645,7 @@ public interface Errors { DiagnosticFactory0 UNUSED_LAMBDA_EXPRESSION = DiagnosticFactory0.create(WARNING); DiagnosticFactory1 VAL_REASSIGNMENT = DiagnosticFactory1.create(ERROR); + DiagnosticFactory1 CAPTURED_VAL_INITIALIZATION = DiagnosticFactory1.create(ERROR); DiagnosticFactory1 SETTER_PROJECTED_OUT = DiagnosticFactory1.create(ERROR); DiagnosticFactory1 INITIALIZATION_BEFORE_DECLARATION = DiagnosticFactory1.create(ERROR); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index 42178c21d2d..791e1a54e26 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -268,6 +268,7 @@ public class DefaultErrorMessages { MAP.put(UNUSED_LAMBDA_EXPRESSION, "The lambda expression is unused. If you mean a block, you can use 'run { ... }'"); MAP.put(VAL_REASSIGNMENT, "Val cannot be reassigned", NAME); + MAP.put(CAPTURED_VAL_INITIALIZATION, "Captured values initialization is forbidden due to possible reassignment", NAME); MAP.put(SETTER_PROJECTED_OUT, "Setter for ''{0}'' is removed by type projection", NAME); MAP.put(INVISIBLE_SETTER, "Cannot assign to ''{0}'': the setter is ''{1}'' in {2}", NAME, TO_STRING, NAME_OF_PARENT_OR_FILE); MAP.put(INITIALIZATION_BEFORE_DECLARATION, "Variable cannot be initialized before declaration", NAME); diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/initInLambda.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/initInLambda.kt new file mode 100644 index 00000000000..82b424397a1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/initInLambda.kt @@ -0,0 +1,77 @@ +// !DIAGNOSTICS: -ASSIGNED_BUT_NEVER_ACCESSED_VARIABLE + +fun ignoreIt(f: () -> Unit) {} + +fun exec(f: () -> Unit) = f() + +fun foo() { + var x: Int + ignoreIt() { + // Ok + x = 42 + } + // Error! + x.hashCode() +} + +fun bar() { + val x: Int + exec { + x = 13 + } +} + +fun bar2() { + val x: Int + fun foo() { + x = 3 + } + foo() +} + +class My(val cond: Boolean) { + + val y: Int + + init { + val x: Int + if (cond) { + exec { + + } + x = 1 + } + else { + x = 2 + } + y = x + } + + constructor(): this(false) { + val x: Int + x = 2 + exec { + x.hashCode() + } + } +} + +class Your { + val y = if (true) { + val xx: Int + exec { + xx = 42 + } + 24 + } + else 0 +} + +val z = if (true) { + val xx: Int + exec { + xx = 24 + } + 42 +} +else 0 \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/initInLambda.txt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/initInLambda.txt new file mode 100644 index 00000000000..24766701460 --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/initInLambda.txt @@ -0,0 +1,26 @@ +package + +public val z: kotlin.Int +public fun bar(): kotlin.Unit +public fun bar2(): kotlin.Unit +public fun exec(/*0*/ f: () -> kotlin.Unit): kotlin.Unit +public fun foo(): kotlin.Unit +public fun ignoreIt(/*0*/ f: () -> kotlin.Unit): kotlin.Unit + +public final class My { + public constructor My() + public constructor My(/*0*/ cond: kotlin.Boolean) + public final val cond: kotlin.Boolean + public final val y: kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class Your { + public constructor Your() + public final val y: kotlin.Int + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/initializationInLambda.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/initializationInLocalFun.kt similarity index 100% rename from compiler/testData/diagnostics/tests/controlFlowAnalysis/initializationInLambda.kt rename to compiler/testData/diagnostics/tests/controlFlowAnalysis/initializationInLocalFun.kt diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/initializationInLambda.txt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/initializationInLocalFun.txt similarity index 100% rename from compiler/testData/diagnostics/tests/controlFlowAnalysis/initializationInLambda.txt rename to compiler/testData/diagnostics/tests/controlFlowAnalysis/initializationInLocalFun.txt diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 905402411b0..90222754b1f 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -2889,9 +2889,15 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } - @TestMetadata("initializationInLambda.kt") - public void testInitializationInLambda() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlFlowAnalysis/initializationInLambda.kt"); + @TestMetadata("initInLambda.kt") + public void testInitInLambda() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlFlowAnalysis/initInLambda.kt"); + doTest(fileName); + } + + @TestMetadata("initializationInLocalFun.kt") + public void testInitializationInLocalFun() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlFlowAnalysis/initializationInLocalFun.kt"); doTest(fileName); } diff --git a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt index 26f6f718b79..c9f155c3916 100644 --- a/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt +++ b/idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt @@ -93,16 +93,21 @@ object J2KPostProcessingRegistrar { } registerDiagnosticBasedProcessingFactory( - Errors.VAL_REASSIGNMENT, - fun (element: KtSimpleNameExpression, diagnostic: Diagnostic): (() -> Unit)? { - val property = element.mainReference.resolve() as? KtProperty ?: return null - return { - if (!property.isVar) { - property.valOrVarKeyword.replace(KtPsiFactory(element.project).createVarKeyword()) - } + Errors.VAL_REASSIGNMENT, Errors.CAPTURED_VAL_INITIALIZATION + ) { + element: KtSimpleNameExpression, diagnostic: Diagnostic -> + val property = element.mainReference.resolve() as? KtProperty + if (property == null) { + null + } + else { + { + if (!property.isVar) { + property.valOrVarKeyword.replace(KtPsiFactory(element.project).createVarKeyword()) } } - ) + } + } } private inline fun > registerIntentionBasedProcessing( @@ -132,20 +137,20 @@ object J2KPostProcessingRegistrar { } private inline fun registerDiagnosticBasedProcessing( - diagnosticFactory: DiagnosticFactory<*>, + vararg diagnosticFactory: DiagnosticFactory<*>, crossinline fix: (TElement, Diagnostic) -> Unit ) { - registerDiagnosticBasedProcessingFactory(diagnosticFactory) { element: TElement, diagnostic: Diagnostic -> { fix(element, diagnostic) } } + registerDiagnosticBasedProcessingFactory(*diagnosticFactory) { element: TElement, diagnostic: Diagnostic -> { fix(element, diagnostic) } } } private inline fun registerDiagnosticBasedProcessingFactory( - diagnosticFactory: DiagnosticFactory<*>, + vararg diagnosticFactory: DiagnosticFactory<*>, crossinline fixFactory: (TElement, Diagnostic) -> (() -> Unit)? ) { _processings.add(object : J2kPostProcessing { override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? { if (!TElement::class.java.isInstance(element)) return null - val diagnostic = diagnostics.forElement(element).firstOrNull { it.factory == diagnosticFactory } ?: return null + val diagnostic = diagnostics.forElement(element).firstOrNull { it.factory in diagnosticFactory } ?: return null return fixFactory(element as TElement, diagnostic) } }) diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVariableMutabilityFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVariableMutabilityFix.kt index a2029e3e3b8..0af30e5b777 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVariableMutabilityFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVariableMutabilityFix.kt @@ -20,7 +20,9 @@ import com.intellij.codeInsight.intention.IntentionAction import com.intellij.openapi.editor.Editor import com.intellij.openapi.project.Project import com.intellij.psi.PsiFile +import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.diagnostics.Diagnostic +import org.jetbrains.kotlin.diagnostics.DiagnosticFactory1 import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* @@ -51,14 +53,18 @@ class ChangeVariableMutabilityFix(element: KtValVarKeywordOwner, private val mak } } - val VAL_REASSIGNMENT_FACTORY: KotlinSingleIntentionActionFactory = object: KotlinSingleIntentionActionFactory() { + class ReassignmentActionFactory(val factory: DiagnosticFactory1<*, DeclarationDescriptor>) : KotlinSingleIntentionActionFactory() { override fun createAction(diagnostic: Diagnostic): IntentionAction? { - val propertyDescriptor = Errors.VAL_REASSIGNMENT.cast(diagnostic).a + val propertyDescriptor = factory.cast(diagnostic).a val declaration = DescriptorToSourceUtils.descriptorToDeclaration(propertyDescriptor) as? KtValVarKeywordOwner ?: return null return ChangeVariableMutabilityFix(declaration, true) } } + val VAL_REASSIGNMENT_FACTORY = ReassignmentActionFactory(Errors.VAL_REASSIGNMENT) + + val CAPTURED_VAL_INITIALIZATION_FACTORY = ReassignmentActionFactory(Errors.CAPTURED_VAL_INITIALIZATION) + val VAR_OVERRIDDEN_BY_VAL_FACTORY: KotlinSingleIntentionActionFactory = object: KotlinSingleIntentionActionFactory() { override fun createAction(diagnostic: Diagnostic): IntentionAction? { val element = diagnostic.psiElement diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt index 5dc65252146..582e534de08 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/QuickFixRegistrar.kt @@ -166,6 +166,7 @@ class QuickFixRegistrar : QuickFixContributor { VAL_WITH_SETTER.registerFactory(ChangeVariableMutabilityFix.VAL_WITH_SETTER_FACTORY) VAL_REASSIGNMENT.registerFactory(ChangeVariableMutabilityFix.VAL_REASSIGNMENT_FACTORY) + CAPTURED_VAL_INITIALIZATION.registerFactory(ChangeVariableMutabilityFix.CAPTURED_VAL_INITIALIZATION_FACTORY) VAR_OVERRIDDEN_BY_VAL.registerFactory(ChangeVariableMutabilityFix.VAR_OVERRIDDEN_BY_VAL_FACTORY) VAL_OR_VAR_ON_FUN_PARAMETER.registerFactory(RemoveValVarFromParameterFix) diff --git a/idea/testData/quickfix/variables/changeMutability/capturedValInitialization.kt b/idea/testData/quickfix/variables/changeMutability/capturedValInitialization.kt new file mode 100644 index 00000000000..32fec5d064a --- /dev/null +++ b/idea/testData/quickfix/variables/changeMutability/capturedValInitialization.kt @@ -0,0 +1,9 @@ +// "Make variable mutable" "true" +fun exec(f: () -> Unit) = f() + +fun foo() { + val x: Int + exec { + x = 42 + } +} diff --git a/idea/testData/quickfix/variables/changeMutability/capturedValInitialization.kt.after b/idea/testData/quickfix/variables/changeMutability/capturedValInitialization.kt.after new file mode 100644 index 00000000000..c7e2aafce69 --- /dev/null +++ b/idea/testData/quickfix/variables/changeMutability/capturedValInitialization.kt.after @@ -0,0 +1,9 @@ +// "Make variable mutable" "true" +fun exec(f: () -> Unit) = f() + +fun foo() { + var x: Int + exec { + x = 42 + } +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index 3485aea4d04..9437570c826 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -7728,6 +7728,12 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/variables/changeMutability"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), true); } + @TestMetadata("capturedValInitialization.kt") + public void testCapturedValInitialization() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/variables/changeMutability/capturedValInitialization.kt"); + doTest(fileName); + } + @TestMetadata("funParameter.kt") public void testFunParameter() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/variables/changeMutability/funParameter.kt");