From 48fe9fb2c0a75dc469c62a701f31bf352caa496a Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 4 Jul 2016 15:28:52 +0300 Subject: [PATCH] Fix AssertionError in OverrideResolver on bad hierarchy Previously the code was operating under the assumption that if the implementation of some function (both the implementation and the function come from supertypes) does not have a proper return type (the one which is a subtype of return type of all declarations of this function in the supertypes), then there's necessarily at least one abstract declaration of the function, such that the implementation's return type is not a subtype of the return type of that declaration. The assertion makes sense when the hierarchy above the current class does not have any errors: we should report at least one function as being "not implemented" in the current class. However, as demonstrated by the test case, if there's an error already in the supertypes with regard to overridability of members, this assertion may be wrong. Reporting the "not implemented" error in such case is in fact not necessary because of the already existing error ("return type mismatch" in the test) in the supertypes #KT-12482 Fixed --- .../kotlin/resolve/OverrideResolver.java | 17 +++++----- .../diagnostics/tests/override/kt12482.kt | 11 +++++++ .../diagnostics/tests/override/kt12482.txt | 31 +++++++++++++++++++ .../checkers/DiagnosticsTestGenerated.java | 6 ++++ 4 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/override/kt12482.kt create mode 100644 compiler/testData/diagnostics/tests/override/kt12482.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java index e3d9374af1a..ae25e8dcf21 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverrideResolver.java @@ -28,6 +28,7 @@ import com.intellij.util.containers.MultiMap; import com.intellij.util.containers.SmartHashSet; import com.intellij.util.containers.hash.EqualityPolicy; import kotlin.Unit; +import kotlin.collections.CollectionsKt; import kotlin.jvm.functions.Function1; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -543,6 +544,7 @@ public class OverrideResolver { int numImplementations = implementations.size(); + // The most common case: there's one implementation in the supertypes with the matching return type if (numImplementations == 1 && isReturnTypeOkForOverride(descriptor, implementations.get(0))) return; List abstractOverridden = new ArrayList(allFilteredOverriddenDeclarations.size()); @@ -673,17 +675,14 @@ public class OverrideResolver { @NotNull private static List collectAbstractMethodsWithMoreSpecificReturnType( @NotNull List abstractOverridden, - @NotNull CallableMemberDescriptor implementation + @NotNull final CallableMemberDescriptor implementation ) { - List result = new ArrayList(abstractOverridden.size()); - for (CallableMemberDescriptor abstractMember : abstractOverridden) { - if (!isReturnTypeOkForOverride(abstractMember, implementation)) { - result.add(abstractMember); + return CollectionsKt.filter(abstractOverridden, new Function1() { + @Override + public Boolean invoke(@NotNull CallableMemberDescriptor abstractMember) { + return !isReturnTypeOkForOverride(abstractMember, implementation); } - } - assert !result.isEmpty() : "Implementation (" + implementation + ") doesn't have the most specific type, " + - "but none of the other overridden methods does either: " + abstractOverridden; - return result; + }); } @NotNull diff --git a/compiler/testData/diagnostics/tests/override/kt12482.kt b/compiler/testData/diagnostics/tests/override/kt12482.kt new file mode 100644 index 00000000000..88e62deef89 --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/kt12482.kt @@ -0,0 +1,11 @@ +interface A { + fun test(): String = "A" +} + +interface B : A { + override fun test(): Unit = "B" +} + +open class C : A + +class D : C(), B diff --git a/compiler/testData/diagnostics/tests/override/kt12482.txt b/compiler/testData/diagnostics/tests/override/kt12482.txt new file mode 100644 index 00000000000..03104983ee7 --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/kt12482.txt @@ -0,0 +1,31 @@ +package + +public interface A { + 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 fun test(): kotlin.String + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public interface B : A { + 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 +} + +public open class C : A { + public constructor C() + 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 test(): kotlin.String + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class D : C, B { + public constructor D() + 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.String + public open override /*2*/ /*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 3dd2d147ee7..267de57f91f 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -12753,6 +12753,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("kt12482.kt") + public void testKt12482() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/override/kt12482.kt"); + doTest(fileName); + } + @TestMetadata("kt1862.kt") public void testKt1862() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/override/kt1862.kt");