From 7c90fbe4de632ccf7c7a2957a76d8aaf48aff666 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 7 Apr 2016 12:22:14 +0300 Subject: [PATCH] Compute return type for property getters more safely Do not call `isError()` on property's type right after creation of PropertyGetterDescriptor because the property has no getter yet (it's created but not yet stored to the property, that happens a bit later), and `isError()` leads to computation of the delegate type, which for delegated properties performs some complex resolution (see `VariableTypeResolver#process`) which relies on the fact that the property already has a getter. Since the purpose of the original change (883e2e4d) was to support a quick fix which would add the type to a property in an expression like "val x get() = ...", check the type (or initializer) presence in the PSI instead, this is safer and is still suitable for the quick fix. Also fix arguments to "wrong getter type" diagnostic: previously something useless like "expected Int, actual Int" was reported #KT-11809 Fixed --- .../kotlin/resolve/DescriptorResolver.java | 47 +++++++++++++------ .../delegatedProperty/nonDefaultAccessors.kt | 12 +++++ .../delegatedProperty/nonDefaultAccessors.txt | 12 +++++ .../diagnostics/tests/regressions/kt251.kt | 2 +- .../checkers/DiagnosticsTestGenerated.java | 6 +++ 5 files changed, 64 insertions(+), 15 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/delegatedProperty/nonDefaultAccessors.kt create mode 100644 compiler/testData/diagnostics/tests/delegatedProperty/nonDefaultAccessors.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java index 30b93712402..d42ae7149d1 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DescriptorResolver.java @@ -897,16 +897,6 @@ public class DescriptorResolver { annotationSplitter.getAnnotationsForTarget(PROPERTY_GETTER), annotationResolver.resolveAnnotationsWithoutArguments(scopeWithTypeParameters, getter.getModifierList(), trace))); - KotlinType outType = propertyDescriptor.getType(); - KotlinType returnType = outType; - KtTypeReference returnTypeReference = getter.getReturnTypeReference(); - if (returnTypeReference != null) { - returnType = typeResolver.resolveType(scopeWithTypeParameters, returnTypeReference, trace, true); - if (!TypeUtils.equalTypes(returnType, outType)) { - trace.report(WRONG_GETTER_RETURN_TYPE.on(returnTypeReference, propertyDescriptor.getReturnType(), outType)); - } - } - getterDescriptor = new PropertyGetterDescriptorImpl( propertyDescriptor, getterAnnotations, resolveModalityFromModifiers(getter, propertyDescriptor.getModality()), @@ -914,10 +904,8 @@ public class DescriptorResolver { /* isDefault = */ false, getter.hasModifier(EXTERNAL_KEYWORD), CallableMemberDescriptor.Kind.DECLARATION, null, KotlinSourceElementKt.toSourceElement(getter) ); - if (returnType.isError() && !getter.hasBlockBody() && getter.hasBody()) { - returnType = inferReturnTypeFromExpressionBody(storageManager, expressionTypingServices, trace, scopeWithTypeParameters, - DataFlowInfoFactory.EMPTY, getter, getterDescriptor); - } + KotlinType returnType = + determineGetterReturnType(scopeWithTypeParameters, trace, getterDescriptor, getter, propertyDescriptor.getType()); getterDescriptor.initialize(returnType); trace.record(BindingContext.PROPERTY_ACCESSOR, getter, getterDescriptor); } @@ -930,6 +918,37 @@ public class DescriptorResolver { return getterDescriptor; } + @NotNull + private KotlinType determineGetterReturnType( + @NotNull LexicalScope scope, + @NotNull BindingTrace trace, + @NotNull PropertyGetterDescriptor getterDescriptor, + @NotNull KtPropertyAccessor getter, + @NotNull KotlinType propertyType + ) { + KtTypeReference returnTypeReference = getter.getReturnTypeReference(); + if (returnTypeReference != null) { + KotlinType explicitReturnType = typeResolver.resolveType(scope, returnTypeReference, trace, true); + if (!TypeUtils.equalTypes(explicitReturnType, propertyType)) { + trace.report(WRONG_GETTER_RETURN_TYPE.on(returnTypeReference, propertyType, explicitReturnType)); + } + return explicitReturnType; + } + + // If a property has no type specified in the PSI but the getter does (or has an initializer e.g. "val x get() = ..."), + // infer the correct type for the getter but leave the error type for the property. + // This is useful for an IDE quick fix which would add the type to the property + KtProperty property = getter.getProperty(); + if (!property.hasDelegateExpressionOrInitializer() && property.getTypeReference() == null && + getter.hasBody() && !getter.hasBlockBody()) { + return inferReturnTypeFromExpressionBody( + storageManager, expressionTypingServices, trace, scope, DataFlowInfoFactory.EMPTY, getter, getterDescriptor + ); + } + + return propertyType; + } + @NotNull /*package*/ static DeferredType inferReturnTypeFromExpressionBody( @NotNull StorageManager storageManager, diff --git a/compiler/testData/diagnostics/tests/delegatedProperty/nonDefaultAccessors.kt b/compiler/testData/diagnostics/tests/delegatedProperty/nonDefaultAccessors.kt new file mode 100644 index 00000000000..a3d8233880b --- /dev/null +++ b/compiler/testData/diagnostics/tests/delegatedProperty/nonDefaultAccessors.kt @@ -0,0 +1,12 @@ +// KT-11809 Assertion error when delegated property has getter + +class A { + val p1 by this + get + + var p2 by this + get() = "" + + operator fun getValue(a: Any?, p: Any?) = "" + operator fun setValue(a: Any?, p: Any?, v: Any?) {} +} diff --git a/compiler/testData/diagnostics/tests/delegatedProperty/nonDefaultAccessors.txt b/compiler/testData/diagnostics/tests/delegatedProperty/nonDefaultAccessors.txt new file mode 100644 index 00000000000..60699b2793e --- /dev/null +++ b/compiler/testData/diagnostics/tests/delegatedProperty/nonDefaultAccessors.txt @@ -0,0 +1,12 @@ +package + +public final class A { + public constructor A() + public final val p1: kotlin.String + public final var p2: kotlin.String + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final operator fun getValue(/*0*/ a: kotlin.Any?, /*1*/ p: kotlin.Any?): kotlin.String + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final operator fun setValue(/*0*/ a: kotlin.Any?, /*1*/ p: kotlin.Any?, /*2*/ v: kotlin.Any?): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/regressions/kt251.kt b/compiler/testData/diagnostics/tests/regressions/kt251.kt index 0f3ef696687..fdfb36382b3 100644 --- a/compiler/testData/diagnostics/tests/regressions/kt251.kt +++ b/compiler/testData/diagnostics/tests/regressions/kt251.kt @@ -5,7 +5,7 @@ class A() { field = value } val y: Int - get(): String = "s" + get(): String = "s" val z: Int get() { return "s" diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 8e3262cc6e4..53e8b198477 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -4767,6 +4767,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("nonDefaultAccessors.kt") + public void testNonDefaultAccessors() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/delegatedProperty/nonDefaultAccessors.kt"); + doTest(fileName); + } + @TestMetadata("propertyDefferedType.kt") public void testPropertyDefferedType() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/delegatedProperty/propertyDefferedType.kt");