From 058e4b833bdfc45002c8fe79cda831bed1193234 Mon Sep 17 00:00:00 2001 From: Andrey Breslav Date: Thu, 26 Apr 2012 17:32:37 +0400 Subject: [PATCH] Checks for default parameter value consistency Inheriting default values from multiple superclasses is prohibited --- .../ValueParameterDescriptorImpl.java | 2 +- .../jet/lang/diagnostics/Errors.java | 2 + .../rendering/DefaultErrorMessages.java | 5 ++ .../jet/lang/psi/JetClassOrObject.java | 3 +- .../jet/lang/resolve/OverrideResolver.java | 69 +++++++++++++++++-- ...NoErrorsWhenInheritingFromOneTypeTwice.jet | 15 ++++ .../MultipleDefaultParametersInSupertypes.jet | 15 ++++ ...efaultParametersInSupertypesNoOverride.jet | 15 ++++ ...DefaultsInSupertypesNoExplicitOverride.jet | 10 +++ ...alues-DefaultValueFromOnlyOneSupertype.jet | 11 +++ 10 files changed, 138 insertions(+), 9 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/override/DefaultParameterValues-NoErrorsWhenInheritingFromOneTypeTwice.jet create mode 100644 compiler/testData/diagnostics/tests/override/MultipleDefaultParametersInSupertypes.jet create mode 100644 compiler/testData/diagnostics/tests/override/MultipleDefaultParametersInSupertypesNoOverride.jet create mode 100644 compiler/testData/diagnostics/tests/override/MultipleDefaultsInSupertypesNoExplicitOverride.jet create mode 100644 compiler/testData/diagnostics/tests/override/ParameterDefaultValues-DefaultValueFromOnlyOneSupertype.jet diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/ValueParameterDescriptorImpl.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/ValueParameterDescriptorImpl.java index 9928db66858..766b544c73c 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/ValueParameterDescriptorImpl.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/ValueParameterDescriptorImpl.java @@ -36,7 +36,7 @@ public class ValueParameterDescriptorImpl extends VariableDescriptorImpl impleme private final boolean isVar; private final int index; private final ValueParameterDescriptor original; - private final Set overriddenDescriptors = Sets.newHashSet(); + private final Set overriddenDescriptors = Sets.newLinkedHashSet(); // Linked is essential public ValueParameterDescriptorImpl( @NotNull DeclarationDescriptor containingDeclaration, diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java index add31fcb58d..582cd6708e9 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java @@ -381,6 +381,8 @@ public interface Errors { DiagnosticFactory2.create(ERROR); SimpleDiagnosticFactory DEFAULT_VALUE_NOT_ALLOWED_IN_OVERRIDE = SimpleDiagnosticFactory.create(ERROR, PositioningStrategies.PARAMETER_DEFAULT_VALUE); + DiagnosticFactory1 MULTIPLE_DEFAULTS_INHERITED_FROM_SUPERTYPES = DiagnosticFactory1.create(ERROR, PositioningStrategies.POSITION_NAME_IDENTIFIER); + DiagnosticFactory1 MULTIPLE_DEFAULTS_INHERITED_FROM_SUPERTYPES_WHEN_NO_EXPLICIT_OVERRIDE = DiagnosticFactory1.create(ERROR, PositioningStrategies.POSITION_NAME_IDENTIFIER); DiagnosticFactory2 CONFLICTING_OVERLOADS = DiagnosticFactory2.create(ERROR, new PositioningStrategy() { diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java index 4b8507b453a..b70a76a378c 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java @@ -386,6 +386,11 @@ public class DefaultErrorMessages { MAP.put(NOT_AN_ANNOTATION_CLASS, "''{0}'' is not an annotation class", TO_STRING); MAP.put(DEFAULT_VALUE_NOT_ALLOWED_IN_OVERRIDE, "An overriding function is not allowed to specify default values for its parameters"); + MAP.put(MULTIPLE_DEFAULTS_INHERITED_FROM_SUPERTYPES, "More than one overridden descriptor declares a default value for ''{0}''. " + + "As the compiler can not make sure these values agree, this is not allowed.", TO_STRING); + + MAP.put(MULTIPLE_DEFAULTS_INHERITED_FROM_SUPERTYPES_WHEN_NO_EXPLICIT_OVERRIDE, "More than one overridden descriptor declares a default value for ''{0}''. " + + "As the compiler can not make sure these values agree, this is not allowed.", TO_STRING); MAP.setImmutable(); } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetClassOrObject.java b/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetClassOrObject.java index 1110c6630b4..42e0d6d1239 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetClassOrObject.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetClassOrObject.java @@ -17,6 +17,7 @@ package org.jetbrains.jet.lang.psi; import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiNameIdentifierOwner; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -25,7 +26,7 @@ import java.util.List; /** * @author max */ -public interface JetClassOrObject extends PsiElement { +public interface JetClassOrObject extends PsiElement, PsiNameIdentifierOwner { List getDeclarations(); @Nullable 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 cf9631b08cb..b18efcc0383 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java @@ -16,7 +16,10 @@ package org.jetbrains.jet.lang.resolve; -import com.google.common.collect.*; +import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.Lists; +import com.google.common.collect.Multimap; +import com.google.common.collect.Sets; import com.intellij.lang.ASTNode; import com.intellij.psi.PsiElement; import com.intellij.util.containers.LinkedMultiMap; @@ -80,6 +83,20 @@ public class OverrideResolver { checkVisibility(); checkOverrides(invisibleOverriddenDescriptors); + checkParameterOverrides(); + } + + private void checkParameterOverrides() { + List allClasses = Lists.newArrayList(context.getClasses().values()); + allClasses.addAll(context.getObjects().values()); + for (MutableClassDescriptor classDescriptor : allClasses) { + Collection members = classDescriptor.getCallableMembers(); + for (CallableMemberDescriptor member : members) { + //if (member.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE) { + checkOverridesForParameters(member); + //} + } + } } /** @@ -394,10 +411,6 @@ public class OverrideResolver { } } - if (hasOverrideModifier) { - checkOverridesForParameters(declared); - } - if (hasOverrideModifier && declared.getOverriddenDescriptors().size() == 0) { if (!invisibleOverriddenDescriptors.get(declared).isEmpty()) { CallableDescriptor descriptor = invisibleOverriddenDescriptors.values().iterator().next(); @@ -415,6 +428,16 @@ public class OverrideResolver { } private void checkOverridesForParameters(CallableMemberDescriptor declared) { + boolean fakeOverride = declared.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE; + if (!fakeOverride) { + // No check if the function is not marked as 'override' + JetModifierListOwner declaration = + (JetModifierListOwner) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), declared); + if (!declaration.hasModifier(JetTokens.OVERRIDE_KEYWORD)) { + return; + } + } + // Let p1 be a parameter of the overriding function // Let p2 be a parameter of the function being overridden // Then @@ -422,10 +445,42 @@ public class OverrideResolver { // b) p1 must have the same name as p2 for (ValueParameterDescriptor parameterFromSubclass : declared.getValueParameters()) { JetParameter parameter = - (JetParameter) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), parameterFromSubclass); - if (parameterFromSubclass.hasDefaultValue()) { + fakeOverride ? null : + (JetParameter) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), parameterFromSubclass); + + if (parameterFromSubclass.hasDefaultValue() && !fakeOverride) { trace.report(DEFAULT_VALUE_NOT_ALLOWED_IN_OVERRIDE.on(parameter)); } + + // If p1 overrides p2 and p3 and p2 overrides p3, we remove p2 from p1's overridden list + Set uniqueOverridden = Sets.newHashSet(parameterFromSubclass.getOverriddenDescriptors()); + for (ValueParameterDescriptor p2 : parameterFromSubclass.getOverriddenDescriptors()) { + for (ValueParameterDescriptor p3 : p2.getOverriddenDescriptors()) { + if (uniqueOverridden.contains(p3)) { + uniqueOverridden.remove(p2); + } + } + } + + boolean superWithDefault = false; + for (ValueParameterDescriptor parameterFromSuperclass : uniqueOverridden) { + if (parameterFromSuperclass.hasDefaultValue()) { + if (!superWithDefault) { + superWithDefault = true; + } + else { + if (fakeOverride) { + JetClassOrObject classElement = (JetClassOrObject) BindingContextUtils + .descriptorToDeclaration(trace.getBindingContext(), declared.getContainingDeclaration()); + 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; + } + } + } } } diff --git a/compiler/testData/diagnostics/tests/override/DefaultParameterValues-NoErrorsWhenInheritingFromOneTypeTwice.jet b/compiler/testData/diagnostics/tests/override/DefaultParameterValues-NoErrorsWhenInheritingFromOneTypeTwice.jet new file mode 100644 index 00000000000..d6d0b98d07d --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/DefaultParameterValues-NoErrorsWhenInheritingFromOneTypeTwice.jet @@ -0,0 +1,15 @@ +trait Y { + fun foo(a : Int = 1) +} + +trait YSub : Y { + +} + +class Z2 : Y, YSub { + override fun foo(a : Int) {} +} + +class Z2O : Y, YSub { + override fun foo(a : Int) {} +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/override/MultipleDefaultParametersInSupertypes.jet b/compiler/testData/diagnostics/tests/override/MultipleDefaultParametersInSupertypes.jet new file mode 100644 index 00000000000..69057c4aecd --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/MultipleDefaultParametersInSupertypes.jet @@ -0,0 +1,15 @@ +trait X { + fun foo(a : Int = 1) +} + +trait Y { + fun foo(a : Int = 1) +} + +class Z : X, Y { + override fun foo(a : Int) {} +} + +object ZO : X, Y { + override fun foo(a : Int) {} +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/override/MultipleDefaultParametersInSupertypesNoOverride.jet b/compiler/testData/diagnostics/tests/override/MultipleDefaultParametersInSupertypesNoOverride.jet new file mode 100644 index 00000000000..647f92f359f --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/MultipleDefaultParametersInSupertypesNoOverride.jet @@ -0,0 +1,15 @@ +trait X { + fun foo(a : Int = 1) +} + +trait Y { + fun foo(a : Int = 1) +} + +class Z : X, Y { + fun foo(a : Int) {} +} + +object ZO : X, Y { + fun foo(a : Int) {} +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/override/MultipleDefaultsInSupertypesNoExplicitOverride.jet b/compiler/testData/diagnostics/tests/override/MultipleDefaultsInSupertypesNoExplicitOverride.jet new file mode 100644 index 00000000000..ab30bf1408e --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/MultipleDefaultsInSupertypesNoExplicitOverride.jet @@ -0,0 +1,10 @@ +trait X { + fun foo(a : Int = 1) +} + +trait Y { + fun foo(a : Int = 1) +} + +class Z1 : X, Y {} // BUG +object Z1O : X, Y {} // BUG diff --git a/compiler/testData/diagnostics/tests/override/ParameterDefaultValues-DefaultValueFromOnlyOneSupertype.jet b/compiler/testData/diagnostics/tests/override/ParameterDefaultValues-DefaultValueFromOnlyOneSupertype.jet new file mode 100644 index 00000000000..9624f096d3c --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/ParameterDefaultValues-DefaultValueFromOnlyOneSupertype.jet @@ -0,0 +1,11 @@ +trait X { + fun foo(a : Int = 1) +} + +trait Y { + fun foo(a : Int) +} + +class Z : X, Y { + override fun foo(a : Int) {} +} \ No newline at end of file