From e31e12474f7714be59da5fa42a1d63c8f8f7980c Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Fri, 20 Jan 2017 16:38:38 +0300 Subject: [PATCH] KT-15844 Property with type inferred from getter could access primary constructor parameters inside the getter (which is incorrect, and caused problems in compilation). Fix scopes for property descriptor resolution. 'getScopeForMemberDeclarationResolution' in AbstractLazyMemberScope should always return member declaration resolution scope. Two scopes are required, because both property initializer and property accessors should see property type parameters. --- .../kotlin/resolve/DescriptorResolver.java | 66 +++++++++++++------ .../descriptors/AbstractLazyMemberScope.kt | 9 +-- .../lazy/descriptors/LazyClassMemberScope.kt | 11 ++-- .../descriptors/LazyPackageMemberScope.kt | 7 +- .../primaryConstructorParameter.kt | 23 +++++++ .../primaryConstructorParameter.txt | 24 +++++++ .../checkers/DiagnosticsTestGenerated.java | 6 ++ .../types/DefaultModalityModifiersTest.java | 4 +- 8 files changed, 115 insertions(+), 35 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/properties/inferenceFromGetters/primaryConstructorParameter.kt create mode 100644 compiler/testData/diagnostics/tests/properties/inferenceFromGetters/primaryConstructorParameter.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java index 13e6e619e82..56a5a696cf4 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java @@ -402,6 +402,20 @@ public class DescriptorResolver { LexicalScope scopeForAnnotationsResolve, List typeParameters, BindingTrace trace + ) { + List descriptors = + resolveTypeParametersForDescriptor(containingDescriptor, scopeForAnnotationsResolve, typeParameters, trace); + for (TypeParameterDescriptorImpl descriptor : descriptors) { + extensibleScope.addClassifierDescriptor(descriptor); + } + return descriptors; + } + + private List resolveTypeParametersForDescriptor( + DeclarationDescriptor containingDescriptor, + LexicalScope scopeForAnnotationsResolve, + List typeParameters, + BindingTrace trace ) { assert containingDescriptor instanceof FunctionDescriptor || containingDescriptor instanceof PropertyDescriptor || @@ -411,15 +425,13 @@ public class DescriptorResolver { List result = new ArrayList(); for (int i = 0, typeParametersSize = typeParameters.size(); i < typeParametersSize; i++) { KtTypeParameter typeParameter = typeParameters.get(i); - result.add(resolveTypeParameterForDescriptor( - containingDescriptor, extensibleScope, scopeForAnnotationsResolve, typeParameter, i, trace)); + result.add(resolveTypeParameterForDescriptor(containingDescriptor, scopeForAnnotationsResolve, typeParameter, i, trace)); } return result; } private TypeParameterDescriptorImpl resolveTypeParameterForDescriptor( final DeclarationDescriptor containingDescriptor, - LexicalWritableScope extensibleScope, LexicalScope scopeForAnnotationsResolve, final KtTypeParameter typeParameter, int index, @@ -452,7 +464,6 @@ public class DescriptorResolver { supertypeLoopsResolver ); trace.record(BindingContext.TYPE_PARAMETER, typeParameter, typeParameterDescriptor); - extensibleScope.addClassifierDescriptor(typeParameterDescriptor); return typeParameterDescriptor; } @@ -777,7 +788,8 @@ public class DescriptorResolver { @NotNull public PropertyDescriptor resolvePropertyDescriptor( @NotNull DeclarationDescriptor containingDeclaration, - @NotNull LexicalScope scope, + @NotNull LexicalScope scopeForDeclarationResolution, + @NotNull LexicalScope scopeForInitializerResolution, @NotNull KtProperty property, @NotNull final BindingTrace trace, @NotNull DataFlowInfo dataFlowInfo @@ -794,7 +806,7 @@ public class DescriptorResolver { final AnnotationSplitter.PropertyWrapper wrapper = new AnnotationSplitter.PropertyWrapper(property); - Annotations allAnnotations = annotationResolver.resolveAnnotationsWithoutArguments(scope, modifierList, trace); + Annotations allAnnotations = annotationResolver.resolveAnnotationsWithoutArguments(scopeForDeclarationResolution, modifierList, trace); AnnotationSplitter annotationSplitter = new AnnotationSplitter(storageManager, allAnnotations, new Function0>() { @Override @@ -826,43 +838,55 @@ public class DescriptorResolver { wrapper.setDescriptor(propertyDescriptor); List typeParameterDescriptors; - LexicalScope scopeWithTypeParameters; + LexicalScope scopeForDeclarationResolutionWithTypeParameters; + LexicalScope scopeForInitializerResolutionWithTypeParameters; KotlinType receiverType = null; { List typeParameters = property.getTypeParameters(); if (typeParameters.isEmpty()) { - scopeWithTypeParameters = scope; + scopeForDeclarationResolutionWithTypeParameters = scopeForDeclarationResolution; + scopeForInitializerResolutionWithTypeParameters = scopeForInitializerResolution; typeParameterDescriptors = Collections.emptyList(); } else { - LexicalWritableScope writableScope = new LexicalWritableScope( - scope, containingDeclaration, false, new TraceBasedLocalRedeclarationChecker(trace, overloadChecker), + LexicalWritableScope writableScopeForDeclarationResolution = new LexicalWritableScope( + scopeForDeclarationResolution, containingDeclaration, false, new TraceBasedLocalRedeclarationChecker(trace, overloadChecker), + LexicalScopeKind.PROPERTY_HEADER); + LexicalWritableScope writableScopeForInitializerResolution = new LexicalWritableScope( + scopeForInitializerResolution, containingDeclaration, false, LocalRedeclarationChecker.DO_NOTHING.INSTANCE, LexicalScopeKind.PROPERTY_HEADER); typeParameterDescriptors = resolveTypeParametersForDescriptor( - propertyDescriptor, writableScope, scope, typeParameters, trace); - writableScope.freeze(); - resolveGenericBounds(property, propertyDescriptor, writableScope, typeParameterDescriptors, trace); - scopeWithTypeParameters = writableScope; + propertyDescriptor, + scopeForDeclarationResolution, typeParameters, trace); + for (TypeParameterDescriptor descriptor : typeParameterDescriptors) { + writableScopeForDeclarationResolution.addClassifierDescriptor(descriptor); + writableScopeForInitializerResolution.addClassifierDescriptor(descriptor); + } + writableScopeForDeclarationResolution.freeze(); + writableScopeForInitializerResolution.freeze(); + resolveGenericBounds(property, propertyDescriptor, writableScopeForDeclarationResolution, typeParameterDescriptors, trace); + scopeForDeclarationResolutionWithTypeParameters = writableScopeForDeclarationResolution; + scopeForInitializerResolutionWithTypeParameters = writableScopeForInitializerResolution; } KtTypeReference receiverTypeRef = property.getReceiverTypeReference(); if (receiverTypeRef != null) { - receiverType = typeResolver.resolveType(scopeWithTypeParameters, receiverTypeRef, trace, true); + receiverType = typeResolver.resolveType(scopeForDeclarationResolutionWithTypeParameters, receiverTypeRef, trace, true); } } ReceiverParameterDescriptor receiverDescriptor = DescriptorFactory.createExtensionReceiverParameterForCallable(propertyDescriptor, receiverType); - LexicalScope scopeForInitializer = ScopeUtils.makeScopeForPropertyInitializer(scopeWithTypeParameters, propertyDescriptor); + LexicalScope scopeForInitializer = ScopeUtils.makeScopeForPropertyInitializer(scopeForInitializerResolutionWithTypeParameters, propertyDescriptor); KotlinType typeIfKnown = variableTypeAndInitializerResolver.resolveTypeNullable( propertyDescriptor, scopeForInitializer, property, dataFlowInfo, /* local = */ trace, false ); PropertyGetterDescriptorImpl getter = resolvePropertyGetterDescriptor( - scopeWithTypeParameters, property, propertyDescriptor, annotationSplitter, trace, typeIfKnown); + scopeForDeclarationResolutionWithTypeParameters, property, propertyDescriptor, annotationSplitter, trace, typeIfKnown); KotlinType type = typeIfKnown != null ? typeIfKnown : getter.getReturnType(); @@ -876,7 +900,7 @@ public class DescriptorResolver { receiverDescriptor); PropertySetterDescriptor setter = resolvePropertySetterDescriptor( - scopeWithTypeParameters, property, propertyDescriptor, annotationSplitter, trace); + scopeForDeclarationResolutionWithTypeParameters, property, propertyDescriptor, annotationSplitter, trace); propertyDescriptor.initialize(getter, setter); @@ -1007,7 +1031,7 @@ public class DescriptorResolver { @NotNull private PropertyGetterDescriptorImpl resolvePropertyGetterDescriptor( - @NotNull LexicalScope scopeWithTypeParameters, + @NotNull LexicalScope scopeForDeclarationResolution, @NotNull KtProperty property, @NotNull PropertyDescriptor propertyDescriptor, @NotNull AnnotationSplitter annotationSplitter, @@ -1020,7 +1044,7 @@ public class DescriptorResolver { if (getter != null) { Annotations getterAnnotations = new CompositeAnnotations(CollectionsKt.listOf( annotationSplitter.getAnnotationsForTarget(PROPERTY_GETTER), - annotationResolver.resolveAnnotationsWithoutArguments(scopeWithTypeParameters, getter.getModifierList(), trace))); + annotationResolver.resolveAnnotationsWithoutArguments(scopeForDeclarationResolution, getter.getModifierList(), trace))); getterDescriptor = new PropertyGetterDescriptorImpl( propertyDescriptor, getterAnnotations, @@ -1031,7 +1055,7 @@ public class DescriptorResolver { property.hasModifier(KtTokens.INLINE_KEYWORD) || getter.hasModifier(KtTokens.INLINE_KEYWORD), CallableMemberDescriptor.Kind.DECLARATION, null, KotlinSourceElementKt.toSourceElement(getter) ); - getterType = determineGetterReturnType(scopeWithTypeParameters, trace, getterDescriptor, getter, propertyTypeIfKnown); + getterType = determineGetterReturnType(scopeForDeclarationResolution, trace, getterDescriptor, getter, propertyTypeIfKnown); trace.record(BindingContext.PROPERTY_ACCESSOR, getter, getterDescriptor); } else { diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/AbstractLazyMemberScope.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/AbstractLazyMemberScope.kt index 183503ba036..f5cd3068e39 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/AbstractLazyMemberScope.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/AbstractLazyMemberScope.kt @@ -82,10 +82,9 @@ protected constructor( val declarations = declarationProvider.getFunctionDeclarations(name) for (functionDeclaration in declarations) { - val resolutionScope = getScopeForMemberDeclarationResolution(functionDeclaration) result.add(c.functionDescriptorResolver.resolveFunctionDescriptor( thisDescriptor, - resolutionScope, + getScopeForMemberDeclarationResolution(functionDeclaration), functionDeclaration, trace, c.declarationScopeProvider.getOuterDataFlowInfoForDeclaration(functionDeclaration))) @@ -98,6 +97,8 @@ protected constructor( protected abstract fun getScopeForMemberDeclarationResolution(declaration: KtDeclaration): LexicalScope + protected abstract fun getScopeForInitializerResolution(declaration: KtDeclaration): LexicalScope + protected abstract fun getNonDeclaredClasses(name: Name, result: MutableSet) protected abstract fun getNonDeclaredFunctions(name: Name, result: MutableSet) @@ -112,10 +113,10 @@ protected constructor( val declarations = declarationProvider.getPropertyDeclarations(name) for (propertyDeclaration in declarations) { - val resolutionScope = getScopeForMemberDeclarationResolution(propertyDeclaration) val propertyDescriptor = c.descriptorResolver.resolvePropertyDescriptor( thisDescriptor, - resolutionScope, + getScopeForMemberDeclarationResolution(propertyDeclaration), + getScopeForInitializerResolution(propertyDeclaration), propertyDeclaration, trace, c.declarationScopeProvider.getOuterDataFlowInfoForDeclaration(propertyDeclaration)) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassMemberScope.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassMemberScope.kt index 8e2bf421805..bd6d04f8a45 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassMemberScope.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassMemberScope.kt @@ -95,12 +95,11 @@ open class LazyClassMemberScope( private val primaryConstructor: NullableLazyValue = c.storageManager.createNullableLazyValue { resolvePrimaryConstructor() } - override fun getScopeForMemberDeclarationResolution(declaration: KtDeclaration): LexicalScope { - if (declaration is KtProperty) { - return thisDescriptor.scopeForInitializerResolution - } - return thisDescriptor.scopeForMemberDeclarationResolution - } + override fun getScopeForMemberDeclarationResolution(declaration: KtDeclaration): LexicalScope = + thisDescriptor.scopeForMemberDeclarationResolution + + override fun getScopeForInitializerResolution(declaration: KtDeclaration): LexicalScope = + thisDescriptor.scopeForInitializerResolution private fun generateFakeOverrides(name: Name, fromSupertypes: Collection, result: MutableCollection, exactDescriptorClass: Class) { OverridingUtil.generateOverridesInFunctionGroup(name, fromSupertypes, ArrayList(result), thisDescriptor, object : OverridingStrategy() { diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyPackageMemberScope.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyPackageMemberScope.kt index 0ba0765d113..98932bdde9c 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyPackageMemberScope.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyPackageMemberScope.kt @@ -36,8 +36,11 @@ class LazyPackageMemberScope( return computeDescriptorsFromDeclaredElements(kindFilter, nameFilter, NoLookupLocation.WHEN_GET_ALL_DESCRIPTORS) } - override fun getScopeForMemberDeclarationResolution(declaration: KtDeclaration) - = resolveSession.fileScopeProvider.getFileResolutionScope(declaration.getContainingKtFile()) + override fun getScopeForMemberDeclarationResolution(declaration: KtDeclaration) = + resolveSession.fileScopeProvider.getFileResolutionScope(declaration.containingKtFile) + + override fun getScopeForInitializerResolution(declaration: KtDeclaration) = + getScopeForMemberDeclarationResolution(declaration) override fun getNonDeclaredClasses(name: Name, result: MutableSet) { // No extra classes diff --git a/compiler/testData/diagnostics/tests/properties/inferenceFromGetters/primaryConstructorParameter.kt b/compiler/testData/diagnostics/tests/properties/inferenceFromGetters/primaryConstructorParameter.kt new file mode 100644 index 00000000000..bdcd95cfc6d --- /dev/null +++ b/compiler/testData/diagnostics/tests/properties/inferenceFromGetters/primaryConstructorParameter.kt @@ -0,0 +1,23 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +object Delegate { + operator fun getValue(x: Any?, y: Any?): String = "" +} + +fun delegateFactory(p: Any) = Delegate + +class C(p: Any, val v: Any) { + + val test1 get() = p + + val test2 get() = v + + // NB here we can use both 'T' (property type parameter) and 'p' (primary constructor parameter) + val List.test3 by delegateFactory(p) + + val test4 get() { return p } + + var test5 + get() { return p } + set(nv) { p.let {} } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/properties/inferenceFromGetters/primaryConstructorParameter.txt b/compiler/testData/diagnostics/tests/properties/inferenceFromGetters/primaryConstructorParameter.txt new file mode 100644 index 00000000000..10619316db1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/properties/inferenceFromGetters/primaryConstructorParameter.txt @@ -0,0 +1,24 @@ +package + +public fun delegateFactory(/*0*/ p: kotlin.Any): Delegate + +public final class C { + public constructor C(/*0*/ p: kotlin.Any, /*1*/ v: kotlin.Any) + public final val test1: [ERROR : Error function type] + public final val test2: kotlin.Any + public final val test4: [ERROR : No type, no body] + public final var test5: [ERROR : No type, no body] + public final val v: kotlin.Any + public final val kotlin.collections.List.test3: kotlin.String + 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 object Delegate { + private constructor Delegate() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final operator fun getValue(/*0*/ x: kotlin.Any?, /*1*/ y: kotlin.Any?): kotlin.String + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index bdce5cb01b1..440569c10ef 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -15612,6 +15612,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("primaryConstructorParameter.kt") + public void testPrimaryConstructorParameter() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/properties/inferenceFromGetters/primaryConstructorParameter.kt"); + doTest(fileName); + } + @TestMetadata("recursiveGetter.kt") public void testRecursiveGetter() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/properties/inferenceFromGetters/recursiveGetter.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/types/DefaultModalityModifiersTest.java b/compiler/tests/org/jetbrains/kotlin/types/DefaultModalityModifiersTest.java index 5ab9bce66ca..93d8402d99e 100644 --- a/compiler/tests/org/jetbrains/kotlin/types/DefaultModalityModifiersTest.java +++ b/compiler/tests/org/jetbrains/kotlin/types/DefaultModalityModifiersTest.java @@ -145,7 +145,7 @@ public class DefaultModalityModifiersTest extends KotlinTestWithEnvironment { List declarations = aClass.getDeclarations(); KtProperty property = (KtProperty) declarations.get(0); PropertyDescriptor propertyDescriptor = descriptorResolver.resolvePropertyDescriptor( - classDescriptor, scope, property, KotlinTestUtils.DUMMY_TRACE, DataFlowInfoFactory.EMPTY); + classDescriptor, scope, scope, property, KotlinTestUtils.DUMMY_TRACE, DataFlowInfoFactory.EMPTY); assertEquals(expectedPropertyModality, propertyDescriptor.getModality()); } @@ -158,7 +158,7 @@ public class DefaultModalityModifiersTest extends KotlinTestWithEnvironment { List declarations = aClass.getDeclarations(); KtProperty property = (KtProperty) declarations.get(0); PropertyDescriptor propertyDescriptor = descriptorResolver.resolvePropertyDescriptor( - classDescriptor, scope, property, KotlinTestUtils.DUMMY_TRACE, DataFlowInfoFactory.EMPTY); + classDescriptor, scope, scope, property, KotlinTestUtils.DUMMY_TRACE, DataFlowInfoFactory.EMPTY); PropertyAccessorDescriptor propertyAccessor = isGetter ? propertyDescriptor.getGetter() : propertyDescriptor.getSetter();