Refactor OverrideResolver and OverridingUtil

Fix warnings, simplify code. Untangle a very complex code related to
diagnostics on multiple default values and different names of parameters in
supertypes; fix a bug there, add test

compiler/backend-common/src/bridges/impl.kt

@@ -21,7 +21,6 @@ import org.jetbrains.jet.lang.resolve.DescriptorUtils
 import org.jetbrains.jet.lang.resolve.OverridingUtil
 import org.jetbrains.jet.lang.resolve.calls.CallResolverUtil
 import org.jetbrains.jet.lang.types.TypeUtils
-import java.util.LinkedHashSet
 
 public fun <Signature> generateBridgesForFunctionDescriptor(
         descriptor: FunctionDescriptor,
@@ -74,7 +73,7 @@ public fun findTraitImplementation(descriptor: CallableMemberDescriptor): Callab
     // TODO: this logic is quite common for bridge generation, find a way to abstract it to a single place
     // TODO: don't use filterOutOverridden() here, it's an internal front-end utility (see its implementation)
     val overriddenDeclarations = OverridingUtil.getOverriddenDeclarations(descriptor)
-    val filteredOverriddenDeclarations = OverridingUtil.filterOutOverridden(LinkedHashSet(overriddenDeclarations))
+    val filteredOverriddenDeclarations = OverridingUtil.filterOutOverridden(overriddenDeclarations)
 
     var implementation: CallableMemberDescriptor? = null
     for (overriddenDeclaration in filteredOverriddenDeclarations) {

compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java

@@ -24,12 +24,11 @@ import com.intellij.psi.PsiElement;
 import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.LinkedMultiMap;
 import com.intellij.util.containers.MultiMap;
-import kotlin.KotlinPackage;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.lang.descriptors.*;
 import org.jetbrains.jet.lang.descriptors.annotations.AnnotationDescriptor;
 import org.jetbrains.jet.lang.descriptors.impl.MutableClassDescriptor;
-import org.jetbrains.jet.lang.diagnostics.Errors;
 import org.jetbrains.jet.lang.psi.*;
 import org.jetbrains.jet.lang.resolve.calls.CallResolverUtil;
 import org.jetbrains.jet.lang.resolve.name.Name;
@@ -150,9 +149,10 @@ public class OverrideResolver {
 
                         @Override
                         public void conflict(@NotNull CallableMemberDescriptor fromSuper, @NotNull CallableMemberDescriptor fromCurrent) {
-                            JetDeclaration declaration = (JetDeclaration) BindingContextUtils
+                            JetDeclaration declaration =
-                                    .descriptorToDeclaration(trace.getBindingContext(), fromCurrent);
+                                    (JetDeclaration) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), fromCurrent);
-                            trace.report(Errors.CONFLICTING_OVERLOADS.on(declaration, fromCurrent, fromCurrent.getContainingDeclaration().getName().asString()));
+                            //noinspection ConstantConditions
+                            trace.report(CONFLICTING_OVERLOADS.on(declaration, fromCurrent, fromCurrent.getContainingDeclaration().getName().asString()));
                         }
                     });
         }
@@ -238,38 +238,24 @@ public class OverrideResolver {
         }
         if (nameIdentifier == null) return;
 
-        for (CallableMemberDescriptor memberDescriptor : manyImpl) {
+        if (!manyImpl.isEmpty()) {
-            trace.report(MANY_IMPL_MEMBER_NOT_IMPLEMENTED.on(nameIdentifier, klass, memberDescriptor));
+            trace.report(MANY_IMPL_MEMBER_NOT_IMPLEMENTED.on(nameIdentifier, klass, manyImpl.iterator().next()));
-            break;
         }
 
-
+        if (classDescriptor.getModality() != Modality.ABSTRACT && !abstractNoImpl.isEmpty()) {
-        if (classDescriptor.getModality() == Modality.ABSTRACT) {
+            trace.report(ABSTRACT_MEMBER_NOT_IMPLEMENTED.on(nameIdentifier, klass, abstractNoImpl.iterator().next()));
-            return;
-        }
-
-        for (CallableMemberDescriptor memberDescriptor : abstractNoImpl) {
-            trace.report(ABSTRACT_MEMBER_NOT_IMPLEMENTED.on(nameIdentifier, klass, memberDescriptor));
-            break;
         }
     }
 
     public static void collectMissingImplementations(
-            MutableClassDescriptor classDescriptor, Set<CallableMemberDescriptor> abstractNoImpl, Set<CallableMemberDescriptor> manyImpl
+            @NotNull ClassDescriptor classDescriptor,
+            @NotNull Set<CallableMemberDescriptor> abstractNoImpl,
+            @NotNull Set<CallableMemberDescriptor> manyImpl
     ) {
-        for (CallableMemberDescriptor descriptor : classDescriptor.getAllCallableMembers()) {
+        for (DeclarationDescriptor member : classDescriptor.getDefaultType().getMemberScope().getAllDescriptors()) {
-            collectMissingImplementations(descriptor, abstractNoImpl, manyImpl);
+            if (member instanceof CallableMemberDescriptor) {
-        }
+                collectMissingImplementations((CallableMemberDescriptor) member, abstractNoImpl, manyImpl);
-    }
+            }
-
-    public static void collectMissingImplementations(
-            ClassDescriptor classDescriptor, Set<CallableMemberDescriptor> abstractNoImpl, Set<CallableMemberDescriptor> manyImpl
-    ) {
-        Iterator<CallableMemberDescriptor> callableMembers = KotlinPackage.filterIsInstance(
-                classDescriptor.getDefaultType().getMemberScope().getAllDescriptors().iterator(), CallableMemberDescriptor.class
-        );
-        while (callableMembers.hasNext()) {
-            collectMissingImplementations(callableMembers.next(), abstractNoImpl, manyImpl);
         }
     }
 
@@ -398,14 +384,9 @@ public class OverrideResolver {
     ) {
         Map<CallableMemberDescriptor, Set<CallableMemberDescriptor>> overriddenDeclarationsByDirectParent = Maps.newLinkedHashMap();
         for (CallableMemberDescriptor descriptor : directOverriddenDescriptors) {
-            Collection<CallableMemberDescriptor> overriddenDeclarations = OverridingUtil.getOverriddenDeclarations(descriptor);
+            Set<CallableMemberDescriptor> overriddenDeclarations = OverridingUtil.getOverriddenDeclarations(descriptor);
-            Set<CallableMemberDescriptor> filteredOverrides = OverridingUtil.filterOutOverridden(
+            Set<CallableMemberDescriptor> filteredOverrides = OverridingUtil.filterOutOverridden(overriddenDeclarations);
-                    Sets.newLinkedHashSet(overriddenDeclarations));
+            overriddenDeclarationsByDirectParent.put(descriptor, new LinkedHashSet<CallableMemberDescriptor>(filteredOverrides));
-            Set<CallableMemberDescriptor> overridden = Sets.newLinkedHashSet();
-            for (CallableMemberDescriptor memberDescriptor : filteredOverrides) {
-                overridden.add(memberDescriptor);
-            }
-            overriddenDeclarationsByDirectParent.put(descriptor, overridden);
         }
         return overriddenDeclarationsByDirectParent;
     }
@@ -501,7 +482,7 @@ public class OverrideResolver {
         }
     }
 
-    private void checkOverridesForMemberMarkedOverride(
+    private static void checkOverridesForMemberMarkedOverride(
             @NotNull CallableMemberDescriptor declared,
             boolean checkIfOverridesNothing,
             @NotNull CheckOverrideReportStrategy reportError
@@ -599,24 +580,27 @@ public class OverrideResolver {
                                               annotation);
     }
 
-    private CallableMemberDescriptor findInvisibleOverriddenDescriptor(CallableMemberDescriptor declared, ClassDescriptor declaringClass) {
+    @Nullable
-        CallableMemberDescriptor invisibleOverride = null;
+    private static CallableMemberDescriptor findInvisibleOverriddenDescriptor(
-        outer:
+            @NotNull CallableMemberDescriptor declared,
+            @NotNull ClassDescriptor declaringClass
+    ) {
         for (JetType supertype : declaringClass.getTypeConstructor().getSupertypes()) {
             Set<CallableMemberDescriptor> all = Sets.newLinkedHashSet();
             all.addAll(supertype.getMemberScope().getFunctions(declared.getName()));
+            //noinspection unchecked
             all.addAll((Collection) supertype.getMemberScope().getProperties(declared.getName()));
             for (CallableMemberDescriptor fromSuper : all) {
                 if (OverridingUtil.isOverridableBy(fromSuper, declared).getResult() == OVERRIDABLE) {
-                    invisibleOverride = fromSuper;
                     if (Visibilities.isVisible(fromSuper, declared)) {
-                        throw new IllegalStateException("Descriptor " + fromSuper + " is overridable by " + declared + " and visible but does not appear in its getOverriddenDescriptors()");
+                        throw new IllegalStateException("Descriptor " + fromSuper + " is overridable by " + declared +
+                                                        " and visible but does not appear in its getOverriddenDescriptors()");
                     }
-                    break outer;
+                    return fromSuper;
                 }
             }
         }
-        return invisibleOverride;
+        return null;
     }
 
     private void checkParameterOverridesForAllClasses(@NotNull TopDownAnalysisContext c) {
@@ -630,12 +614,12 @@ public class OverrideResolver {
     }
 
     private void checkOverridesForParameters(@NotNull CallableMemberDescriptor declared) {
-        boolean noDeclaration = declared.getKind() != CallableMemberDescriptor.Kind.DECLARATION;
+        boolean isDeclaration = declared.getKind() == CallableMemberDescriptor.Kind.DECLARATION;
-        if (!noDeclaration) {
+        if (isDeclaration) {
             // No check if the function is not marked as 'override'
             JetModifierListOwner declaration =
                     (JetModifierListOwner) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), declared);
-            if (!declaration.hasModifier(JetTokens.OVERRIDE_KEYWORD)) {
+            if (declaration != null && !declaration.hasModifier(JetTokens.OVERRIDE_KEYWORD)) {
                 return;
             }
         }
@@ -646,55 +630,85 @@ public class OverrideResolver {
         //  a) p1 is not allowed to have a default value declared
         //  b) p1 must have the same name as p2
         for (ValueParameterDescriptor parameterFromSubclass : declared.getValueParameters()) {
-            JetParameter parameter =
+            int defaultsInSuper = 0;
-                    noDeclaration ? null :
-                            (JetParameter) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), parameterFromSubclass);
-
-            JetClassOrObject classElement = noDeclaration ? (JetClassOrObject) BindingContextUtils
-                    .descriptorToDeclaration(trace.getBindingContext(), declared.getContainingDeclaration()) : null;
-
-            if (parameterFromSubclass.declaresDefaultValue() && !noDeclaration) {
-                trace.report(DEFAULT_VALUE_NOT_ALLOWED_IN_OVERRIDE.on(parameter));
-            }
-
-            boolean superWithDefault = false;
             for (ValueParameterDescriptor parameterFromSuperclass : parameterFromSubclass.getOverriddenDescriptors()) {
                 if (parameterFromSuperclass.declaresDefaultValue()) {
-                    if (!superWithDefault) {
+                    defaultsInSuper++;
-                        superWithDefault = true;
-                    }
-                    else {
-                        if (noDeclaration) {
-                            trace.report(MULTIPLE_DEFAULTS_INHERITED_FROM_SUPERTYPES_WHEN_NO_EXPLICIT_OVERRIDE.on(classElement, parameterFromSubclass));
-                        }
-                        else {
-                            trace.report(MULTIPLE_DEFAULTS_INHERITED_FROM_SUPERTYPES.on(parameter, parameterFromSubclass));
-                        }
-                        break;
-                    }
                 }
+            }
+            boolean multipleDefaultsInSuper = defaultsInSuper > 1;
 
-                DeclarationDescriptor superFunction = parameterFromSuperclass.getContainingDeclaration();
+            if (isDeclaration) {
-                if (declared.hasStableParameterNames() &&
+                checkNameAndDefaultForDeclaredParameter(parameterFromSubclass, multipleDefaultsInSuper);
-                    superFunction instanceof CallableDescriptor && ((CallableDescriptor) superFunction).hasStableParameterNames() &&
+            }
-                    !parameterFromSuperclass.getName().equals(parameterFromSubclass.getName())) {
+            else {
-                    if (noDeclaration) {
+                checkNameAndDefaultForFakeOverrideParameter(declared, parameterFromSubclass, multipleDefaultsInSuper);
-                        trace.report(DIFFERENT_NAMES_FOR_THE_SAME_PARAMETER_IN_SUPERTYPES.on(classElement, declared.getOverriddenDescriptors(), parameterFromSuperclass.getIndex() + 1));
-                    }
-                    else {
-                        trace.report(PARAMETER_NAME_CHANGED_ON_OVERRIDE.on(parameter, (ClassDescriptor) superFunction.getContainingDeclaration(), parameterFromSuperclass));
-                    }
-                }
             }
         }
     }
 
-    private boolean checkPropertyKind(CallableMemberDescriptor descriptor, boolean isVar) {
+    private void checkNameAndDefaultForDeclaredParameter(@NotNull ValueParameterDescriptor descriptor, boolean multipleDefaultsInSuper) {
-        if (descriptor instanceof PropertyDescriptor) {
+        JetParameter parameter = (JetParameter) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), descriptor);
-            PropertyDescriptor propertyDescriptor = (PropertyDescriptor) descriptor;
+        assert parameter != null : "Declaration not found for parameter: " + descriptor;
-            return propertyDescriptor.isVar() == isVar;
+
+        if (descriptor.declaresDefaultValue()) {
+            trace.report(DEFAULT_VALUE_NOT_ALLOWED_IN_OVERRIDE.on(parameter));
         }
-        return false;
+
+        if (multipleDefaultsInSuper) {
+            trace.report(MULTIPLE_DEFAULTS_INHERITED_FROM_SUPERTYPES.on(parameter, descriptor));
+        }
+
+        for (ValueParameterDescriptor parameterFromSuperclass : descriptor.getOverriddenDescriptors()) {
+            if (shouldReportParameterNameOverrideWarning(descriptor, parameterFromSuperclass)) {
+                //noinspection ConstantConditions
+                trace.report(PARAMETER_NAME_CHANGED_ON_OVERRIDE.on(
+                        parameter,
+                        (ClassDescriptor) parameterFromSuperclass.getContainingDeclaration().getContainingDeclaration(),
+                        parameterFromSuperclass)
+                );
+            }
+        }
+    }
+
+    private void checkNameAndDefaultForFakeOverrideParameter(
+            @NotNull CallableMemberDescriptor containingFunction,
+            @NotNull ValueParameterDescriptor descriptor,
+            boolean multipleDefaultsInSuper
+    ) {
+        DeclarationDescriptor containingClass = containingFunction.getContainingDeclaration();
+        JetClassOrObject classElement =
+                (JetClassOrObject) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), containingClass);
+        assert classElement != null : "Declaration not found for class: " + containingClass;
+
+        if (multipleDefaultsInSuper) {
+            trace.report(MULTIPLE_DEFAULTS_INHERITED_FROM_SUPERTYPES_WHEN_NO_EXPLICIT_OVERRIDE.on(classElement, descriptor));
+        }
+
+        for (ValueParameterDescriptor parameterFromSuperclass : descriptor.getOverriddenDescriptors()) {
+            if (shouldReportParameterNameOverrideWarning(descriptor, parameterFromSuperclass)) {
+                trace.report(DIFFERENT_NAMES_FOR_THE_SAME_PARAMETER_IN_SUPERTYPES.on(
+                        classElement,
+                        containingFunction.getOverriddenDescriptors(),
+                        parameterFromSuperclass.getIndex() + 1)
+                );
+            }
+        }
+    }
+
+    private static boolean shouldReportParameterNameOverrideWarning(
+            @NotNull ValueParameterDescriptor parameterFromSubclass,
+            @NotNull ValueParameterDescriptor parameterFromSuperclass
+    ) {
+        DeclarationDescriptor subFunction = parameterFromSubclass.getContainingDeclaration();
+        DeclarationDescriptor superFunction = parameterFromSuperclass.getContainingDeclaration();
+        return subFunction instanceof CallableDescriptor && ((CallableDescriptor) subFunction).hasStableParameterNames() &&
+               superFunction instanceof CallableDescriptor && ((CallableDescriptor) superFunction).hasStableParameterNames() &&
+               !parameterFromSuperclass.getName().equals(parameterFromSubclass.getName());
+    }
+
+    private static boolean checkPropertyKind(@NotNull CallableMemberDescriptor descriptor, boolean isVar) {
+        return descriptor instanceof PropertyDescriptor && ((PropertyDescriptor) descriptor).isVar() == isVar;
     }
 
     private void checkVisibility(@NotNull TopDownAnalysisContext c) {

compiler/testData/diagnostics/tests/override/MultipleDefaultsAndNamesInSupertypes.kt

@@ -0,0 +1,13 @@
+trait A {
+    fun foo(x: Int = 42): Int
+}
+
+open class B {
+    fun foo(x: Int = 239) = x
+}
+
+trait C {
+    fun foo(y: Int)
+}
+
+class <!MULTIPLE_DEFAULTS_INHERITED_FROM_SUPERTYPES_WHEN_NO_EXPLICIT_OVERRIDE, DIFFERENT_NAMES_FOR_THE_SAME_PARAMETER_IN_SUPERTYPES!>Z<!> : A, B(), C

compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java

@@ -5404,6 +5404,11 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
                 doTest("compiler/testData/diagnostics/tests/override/MultipleDefaultParametersInSupertypesNoOverride.kt");
             }
             
+            @TestMetadata("MultipleDefaultsAndNamesInSupertypes.kt")
+            public void testMultipleDefaultsAndNamesInSupertypes() throws Exception {
+                doTest("compiler/testData/diagnostics/tests/override/MultipleDefaultsAndNamesInSupertypes.kt");
+            }
+            
             @TestMetadata("MultipleDefaultsInSupertypesNoExplicitOverride.kt")
             public void testMultipleDefaultsInSupertypesNoExplicitOverride() throws Exception {
                 doTest("compiler/testData/diagnostics/tests/override/MultipleDefaultsInSupertypesNoExplicitOverride.kt");

core/descriptors/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java

@@ -55,11 +55,13 @@ public class OverridingUtil {
 
     @NotNull
     public static <D extends CallableDescriptor> Set<D> filterOutOverridden(@NotNull Set<D> candidateSet) {
+        //noinspection unchecked
         return filterOverrides(candidateSet, Function.ID, Filtering.RETAIN_OVERRIDING);
     }
 
     @NotNull
     public static <D> Set<D> filterOutOverriding(@NotNull Set<D> candidateSet) {
+        //noinspection unchecked
         return filterOverrides(candidateSet, Function.ID, Filtering.RETAIN_OVERRIDDEN);
     }
 
@@ -293,48 +295,49 @@ public class OverridingUtil {
 
         Map<TypeConstructor, TypeProjection> substitutionContext = Maps.newHashMap();
         for (int i = 0; i < superTypeParameters.size(); i++) {
-            TypeParameterDescriptor superTypeParameter = superTypeParameters.get(i);
-            TypeParameterDescriptor subTypeParameter = subTypeParameters.get(i);
             substitutionContext.put(
-                    superTypeParameter.getTypeConstructor(),
+                    superTypeParameters.get(i).getTypeConstructor(),
-                    new TypeProjectionImpl(subTypeParameter.getDefaultType()));
+                    new TypeProjectionImpl(subTypeParameters.get(i).getDefaultType())
+            );
         }
         return TypeSubstitutor.create(substitutionContext);
     }
 
-    public static boolean isPropertyTypeOkForOverride(@NotNull JetTypeChecker typeChecker, @NotNull PropertyDescriptor superDescriptor, @NotNull PropertyDescriptor subDescriptor) {
+    public static boolean isPropertyTypeOkForOverride(
+            @NotNull JetTypeChecker typeChecker,
+            @NotNull PropertyDescriptor superDescriptor,
+            @NotNull PropertyDescriptor subDescriptor
+    ) {
         TypeSubstitutor typeSubstitutor = prepareTypeSubstitutor(superDescriptor, subDescriptor);
-        JetType substitutedSuperReturnType = typeSubstitutor.substitute(superDescriptor.getReturnType(), Variance.OUT_VARIANCE);
+        if (typeSubstitutor == null) return false;
+        JetType substitutedSuperReturnType = typeSubstitutor.substitute(superDescriptor.getType(), Variance.OUT_VARIANCE);
         assert substitutedSuperReturnType != null;
-        if (superDescriptor.isVar() && !typeChecker.equalTypes(subDescriptor.getReturnType(), substitutedSuperReturnType)) {
+        return !superDescriptor.isVar() || typeChecker.equalTypes(subDescriptor.getType(), substitutedSuperReturnType);
-            return false;
-        }
-
-        return true;
     }
 
     /**
-     * Get overridden descriptors that are declarations or delegations.
+     * Get overridden descriptors that are declarations, delegations or synthesized and are not dominated by any other such descriptor
-     *
-     * @see CallableMemberDescriptor.Kind#isReal()
      */
     @NotNull
-    public static Collection<CallableMemberDescriptor> getOverriddenDeclarations(@NotNull CallableMemberDescriptor descriptor) {
+    public static Set<CallableMemberDescriptor> getOverriddenDeclarations(@NotNull CallableMemberDescriptor descriptor) {
-        Map<ClassDescriptor, CallableMemberDescriptor> result = Maps.newHashMap();
+        Set<CallableMemberDescriptor> result = new LinkedHashSet<CallableMemberDescriptor>();
         getOverriddenDeclarations(descriptor, result);
-        return result.values();
+        return result;
     }
 
-    private static void getOverriddenDeclarations(CallableMemberDescriptor descriptor, Map<ClassDescriptor, CallableMemberDescriptor> r) {
+    private static void getOverriddenDeclarations(
+            @NotNull CallableMemberDescriptor descriptor,
+            @NotNull Set<CallableMemberDescriptor> result
+    ) {
         if (descriptor.getKind().isReal()) {
-            r.put((ClassDescriptor) descriptor.getContainingDeclaration(), descriptor);
+            result.add(descriptor);
         }
         else {
             if (descriptor.getOverriddenDescriptors().isEmpty()) {
                 throw new IllegalStateException("No overridden descriptors found for (fake override) " + descriptor);
             }
             for (CallableMemberDescriptor overridden : descriptor.getOverriddenDescriptors()) {
-                getOverriddenDeclarations(overridden, r);
+                getOverriddenDeclarations(overridden, result);
             }
         }
     }