From 56d1184515a355d2195184ac693752c4ebd8fcf1 Mon Sep 17 00:00:00 2001 From: Mikhael Bogdanov Date: Tue, 10 Apr 2018 12:13:24 +0200 Subject: [PATCH] Restrict conditions for @JvmDefault super calls Condition was too strict if @JvmDefault implicitly hidden by class in inheritance. Diagnostic is redundant cause there isn't breaking change depending on interface method generation strategy and delegating stub generation in inheriting class. --- .../resolve/jvm/checkers/JvmDefaultChecker.kt | 24 +++++--------- .../jvmDefault/jvmDefaultInInheritance.kt | 21 ++++++++++-- .../jvmDefault/jvmDefaultInInheritance.txt | 32 +++++++++++++++++++ .../annotations/jvmDefault/superCall.kt | 4 +-- .../jvmDefault/superCallAmbiguity.kt | 5 +++ .../jvmDefault/superCallAmbiguity.txt | 14 ++++++++ .../jvmDefault/superCallAmbiguity2.kt | 14 ++++++-- .../jvmDefault/superCallAmbiguity2.txt | 23 +++++++++++++ .../jvmDefault/superCallAmbiguity3.kt | 12 +++++-- .../jvmDefault/superCallAmbiguity3.txt | 23 +++++++++++++ 10 files changed, 148 insertions(+), 24 deletions(-) diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmDefaultChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmDefaultChecker.kt index 016b9affb28..b8cd61c92af 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmDefaultChecker.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmDefaultChecker.kt @@ -12,12 +12,12 @@ import org.jetbrains.kotlin.load.java.descriptors.JavaMethodDescriptor import org.jetbrains.kotlin.psi.KtDeclaration import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils import org.jetbrains.kotlin.resolve.DescriptorUtils +import org.jetbrains.kotlin.resolve.DescriptorUtils.isInterface import org.jetbrains.kotlin.resolve.OverridingUtil import org.jetbrains.kotlin.resolve.annotations.JVM_DEFAULT_FQ_NAME import org.jetbrains.kotlin.resolve.annotations.hasJvmDefaultAnnotation import org.jetbrains.kotlin.resolve.checkers.DeclarationChecker import org.jetbrains.kotlin.resolve.checkers.DeclarationCheckerContext -import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperInterfaces import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm class JvmDefaultChecker(val jvmTarget: JvmTarget) : DeclarationChecker { @@ -42,7 +42,7 @@ class JvmDefaultChecker(val jvmTarget: JvmTarget) : DeclarationChecker { descriptor.unsubstitutedMemberScope.getContributedDescriptors().filterIsInstance().any { it.kind.isReal && it.hasJvmDefaultAnnotation() } - if (!hasDeclaredJvmDefaults && !checkJvmDefaultThroughInheritance(descriptor, enableJvmDefault)) { + if (!hasDeclaredJvmDefaults && !checkJvmDefaultsInHierarchy(descriptor, enableJvmDefault)) { context.trace.report(ErrorsJvm.JVM_DEFAULT_THROUGH_INHERITANCE.on(declaration)) } } @@ -65,25 +65,17 @@ class JvmDefaultChecker(val jvmTarget: JvmTarget) : DeclarationChecker { } } - private fun checkJvmDefaultThroughInheritance(descriptor: DeclarationDescriptor, enableJvmDefault: Boolean): Boolean { + private fun checkJvmDefaultsInHierarchy(descriptor: DeclarationDescriptor, enableJvmDefault: Boolean): Boolean { if (enableJvmDefault) return true if (descriptor !is ClassDescriptor) return true - if (!DescriptorUtils.isInterface(descriptor) && - !DescriptorUtils.isAnnotationClass(descriptor) - ) { - return descriptor.getSuperInterfaces().all { - checkJvmDefaultAnnotation(it) + return descriptor.unsubstitutedMemberScope.getContributedDescriptors().filterIsInstance() + .all { memberDescriptor -> + memberDescriptor.kind.isReal || OverridingUtil.filterOutOverridden(memberDescriptor.overriddenDescriptors.toSet()).all { + !isInterface(it.containingDeclaration) || !it.hasJvmDefaultAnnotation() || it.modality == Modality.ABSTRACT + } } - } - - return checkJvmDefaultAnnotation(descriptor) } - private fun checkJvmDefaultAnnotation(descriptor: ClassDescriptor): Boolean { - return descriptor.unsubstitutedMemberScope.getContributedDescriptors().filterIsInstance().all { - !it.hasJvmDefaultAnnotation() - } - } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/jvmDefaultInInheritance.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/jvmDefaultInInheritance.kt index d9fddee354f..79046057cb9 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/jvmDefaultInInheritance.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/jvmDefaultInInheritance.kt @@ -14,6 +14,23 @@ interface B : A { open class Foo : B open class Foo2 : B, A +open class FooNoError : B { + override fun test() { + } +} +open class Foo2NoError : B, A { + override fun test() { + } +} + class Bar : Foo() -class Bar2 : Foo(), A -class Bar3 : Foo(), B \ No newline at end of file +class Bar2 : Foo(), A +class Bar3 : Foo(), B + +open class BarWithJvmDefault : B { + @JvmDefault + override fun test() { + } +} + +class BarWithJvmDefaultSuper: BarWithJvmDefault() \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/jvmDefaultInInheritance.txt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/jvmDefaultInInheritance.txt index 303acf5ac9c..6b3e3adcff9 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/jvmDefaultInInheritance.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/jvmDefaultInInheritance.txt @@ -38,6 +38,22 @@ public final class Bar3 : Foo, B { public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String } +public open class BarWithJvmDefault : B { + public constructor BarWithJvmDefault() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + @kotlin.jvm.JvmDefault public open override /*1*/ fun test(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class BarWithJvmDefaultSuper : BarWithJvmDefault { + public constructor BarWithJvmDefaultSuper() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + @kotlin.jvm.JvmDefault public open override /*1*/ /*fake_override*/ fun test(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + public open class Foo : B { public constructor Foo() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean @@ -53,3 +69,19 @@ public open class Foo2 : B, A { @kotlin.jvm.JvmDefault public open override /*2*/ /*fake_override*/ fun test(): kotlin.Unit public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String } + +public open class Foo2NoError : B, A { + public constructor Foo2NoError() + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ fun test(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + +public open class FooNoError : B { + public constructor FooNoError() + 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*/ fun test(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCall.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCall.kt index 0bf239995dc..3193f62b307 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCall.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCall.kt @@ -18,7 +18,7 @@ interface C : B { } } -open class Foo : B { +open class Foo : B { override fun test() { super.test() } @@ -37,7 +37,7 @@ open class Bar2 : Bar() { } } -class ManySupers: Foo2(), B { +class ManySupers: Foo2(), B { fun foo() { super.test() super<B>.test() diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity.kt index dc8b2fdfdf2..a4c01edb548 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity.kt @@ -10,6 +10,11 @@ interface B{ } } +interface AB: A, B + +interface BA: B, A + + interface C : A, B { @JvmDefault override fun test() { diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity.txt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity.txt index 6335028d81c..e566b4d40c0 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity.txt @@ -7,6 +7,13 @@ public interface A { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } +public interface AB : A, B { + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + @kotlin.jvm.JvmDefault public open override /*2*/ /*fake_override*/ fun test(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + public interface B { public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int @@ -14,6 +21,13 @@ public interface B { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } +public interface BA : B, A { + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun test(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + public interface C : A, B { public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity2.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity2.kt index 6192ae36cb6..88da42e2aed 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity2.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity2.kt @@ -9,14 +9,24 @@ interface B{ } } -class C : A, B { +interface AB: A, B + +interface BA: B, A + +class C : A, B { override fun test() { super.test() } } -class D : B, A { +class D : B, A { override fun test() { super.test() } +} + +class E: B, A { + fun foo() { + super.test() + } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity2.txt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity2.txt index 2b75896cc3b..ac40e6522ae 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity2.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity2.txt @@ -7,6 +7,13 @@ public interface A { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } +public interface AB : A, B { + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + @kotlin.jvm.JvmDefault public open override /*2*/ /*fake_override*/ fun test(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + public interface B { public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int @@ -14,6 +21,13 @@ public interface B { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } +public interface BA : B, A { + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun test(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + public final class C : A, B { public constructor C() public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean @@ -29,3 +43,12 @@ public final class D : B, A { public open override /*2*/ fun test(): kotlin.Unit public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String } + +public final class E : B, A { + public constructor E() + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final fun foo(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun test(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity3.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity3.kt index f3c5a4a7eef..774d347ef4b 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity3.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity3.kt @@ -9,15 +9,23 @@ interface A { interface B{ fun test() } +interface AB : A, B +interface BA : B, A -class C : A, B { +class C : A, B { override fun test() { super.test() } } -class D : B, A { +class D : B, A { override fun test() { super.test() } +} + +class E: B, A { + fun foo() { + super.test() + } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity3.txt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity3.txt index 45860d8aeed..6ad3e5834c2 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity3.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmDefault/superCallAmbiguity3.txt @@ -7,6 +7,13 @@ public interface A { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } +public interface AB : A, B { + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + @kotlin.jvm.JvmDefault public open override /*2*/ /*fake_override*/ fun test(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + public interface B { public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int @@ -14,6 +21,13 @@ public interface B { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } +public interface BA : B, A { + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun test(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + public final class C : A, B { public constructor C() public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean @@ -29,3 +43,12 @@ public final class D : B, A { public open override /*2*/ fun test(): kotlin.Unit public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String } + +public final class E : B, A { + public constructor E() + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final fun foo(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun test(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +}