From 03358c61d41dbe7afe2e3fb128b1b0b0879fd729 Mon Sep 17 00:00:00 2001 From: Mikhail Zarechenskiy Date: Sun, 31 May 2020 19:04:38 +0300 Subject: [PATCH] Add compatibility warning for SAM conversions --- .../jetbrains/kotlin/diagnostics/Errors.java | 1 + .../rendering/DefaultErrorMessages.java | 2 ++ .../DiagnosticReporterByTrackingStrategy.kt | 3 +++ .../calls/tower/NewResolutionOldInference.kt | 4 +++ .../components/CallableReferenceResolution.kt | 5 ++++ .../calls/model/KotlinCallDiagnostics.kt | 6 +++++ .../calls/model/ResolutionCandidate.kt | 6 +++++ .../resolve/calls/tower/TowerResolver.kt | 27 ++++++++++++++++--- ...solveToOuterScopeForKotlinFunctions.fir.kt | 10 +++++++ ...tyResolveToOuterScopeForKotlinFunctions.kt | 18 ++++++++++--- ...yResolveToOuterScopeForKotlinFunctions.txt | 10 +++++++ 11 files changed, 85 insertions(+), 7 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 7efe975c324..09f3590390b 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -761,6 +761,7 @@ public interface Errors { DiagnosticFactory1 REIFIED_TYPE_UNSAFE_SUBSTITUTION = DiagnosticFactory1.create(WARNING); DiagnosticFactory0 CANDIDATE_CHOSEN_USING_OVERLOAD_RESOLUTION_BY_LAMBDA_ANNOTATION = DiagnosticFactory0.create(WARNING); + DiagnosticFactory0 COMPATIBILITY_WARNING = DiagnosticFactory0.create(WARNING); DiagnosticFactory3 RESOLUTION_TO_CLASSIFIER = 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 6701fab2edf..7a08f6d9c0a 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -898,6 +898,8 @@ public class DefaultErrorMessages { MAP.put(TYPE_PARAMETERS_NOT_ALLOWED, "Type parameters are not allowed here"); MAP.put(CANDIDATE_CHOSEN_USING_OVERLOAD_RESOLUTION_BY_LAMBDA_ANNOTATION, "Candidate was choosen only by @OverloadResolutionByLambdaReturnType annotation"); + MAP.put(COMPATIBILITY_WARNING, "Candidate was chosen to preserve compatibility"); + MAP.put(TYPE_PARAMETER_OF_PROPERTY_NOT_USED_IN_RECEIVER, "Type parameter of a property must be used in its receiver type"); MAP.put(SUPERTYPES_FOR_ANNOTATION_CLASS, "Annotation class cannot have supertypes"); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/DiagnosticReporterByTrackingStrategy.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/DiagnosticReporterByTrackingStrategy.kt index ab0e80c5bf6..62b63747dea 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/DiagnosticReporterByTrackingStrategy.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/DiagnosticReporterByTrackingStrategy.kt @@ -80,6 +80,9 @@ class DiagnosticReporterByTrackingStrategy( CandidateChosenUsingOverloadResolutionByLambdaAnnotation::class.java -> { trace.report(CANDIDATE_CHOSEN_USING_OVERLOAD_RESOLUTION_BY_LAMBDA_ANNOTATION.on(psiKotlinCall.psiCall.callElement)) } + CompatibilityWarning::class.java -> { + trace.report(COMPATIBILITY_WARNING.on(psiKotlinCall.psiCall.callElement)) + } } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tower/NewResolutionOldInference.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tower/NewResolutionOldInference.kt index 578ad57381c..3ee71ac61df 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tower/NewResolutionOldInference.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tower/NewResolutionOldInference.kt @@ -422,6 +422,10 @@ class NewResolutionOldInference( getResultApplicability(diagnostics) } + override fun addCompatibilityWarning(other: Candidate) { + // Only applicable for new inference + } + override val isSuccessful = getResultApplicability(eagerDiagnostics).isSuccess } diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/CallableReferenceResolution.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/CallableReferenceResolution.kt index ad9abcdf1e8..f8f7e3a15a0 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/CallableReferenceResolution.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/CallableReferenceResolution.kt @@ -63,6 +63,11 @@ class CallableReferenceCandidate( val diagnostics: List ) : Candidate { override val resultingApplicability = getResultApplicability(diagnostics) + + override fun addCompatibilityWarning(other: Candidate) { + // TODO: now only arguments can be converted, so CR candidates shouldn't be affected + } + override val isSuccessful get() = resultingApplicability.isSuccess var freshSubstitutor: FreshVariableNewTypeSubstitutor? = null diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/model/KotlinCallDiagnostics.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/model/KotlinCallDiagnostics.kt index 48876cf5fe5..7839e537fd4 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/model/KotlinCallDiagnostics.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/model/KotlinCallDiagnostics.kt @@ -249,3 +249,9 @@ class CandidateChosenUsingOverloadResolutionByLambdaAnnotation : KotlinCallDiagn reporter.onCall(this) } } + +class CompatibilityWarning : KotlinCallDiagnostic(RESOLVED) { + override fun report(reporter: DiagnosticReporter) { + reporter.onCall(this) + } +} \ No newline at end of file diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/model/ResolutionCandidate.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/model/ResolutionCandidate.kt index 669325aeda3..49575725bd4 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/model/ResolutionCandidate.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/model/ResolutionCandidate.kt @@ -165,6 +165,12 @@ class KotlinResolutionCandidate( return maxOf(currentApplicability, systemApplicability, variableApplicability) } + override fun addCompatibilityWarning(other: Candidate) { + if (this !== other) { + addDiagnostic(CompatibilityWarning()) + } + } + override fun toString(): String { val descriptor = DescriptorRenderer.COMPACT.render(resolvedCall.candidateDescriptor) val okOrFail = if (currentApplicability.isSuccess) "OK" else "FAIL" diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/tower/TowerResolver.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/tower/TowerResolver.kt index de983b8a772..bda1e6b06fa 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/tower/TowerResolver.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/tower/TowerResolver.kt @@ -35,6 +35,8 @@ interface Candidate { val isSuccessful: Boolean val resultingApplicability: ResolutionCandidateApplicability + + fun addCompatibilityWarning(other: Candidate) } interface CandidateFactory { @@ -310,9 +312,25 @@ class TowerResolver { override fun getSuccessfulCandidates(): Collection? { if (!isSuccessful) return null - val firstGroupWithResolved = candidateGroups.firstOrNull { - it.any(::isSuccessfulCandidate) - } ?: return null + var compatibilityCandidate: C? = null + var firstGroupWithResolved: Collection? = null + outer@ for (group in candidateGroups) { + for (candidate in group) { + if (isSuccessfulCandidate(candidate)) { + firstGroupWithResolved = group + break@outer + } + + if (compatibilityCandidate == null && isSuccessfulPreserveCompatibility(candidate)) { + compatibilityCandidate = candidate + } + } + } + + if (firstGroupWithResolved == null) return null + if (compatibilityCandidate != null) { + firstGroupWithResolved.forEach { it.addCompatibilityWarning(compatibilityCandidate) } + } return firstGroupWithResolved.filter(::isSuccessfulCandidate) } @@ -322,6 +340,9 @@ class TowerResolver { || candidate.resultingApplicability == ResolutionCandidateApplicability.RESOLVED_WITH_ERROR } + private fun isSuccessfulPreserveCompatibility(candidate: C): Boolean = + candidate.resultingApplicability == ResolutionCandidateApplicability.RESOLVED_NEED_PRESERVE_COMPATIBILITY + override fun pushCandidates(candidates: Collection) { val thereIsSuccessful = candidates.any { it.isSuccessful } if (!isSuccessful && !thereIsSuccessful) { diff --git a/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.fir.kt b/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.fir.kt index d1faa64bd45..72c8bb283de 100644 --- a/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.fir.kt +++ b/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.fir.kt @@ -1,5 +1,15 @@ // !DIAGNOSTICS: -UNUSED_PARAMETER -UNUSED_EXPRESSION +object Test0 { + fun foo(f: Runnable): Int = 0 + fun foo(x: () -> String): String = "" + + fun test(f: () -> Unit) { + val result = foo(f) + result + } +} + object Test1 { fun foo(x: Any) {} fun foo(f: () -> Unit) {} diff --git a/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.kt b/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.kt index d1faa64bd45..088aa864d72 100644 --- a/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.kt +++ b/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.kt @@ -1,5 +1,15 @@ // !DIAGNOSTICS: -UNUSED_PARAMETER -UNUSED_EXPRESSION +object Test0 { + fun foo(f: Runnable): Int = 0 + fun foo(x: () -> String): String = "" + + fun test(f: () -> Unit) { + val result = foo(f) + result + } +} + object Test1 { fun foo(x: Any) {} fun foo(f: () -> Unit) {} @@ -8,7 +18,7 @@ object Test1 { fun foo(r: Runnable) {} fun test(f: () -> Unit) { - foo(f) + foo(f) } } } @@ -30,7 +40,7 @@ object Test3 { fun foo(n: Number, f: () -> Unit): String = "" fun test(f: () -> Unit) { - val result = foo(1, f) + val result = foo(1, f) result } } @@ -42,7 +52,7 @@ object Test4 { fun bar() {} fun test() { - val result = foo(1, ::bar) + val result = foo(1, ::bar) result } } @@ -55,7 +65,7 @@ object Test5 { fun foo(r: Runnable) {} fun test() { - foo { } + foo { } } } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.txt b/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.txt index ebf4fd0b184..d68b901386c 100644 --- a/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.txt +++ b/compiler/testData/diagnostics/tests/j+k/sam/compatibilityResolveToOuterScopeForKotlinFunctions.txt @@ -1,5 +1,15 @@ package +public object Test0 { + private constructor Test0() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final fun foo(/*0*/ x: () -> kotlin.String): kotlin.String + public final fun foo(/*0*/ f: java.lang.Runnable): kotlin.Int + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final fun test(/*0*/ f: () -> kotlin.Unit): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + public object Test1 { private constructor Test1() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean