diff --git a/compiler/backend-common/src/bridges/impl.kt b/compiler/backend-common/src/bridges/impl.kt index 71a51f3ab54..adb0ccd45b8 100644 --- a/compiler/backend-common/src/bridges/impl.kt +++ b/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 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) { 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 e286a56da51..5ce56b962c1 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java +++ b/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 - .descriptorToDeclaration(trace.getBindingContext(), fromCurrent); - trace.report(Errors.CONFLICTING_OVERLOADS.on(declaration, fromCurrent, fromCurrent.getContainingDeclaration().getName().asString())); + JetDeclaration declaration = + (JetDeclaration) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), fromCurrent); + //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) { - trace.report(MANY_IMPL_MEMBER_NOT_IMPLEMENTED.on(nameIdentifier, klass, memberDescriptor)); - break; + if (!manyImpl.isEmpty()) { + trace.report(MANY_IMPL_MEMBER_NOT_IMPLEMENTED.on(nameIdentifier, klass, manyImpl.iterator().next())); } - - if (classDescriptor.getModality() == Modality.ABSTRACT) { - return; - } - - for (CallableMemberDescriptor memberDescriptor : abstractNoImpl) { - trace.report(ABSTRACT_MEMBER_NOT_IMPLEMENTED.on(nameIdentifier, klass, memberDescriptor)); - break; + if (classDescriptor.getModality() != Modality.ABSTRACT && !abstractNoImpl.isEmpty()) { + trace.report(ABSTRACT_MEMBER_NOT_IMPLEMENTED.on(nameIdentifier, klass, abstractNoImpl.iterator().next())); } } public static void collectMissingImplementations( - MutableClassDescriptor classDescriptor, Set abstractNoImpl, Set manyImpl + @NotNull ClassDescriptor classDescriptor, + @NotNull Set abstractNoImpl, + @NotNull Set manyImpl ) { - for (CallableMemberDescriptor descriptor : classDescriptor.getAllCallableMembers()) { - collectMissingImplementations(descriptor, abstractNoImpl, manyImpl); - } - } - - public static void collectMissingImplementations( - ClassDescriptor classDescriptor, Set abstractNoImpl, Set manyImpl - ) { - Iterator callableMembers = KotlinPackage.filterIsInstance( - classDescriptor.getDefaultType().getMemberScope().getAllDescriptors().iterator(), CallableMemberDescriptor.class - ); - while (callableMembers.hasNext()) { - collectMissingImplementations(callableMembers.next(), abstractNoImpl, manyImpl); + for (DeclarationDescriptor member : classDescriptor.getDefaultType().getMemberScope().getAllDescriptors()) { + if (member instanceof CallableMemberDescriptor) { + collectMissingImplementations((CallableMemberDescriptor) member, abstractNoImpl, manyImpl); + } } } @@ -398,14 +384,9 @@ public class OverrideResolver { ) { Map> overriddenDeclarationsByDirectParent = Maps.newLinkedHashMap(); for (CallableMemberDescriptor descriptor : directOverriddenDescriptors) { - Collection overriddenDeclarations = OverridingUtil.getOverriddenDeclarations(descriptor); - Set filteredOverrides = OverridingUtil.filterOutOverridden( - Sets.newLinkedHashSet(overriddenDeclarations)); - Set overridden = Sets.newLinkedHashSet(); - for (CallableMemberDescriptor memberDescriptor : filteredOverrides) { - overridden.add(memberDescriptor); - } - overriddenDeclarationsByDirectParent.put(descriptor, overridden); + Set overriddenDeclarations = OverridingUtil.getOverriddenDeclarations(descriptor); + Set filteredOverrides = OverridingUtil.filterOutOverridden(overriddenDeclarations); + overriddenDeclarationsByDirectParent.put(descriptor, new LinkedHashSet(filteredOverrides)); } 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) { - CallableMemberDescriptor invisibleOverride = null; - outer: + @Nullable + private static CallableMemberDescriptor findInvisibleOverriddenDescriptor( + @NotNull CallableMemberDescriptor declared, + @NotNull ClassDescriptor declaringClass + ) { for (JetType supertype : declaringClass.getTypeConstructor().getSupertypes()) { Set 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; - if (!noDeclaration) { + boolean isDeclaration = declared.getKind() == CallableMemberDescriptor.Kind.DECLARATION; + 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 = - 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; + int defaultsInSuper = 0; for (ValueParameterDescriptor parameterFromSuperclass : parameterFromSubclass.getOverriddenDescriptors()) { if (parameterFromSuperclass.declaresDefaultValue()) { - if (!superWithDefault) { - 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; - } + defaultsInSuper++; } + } + boolean multipleDefaultsInSuper = defaultsInSuper > 1; - DeclarationDescriptor superFunction = parameterFromSuperclass.getContainingDeclaration(); - if (declared.hasStableParameterNames() && - superFunction instanceof CallableDescriptor && ((CallableDescriptor) superFunction).hasStableParameterNames() && - !parameterFromSuperclass.getName().equals(parameterFromSubclass.getName())) { - if (noDeclaration) { - 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)); - } - } + if (isDeclaration) { + checkNameAndDefaultForDeclaredParameter(parameterFromSubclass, multipleDefaultsInSuper); + } + else { + checkNameAndDefaultForFakeOverrideParameter(declared, parameterFromSubclass, multipleDefaultsInSuper); } } } - private boolean checkPropertyKind(CallableMemberDescriptor descriptor, boolean isVar) { - if (descriptor instanceof PropertyDescriptor) { - PropertyDescriptor propertyDescriptor = (PropertyDescriptor) descriptor; - return propertyDescriptor.isVar() == isVar; + private void checkNameAndDefaultForDeclaredParameter(@NotNull ValueParameterDescriptor descriptor, boolean multipleDefaultsInSuper) { + JetParameter parameter = (JetParameter) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), descriptor); + assert parameter != null : "Declaration not found for parameter: " + descriptor; + + 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) { diff --git a/compiler/testData/diagnostics/tests/override/MultipleDefaultsAndNamesInSupertypes.kt b/compiler/testData/diagnostics/tests/override/MultipleDefaultsAndNamesInSupertypes.kt new file mode 100644 index 00000000000..d163ec07dde --- /dev/null +++ b/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 Z : A, B(), C \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java index b356c8e0b9d..ac528d3ede0 100644 --- a/compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java +++ b/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"); diff --git a/core/descriptors/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java b/core/descriptors/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java index 65a5b44654b..25d9a634929 100644 --- a/core/descriptors/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java +++ b/core/descriptors/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java @@ -55,11 +55,13 @@ public class OverridingUtil { @NotNull public static Set filterOutOverridden(@NotNull Set candidateSet) { + //noinspection unchecked return filterOverrides(candidateSet, Function.ID, Filtering.RETAIN_OVERRIDING); } @NotNull public static Set filterOutOverriding(@NotNull Set candidateSet) { + //noinspection unchecked return filterOverrides(candidateSet, Function.ID, Filtering.RETAIN_OVERRIDDEN); } @@ -293,48 +295,49 @@ public class OverridingUtil { Map 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(), - new TypeProjectionImpl(subTypeParameter.getDefaultType())); + superTypeParameters.get(i).getTypeConstructor(), + 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 false; - } - - return true; + return !superDescriptor.isVar() || typeChecker.equalTypes(subDescriptor.getType(), substitutedSuperReturnType); } /** - * Get overridden descriptors that are declarations or delegations. - * - * @see CallableMemberDescriptor.Kind#isReal() + * Get overridden descriptors that are declarations, delegations or synthesized and are not dominated by any other such descriptor */ @NotNull - public static Collection getOverriddenDeclarations(@NotNull CallableMemberDescriptor descriptor) { - Map result = Maps.newHashMap(); + public static Set getOverriddenDeclarations(@NotNull CallableMemberDescriptor descriptor) { + Set result = new LinkedHashSet(); getOverriddenDeclarations(descriptor, result); - return result.values(); + return result; } - private static void getOverriddenDeclarations(CallableMemberDescriptor descriptor, Map r) { + private static void getOverriddenDeclarations( + @NotNull CallableMemberDescriptor descriptor, + @NotNull Set 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); } } }