From 65ddcd0473f86afca00aab968075286017ff472c Mon Sep 17 00:00:00 2001 From: Svetlana Isakova Date: Fri, 10 Aug 2012 12:54:06 +0400 Subject: [PATCH] No repeat 'many impl member not implemented' error in a subclass if there is one in a superclass --- .../jet/lang/resolve/OverrideResolver.java | 140 +++++++++++++++--- .../ParentInheritsManyImplementations.kt | 17 +++ .../checkers/JetDiagnosticsTestGenerated.java | 5 + 3 files changed, 141 insertions(+), 21 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/override/ParentInheritsManyImplementations.kt diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java index 37fbbedce25..3a443f3754b 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java @@ -17,12 +17,10 @@ package org.jetbrains.jet.lang.resolve; import com.google.common.base.Predicate; -import com.google.common.collect.Collections2; -import com.google.common.collect.Lists; -import com.google.common.collect.Multimap; -import com.google.common.collect.Sets; +import com.google.common.collect.*; import com.intellij.lang.ASTNode; import com.intellij.psi.PsiElement; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.LinkedMultiMap; import com.intellij.util.containers.MultiMap; import org.jetbrains.annotations.NotNull; @@ -396,31 +394,131 @@ public class OverrideResolver { @NotNull Set manyImpl ) { if (descriptor.getKind().isReal()) return; + if (descriptor.getVisibility() == Visibilities.INVISIBLE_FAKE) return; - Collection overriddenDeclarations = OverridingUtil.getOverriddenDeclarations(descriptor); - if (overriddenDeclarations.size() == 0) { + Collection directOverridden = descriptor.getOverriddenDescriptors(); + if (directOverridden.size() == 0) { throw new IllegalStateException("A 'fake override' must override something"); } - List nonAbstractManyImpl = Lists.newArrayList(); - Set filteredOverriddenDeclarations = OverridingUtil.filterOverrides(Sets.newHashSet(overriddenDeclarations)); + // collects map from the directly overridden descriptor to the set of declarations: + // -- if directly overridden is not fake, the set consists of one element: this directly overridden + // -- if it's fake, overridden declarations (non-fake) of this descriptor are collected + Map> overriddenDeclarationsByDirectParent = collectOverriddenDeclarations(directOverridden); - boolean allSuperAbstract = true; - for (CallableMemberDescriptor overridden : filteredOverriddenDeclarations) { - if (overridden.getModality() != Modality.ABSTRACT) { - if (descriptor.getVisibility() != Visibilities.INVISIBLE_FAKE) { - nonAbstractManyImpl.add(overridden); - } - allSuperAbstract = false; + List allOverriddenDeclarations = ContainerUtil.flatten(overriddenDeclarationsByDirectParent.values()); + Set allFilteredOverriddenDeclarations = OverridingUtil.filterOverrides(Sets.newHashSet(allOverriddenDeclarations)); + + Set relevantDirectlyOverridden = + getRelevantDirectlyOverridden(overriddenDeclarationsByDirectParent, allFilteredOverriddenDeclarations); + + int implCount = countImplementations(relevantDirectlyOverridden); + if (implCount == 0) { + collectDescriptorsByModality(allFilteredOverriddenDeclarations, abstractNoImpl, Modality.ABSTRACT); + } + else if (implCount > 1) { + collectDescriptorsByModality(allFilteredOverriddenDeclarations, manyImpl, Modality.OPEN, Modality.FINAL); + } + } + + private static int countImplementations(@NotNull Set relevantDirectlyOverridden) { + int implCount = 0; + for (CallableMemberDescriptor overriddenDescriptor : relevantDirectlyOverridden) { + if (overriddenDescriptor.getModality() != Modality.ABSTRACT) { + implCount++; } } - if (nonAbstractManyImpl.size() > 1) { - manyImpl.addAll(nonAbstractManyImpl); - } - else if (allSuperAbstract) { - abstractNoImpl.addAll(overriddenDeclarations); - } + return implCount; + } + private static void collectDescriptorsByModality( + @NotNull Set allOverriddenDeclarations, + @NotNull Set result, + Modality... modalities + ) { + Set modalitySet = Sets.newHashSet(modalities); + for (CallableMemberDescriptor overridden : allOverriddenDeclarations) { + if (modalitySet.contains(overridden.getModality())) { + result.add(overridden); + } + } + } + + @NotNull + private static Set getRelevantDirectlyOverridden( + @NotNull Map> overriddenByParent, + @NotNull Set allFilteredOverriddenDeclarations + ) { + /* Let the following class hierarchy is declared: + + trait A { fun foo() = 1 } + trait B : A + trait C : A + trait D : A { override fun foo() = 2 } + trait E : B, C, D {} + + Traits B and C have fake descriptors for function foo. + The map 'overriddenByParent' is: + { 'foo in B' (fake) -> { 'foo in A' }, 'foo in C' (fake) -> { 'foo in A' }, 'foo in D' -> { 'foo in D'} } + This is a map from directly overridden descriptors (functions 'foo' in B, C, D in this example) to the set of declarations (non-fake), + that are overridden by this descriptor. + + The goal is to leave only relevant directly overridden descriptors to count implementations of our fake function on them. + In the example above there is no error (trait E inherits only one implementation of 'foo' (from D), because this implementation is more precise). + So only 'foo in D' is relevant. + + Directly overridden descriptor is not relevant if it doesn't add any more appropriate non-fake declarations of the concerned function. + More precisely directly overridden descriptor is not relevant if: + - it's declaration set is a subset of declaration set for other directly overridden descriptor + ('foo in B' is not relevant because it's declaration set is a subset of 'foo in C' function's declaration set) + - each member of it's declaration set is overridden by a member of other declaration set + ('foo in C' is not relevant, because 'foo in A' is overridden by 'foo in D', so 'foo in A' is not appropriate non-fake declaration for 'foo') + + For the last condition allFilteredOverriddenDeclarations helps (for the example above it's { 'foo in A' } only): each declaration set + is compared with allFilteredOverriddenDeclarations, if they have no intersection, this means declaration set has only functions that + are overridden by some other function and corresponding directly overridden descriptor is not relevant. + */ + + Map> relevantOverriddenByParent = Maps.newHashMap(overriddenByParent); + + for (Map.Entry> entry : overriddenByParent.entrySet()) { + CallableMemberDescriptor directlyOverridden = entry.getKey(); + Set declarationSet = entry.getValue(); + if (!isRelevant(declarationSet, relevantOverriddenByParent.values(), allFilteredOverriddenDeclarations)) { + relevantOverriddenByParent.remove(directlyOverridden); + } + } + return relevantOverriddenByParent.keySet(); + } + + private static boolean isRelevant( + @NotNull Set declarationSet, + @NotNull Collection> allDeclarationSets, + @NotNull Set allFilteredOverriddenDeclarations + ) { + for (Set otherSet : allDeclarationSets) { + if (otherSet == declarationSet) continue; + if (otherSet.containsAll(declarationSet)) return false; + if (Collections.disjoint(allFilteredOverriddenDeclarations, declarationSet)) return false; + } + return true; + } + + @NotNull + private static Map> collectOverriddenDeclarations( + @NotNull Collection directOverriddenDescriptors + ) { + Map> overriddenDeclarationsByDirectParent = Maps.newHashMap(); + for (CallableMemberDescriptor descriptor : directOverriddenDescriptors) { + Collection overriddenDeclarations = OverridingUtil.getOverriddenDeclarations(descriptor); + Set filteredOverrides = OverridingUtil.filterOverrides(Sets.newHashSet(overriddenDeclarations)); + Set overridden = Sets.newHashSet(); + for (CallableMemberDescriptor memberDescriptor : filteredOverrides) { + overridden.add(memberDescriptor.getOriginal()); + } + overriddenDeclarationsByDirectParent.put(descriptor, overridden); + } + return overriddenDeclarationsByDirectParent; } public static Multimap collectSuperMethods(MutableClassDescriptor classDescriptor) { diff --git a/compiler/testData/diagnostics/tests/override/ParentInheritsManyImplementations.kt b/compiler/testData/diagnostics/tests/override/ParentInheritsManyImplementations.kt new file mode 100644 index 00000000000..7f4c86e780c --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/ParentInheritsManyImplementations.kt @@ -0,0 +1,17 @@ +package d + +trait A { + fun foo() = 1 +} + +trait B { + fun foo() = 2 +} + +open class C : A, B {} + +trait E { + fun foo(): Int +} + +class D : C() {} diff --git a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java index cb83964d342..ddbaa7a0792 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java @@ -1982,6 +1982,11 @@ public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEage doTest("compiler/testData/diagnostics/tests/override/ParameterDefaultValues-DefaultValueFromOnlyOneSupertype.kt"); } + @TestMetadata("ParentInheritsManyImplementations.kt") + public void testParentInheritsManyImplementations() throws Exception { + doTest("compiler/testData/diagnostics/tests/override/ParentInheritsManyImplementations.kt"); + } + @TestMetadata("ProtectedAndPrivateFromSupertypes.kt") public void testProtectedAndPrivateFromSupertypes() throws Exception { doTest("compiler/testData/diagnostics/tests/override/ProtectedAndPrivateFromSupertypes.kt");