From 55cb9561c26c1c8050ec2c53b6ab1b7220e413a1 Mon Sep 17 00:00:00 2001 From: Pavel Kirpichenkov Date: Wed, 11 Sep 2019 14:38:16 +0300 Subject: [PATCH] Provide better error messages for read-only delegate Split error reporting into two parts for incorrect and missing candidates. Missing function error is not reported on provideDelegate. Update error factory and default message for error. Update error texts in quick fix test data. #KT-16526 Fixed --- .../jetbrains/kotlin/diagnostics/Errors.java | 2 +- .../rendering/DefaultErrorMessages.java | 2 +- .../resolve/DelegatedPropertyResolver.kt | 71 +++++++++++-------- .../tests/delegatedProperty/missedGetter.kt | 2 +- .../tests/delegatedProperty/missedSetter.kt | 2 +- .../autoImports/delegateExtensionBoth.test | 8 +-- .../autoImports/delegateExtensionGet.test | 4 +- .../autoImports/delegateExtensionSet.test | 4 +- .../autoImports/delegateNoOperator.test | 2 +- .../canBeVal/delegatedProperty2.kt | 2 +- .../canBeVal/delegatedProperty3.kt | 4 +- 11 files changed, 59 insertions(+), 44 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 579f029f4ab..3d0bb60d00f 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -829,7 +829,7 @@ public interface Errors { DiagnosticFactory0 ITERATOR_ON_NULLABLE = DiagnosticFactory0.create(ERROR); DiagnosticFactory1>> ITERATOR_AMBIGUITY = DiagnosticFactory1.create(ERROR); - DiagnosticFactory2 DELEGATE_SPECIAL_FUNCTION_MISSING = DiagnosticFactory2.create(ERROR); + DiagnosticFactory3 DELEGATE_SPECIAL_FUNCTION_MISSING = DiagnosticFactory3.create(ERROR); DiagnosticFactory2>> DELEGATE_SPECIAL_FUNCTION_AMBIGUITY = DiagnosticFactory2.create(ERROR); DiagnosticFactory2>> DELEGATE_SPECIAL_FUNCTION_NONE_APPLICABLE = DiagnosticFactory2.create(ERROR); DiagnosticFactory3 DELEGATE_SPECIAL_FUNCTION_RETURN_TYPE_MISMATCH = DiagnosticFactory3.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 d1d3d9cdb0a..2fb829ccde2 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -476,7 +476,7 @@ public class DefaultErrorMessages { MAP.put(ITERATOR_ON_NULLABLE, "Not nullable value required to call an 'iterator()' method on for-loop range"); MAP.put(ITERATOR_AMBIGUITY, "Method ''iterator()'' is ambiguous for this expression: {0}", AMBIGUOUS_CALLS); - MAP.put(DELEGATE_SPECIAL_FUNCTION_MISSING, "Missing ''{0}'' method on delegate of type ''{1}''", STRING, RENDER_TYPE); + MAP.put(DELEGATE_SPECIAL_FUNCTION_MISSING, "Type ''{1}'' has no method ''{0}'' and thus it cannot serve as a {2}", STRING, RENDER_TYPE, STRING); MAP.put(DELEGATE_SPECIAL_FUNCTION_AMBIGUITY, "Overload resolution ambiguity on method ''{0}'': {1}", STRING, AMBIGUOUS_CALLS); MAP.put(DELEGATE_SPECIAL_FUNCTION_NONE_APPLICABLE, "Property delegate must have a ''{0}'' method. None of the following functions is suitable: {1}", STRING, AMBIGUOUS_CALLS); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegatedPropertyResolver.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegatedPropertyResolver.kt index 583310d89ef..040ad349541 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegatedPropertyResolver.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegatedPropertyResolver.kt @@ -195,24 +195,19 @@ class DelegatedPropertyResolver( isGet = isGet, isComplete = true ) - reportAndRecordDelegateOperatorResults(functionResults, propertyDescriptor, accessor, delegateExpression, delegateType, trace) + if (functionResults.isSuccess) { + recordDelegateOperatorResults(functionResults, propertyDescriptor, accessor, trace) + } else { + reportGetSetValueResolutionError(functionResults, accessor, delegateExpression, delegateType, trace, isGet) + } } - private fun reportAndRecordDelegateOperatorResults( + private fun recordDelegateOperatorResults( result: OverloadResolutionResults, propertyDescriptor: VariableDescriptorWithAccessors, accessor: VariableAccessorDescriptor, - delegateExpression: KtExpression, - delegateType: KotlinType, trace: BindingTrace ) { - if (!result.isSuccess) { - val call = trace.bindingContext.get(DELEGATED_PROPERTY_CALL, accessor) - ?: throw AssertionError("'getDelegatedPropertyConventionMethod' didn't record a call") - reportDelegateOperatorResolutionError(trace, call, result, delegateExpression, delegateType) - return - } - val resultingDescriptor = result.resultingDescriptor val resultingCall = result.resultingCall @@ -230,16 +225,41 @@ class DelegatedPropertyResolver( trace.record(DELEGATED_PROPERTY_RESOLVED_CALL, accessor, resultingCall) } - private fun reportDelegateOperatorResolutionError( + private fun reportGetSetValueResolutionError( + result: OverloadResolutionResults, + accessor: VariableAccessorDescriptor, + delegateExpression: KtExpression, + delegateType: KotlinType, + trace: BindingTrace, + isGet: Boolean + ) { + val call = trace.bindingContext.get(DELEGATED_PROPERTY_CALL, accessor) + ?: throw AssertionError("'getDelegatedPropertyConventionMethod' didn't record a call") + + val errorReportedForCandidate = reportDelegateErrorIfCandidateExists(trace, call, result, delegateExpression) + if (!errorReportedForCandidate) { + reportDelegateFunctionMissing(call, delegateExpression, delegateType, trace, isGet) + } + } + + private fun reportDelegateFunctionMissing( + delegateOperatorCall: Call, + delegateExpression: KtExpression, + delegateType: KotlinType, + trace: BindingTrace, + isGet: Boolean + ) { + val expectedFunction = renderCall(delegateOperatorCall, trace.bindingContext) + val delegateKind = if (isGet) "delegate" else "delegate for var (read-write property)" + trace.report(DELEGATE_SPECIAL_FUNCTION_MISSING.on(delegateExpression, expectedFunction, delegateType, delegateKind)) + } + + private fun reportDelegateErrorIfCandidateExists( trace: BindingTrace, delegateOperatorCall: Call, delegateOperatorResults: OverloadResolutionResults, - delegateExpression: KtExpression, - delegateType: KotlinType, - operatorRequired: Boolean = true + delegateExpression: KtExpression ): Boolean { - val expectedFunction = renderCall(delegateOperatorCall, trace.bindingContext) - val resolutionErrorFactory = when { delegateOperatorResults.isSingleResult || delegateOperatorResults.isIncomplete || @@ -250,17 +270,12 @@ class DelegatedPropertyResolver( else -> null } - if (resolutionErrorFactory != null) { - trace.report(resolutionErrorFactory.on(delegateExpression, expectedFunction, delegateOperatorResults.resultingCalls)) - return true + resolutionErrorFactory?.let { + val expectedFunction = renderCall(delegateOperatorCall, trace.bindingContext) + trace.report(it.on(delegateExpression, expectedFunction, delegateOperatorResults.resultingCalls)) } - if (operatorRequired) { - trace.report(DELEGATE_SPECIAL_FUNCTION_MISSING.on(delegateExpression, expectedFunction, delegateType)) - return true - } - - return false + return resolutionErrorFactory != null } private fun resolveProvideDelegateMethod( @@ -283,8 +298,8 @@ class DelegatedPropertyResolver( if (!provideDelegateResults.isSuccess) { val call = traceForProvideDelegate.bindingContext.get(BindingContext.PROVIDE_DELEGATE_CALL, propertyDescriptor) ?: throw AssertionError("'getDelegatedPropertyConventionMethod' didn't record a call") - val shouldCommitTrace = reportDelegateOperatorResolutionError( - traceForProvideDelegate, call, provideDelegateResults, byExpression, byExpressionType, operatorRequired = false + val shouldCommitTrace = reportDelegateErrorIfCandidateExists( + traceForProvideDelegate, call, provideDelegateResults, byExpression ) if (shouldCommitTrace) { diff --git a/compiler/testData/diagnostics/tests/delegatedProperty/missedGetter.kt b/compiler/testData/diagnostics/tests/delegatedProperty/missedGetter.kt index 709bd40dd92..71f08682264 100644 --- a/compiler/testData/diagnostics/tests/delegatedProperty/missedGetter.kt +++ b/compiler/testData/diagnostics/tests/delegatedProperty/missedGetter.kt @@ -1,3 +1,3 @@ -val a: Int by )", "A")!>A() +val a: Int by )", "A", "delegate")!>A() class A diff --git a/compiler/testData/diagnostics/tests/delegatedProperty/missedSetter.kt b/compiler/testData/diagnostics/tests/delegatedProperty/missedSetter.kt index 44fe82801cb..e47ac927b04 100644 --- a/compiler/testData/diagnostics/tests/delegatedProperty/missedSetter.kt +++ b/compiler/testData/diagnostics/tests/delegatedProperty/missedSetter.kt @@ -2,7 +2,7 @@ import kotlin.reflect.KProperty -var a: Int by , Int)", "A")!>A() +var a: Int by , Int)", "A", "delegate for var (read-write property)")!>A() class A { operator fun getValue(t: Any?, p: KProperty<*>): Int { diff --git a/idea/testData/quickfix/autoImports/delegateExtensionBoth.test b/idea/testData/quickfix/autoImports/delegateExtensionBoth.test index 8a4bb7306e0..1b9357b165a 100644 --- a/idea/testData/quickfix/autoImports/delegateExtensionBoth.test +++ b/idea/testData/quickfix/autoImports/delegateExtensionBoth.test @@ -1,7 +1,7 @@ // FILE: first.before.kt // "Import" "true" -// ERROR: Missing 'getValue(BigTest, KProperty<*>)' method on delegate of type 'DelegateImpl' -// ERROR: Missing 'setValue(BigTest, KProperty<*>, [ERROR : Type from delegate])' method on delegate of type 'DelegateImpl' +// ERROR: Type 'DelegateImpl' has no method 'getValue(BigTest, KProperty<*>)' and thus it cannot serve as a delegate +// ERROR: Type 'DelegateImpl' has no method 'setValue(BigTest, KProperty<*>, [ERROR : Type from delegate])' and thus it cannot serve as a delegate for var (read-write property) package testing @@ -27,8 +27,8 @@ public operator fun DelegateImpl.setValue(thisRef: Any, property: KProper // FILE: first.after.kt // "Import" "true" -// ERROR: Missing 'getValue(BigTest, KProperty<*>)' method on delegate of type 'DelegateImpl' -// ERROR: Missing 'setValue(BigTest, KProperty<*>, [ERROR : Type from delegate])' method on delegate of type 'DelegateImpl' +// ERROR: Type 'DelegateImpl' has no method 'getValue(BigTest, KProperty<*>)' and thus it cannot serve as a delegate +// ERROR: Type 'DelegateImpl' has no method 'setValue(BigTest, KProperty<*>, [ERROR : Type from delegate])' and thus it cannot serve as a delegate for var (read-write property) package testing diff --git a/idea/testData/quickfix/autoImports/delegateExtensionGet.test b/idea/testData/quickfix/autoImports/delegateExtensionGet.test index a45a182d308..35fa3bf9fa2 100644 --- a/idea/testData/quickfix/autoImports/delegateExtensionGet.test +++ b/idea/testData/quickfix/autoImports/delegateExtensionGet.test @@ -1,6 +1,6 @@ // FILE: first.before.kt // "Import" "true" -// ERROR: Missing 'getValue(BigTest, KProperty<*>)' method on delegate of type 'DelegateImpl' +// ERROR: Type 'DelegateImpl' has no method 'getValue(BigTest, KProperty<*>)' and thus it cannot serve as a delegate package testing @@ -27,7 +27,7 @@ public operator fun DelegateImpl.getValue(thisRef: Any?, property: KPrope // FILE: first.after.kt // "Import" "true" -// ERROR: Missing 'getValue(BigTest, KProperty<*>)' method on delegate of type 'DelegateImpl' +// ERROR: Type 'DelegateImpl' has no method 'getValue(BigTest, KProperty<*>)' and thus it cannot serve as a delegate package testing diff --git a/idea/testData/quickfix/autoImports/delegateExtensionSet.test b/idea/testData/quickfix/autoImports/delegateExtensionSet.test index 7a8e051940d..8df3cac3db4 100644 --- a/idea/testData/quickfix/autoImports/delegateExtensionSet.test +++ b/idea/testData/quickfix/autoImports/delegateExtensionSet.test @@ -1,6 +1,6 @@ // FILE: first.before.kt // "Import" "true" -// ERROR: Missing 'setValue(BigTest, KProperty<*>, Int)' method on delegate of type 'DelegateImpl' +// ERROR: Type 'DelegateImpl' has no method 'setValue(BigTest, KProperty<*>, Int)' and thus it cannot serve as a delegate for var (read-write property) package testing @@ -28,7 +28,7 @@ operator fun DelegateImpl.setValue(thisRef: Any, property: KProperty<*>, // FILE: first.after.kt // "Import" "true" -// ERROR: Missing 'setValue(BigTest, KProperty<*>, Int)' method on delegate of type 'DelegateImpl' +// ERROR: Type 'DelegateImpl' has no method 'setValue(BigTest, KProperty<*>, Int)' and thus it cannot serve as a delegate for var (read-write property) package testing diff --git a/idea/testData/quickfix/autoImports/delegateNoOperator.test b/idea/testData/quickfix/autoImports/delegateNoOperator.test index ff6116101c4..e346ef8ec5f 100644 --- a/idea/testData/quickfix/autoImports/delegateNoOperator.test +++ b/idea/testData/quickfix/autoImports/delegateNoOperator.test @@ -1,6 +1,6 @@ // FILE: first.before.kt // "class org.jetbrains.kotlin.idea.quickfix.ImportFix" "false" -// ERROR: Missing 'getValue(Nothing?, KProperty<*>)' method on delegate of type 'String' +// ERROR: Type 'String' has no method 'getValue(Nothing?, KProperty<*>)' and thus it cannot serve as a delegate // ACTION: To raw string literal package b diff --git a/idea/testData/quickfix/variables/changeMutability/canBeVal/delegatedProperty2.kt b/idea/testData/quickfix/variables/changeMutability/canBeVal/delegatedProperty2.kt index e2c385cf4d4..1e92ac942a1 100644 --- a/idea/testData/quickfix/variables/changeMutability/canBeVal/delegatedProperty2.kt +++ b/idea/testData/quickfix/variables/changeMutability/canBeVal/delegatedProperty2.kt @@ -2,7 +2,7 @@ // ACTION: Create extension function 'Delegate.getValue' // ACTION: Create member function 'Delegate.getValue' // ACTION: Introduce import alias -// ERROR: Missing 'getValue(Nothing?, KProperty<*>)' method on delegate of type 'Delegate' +// ERROR: Type 'Delegate' has no method 'getValue(Nothing?, KProperty<*>)' and thus it cannot serve as a delegate import kotlin.reflect.KProperty fun test() { diff --git a/idea/testData/quickfix/variables/changeMutability/canBeVal/delegatedProperty3.kt b/idea/testData/quickfix/variables/changeMutability/canBeVal/delegatedProperty3.kt index 4775198cee1..124536d1350 100644 --- a/idea/testData/quickfix/variables/changeMutability/canBeVal/delegatedProperty3.kt +++ b/idea/testData/quickfix/variables/changeMutability/canBeVal/delegatedProperty3.kt @@ -2,8 +2,8 @@ // ACTION: Create extension function 'Delegate.getValue', function 'Delegate.setValue' // ACTION: Create member function 'Delegate.getValue', function 'Delegate.setValue' // ACTION: Introduce import alias -// ERROR: Missing 'getValue(Nothing?, KProperty<*>)' method on delegate of type 'Delegate' -// ERROR: Missing 'setValue(Nothing?, KProperty<*>, String)' method on delegate of type 'Delegate' +// ERROR: Type 'Delegate' has no method 'getValue(Nothing?, KProperty<*>)' and thus it cannot serve as a delegate +// ERROR: Type 'Delegate' has no method 'setValue(Nothing?, KProperty<*>, String)' and thus it cannot serve as a delegate for var (read-write property) import kotlin.reflect.KProperty fun test() {