diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/CallableMemberDescriptor.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/CallableMemberDescriptor.java index 6fff75b56b7..1b8889a40b3 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/CallableMemberDescriptor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/CallableMemberDescriptor.java @@ -1,7 +1,14 @@ package org.jetbrains.jet.lang.descriptors; +import org.jetbrains.annotations.NotNull; + +import java.util.Set; + /** * @author abreslav */ public interface CallableMemberDescriptor extends CallableDescriptor, MemberDescriptor { + @NotNull + @Override + Set getOverriddenDescriptors(); } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptor.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptor.java index 1547e7d515e..ecfe2a80bc5 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptor.java @@ -8,7 +8,7 @@ import java.util.Set; /** * @author abreslav */ -public interface FunctionDescriptor extends CallableDescriptor, MemberDescriptor { +public interface FunctionDescriptor extends CallableMemberDescriptor { @Override @NotNull DeclarationDescriptor getContainingDeclaration(); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptorImpl.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptorImpl.java index 585348176ad..c087c90fdd5 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptorImpl.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptorImpl.java @@ -20,7 +20,7 @@ import static org.jetbrains.jet.lang.resolve.scopes.receivers.ReceiverDescriptor /** * @author abreslav */ -public class FunctionDescriptorImpl extends DeclarationDescriptorImpl implements FunctionDescriptor, CallableMemberDescriptor { +public class FunctionDescriptorImpl extends DeclarationDescriptorImpl implements FunctionDescriptor { private List typeParameters; private List unsubstitutedValueParameters; diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyDescriptor.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyDescriptor.java index f7afb0c018b..a4c897e05bd 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyDescriptor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyDescriptor.java @@ -166,7 +166,7 @@ public class PropertyDescriptor extends VariableDescriptorImpl implements Callab @NotNull @Override - public Set getOverriddenDescriptors() { + public Set getOverriddenDescriptors() { return overriddenProperties; } } 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 2065ee465e6..addf843e41e 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java @@ -1,6 +1,5 @@ package org.jetbrains.jet.lang.diagnostics; -import com.google.common.collect.Lists; import com.intellij.lang.ASTNode; import com.intellij.openapi.util.TextRange; import com.intellij.psi.PsiElement; @@ -107,7 +106,7 @@ public interface Errors { SimpleDiagnosticFactory BY_IN_SECONDARY_CONSTRUCTOR = SimpleDiagnosticFactory.create(ERROR, "'by'-clause is only supported for primary constructors"); SimpleDiagnosticFactory INITIALIZER_WITH_NO_ARGUMENTS = SimpleDiagnosticFactory.create(ERROR, "Constructor arguments required"); SimpleDiagnosticFactory MANY_CALLS_TO_THIS = SimpleDiagnosticFactory.create(ERROR, "Only one call to 'this(...)' is allowed"); - PsiElementOnlyDiagnosticFactory1 NOTHING_TO_OVERRIDE = PsiElementOnlyDiagnosticFactory1.create(ERROR, "Function {0} overrides nothing"); + PsiElementOnlyDiagnosticFactory1 NOTHING_TO_OVERRIDE = PsiElementOnlyDiagnosticFactory1.create(ERROR, "{0} overrides nothing"); ParameterizedDiagnosticFactory1 PRIMARY_CONSTRUCTOR_MISSING_STATEFUL_PROPERTY = ParameterizedDiagnosticFactory1.create(ERROR, "This class must have a primary constructor, because property {0} has a backing field"); ParameterizedDiagnosticFactory1 PRIMARY_CONSTRUCTOR_MISSING_SUPER_CONSTRUCTOR_CALL = new ParameterizedDiagnosticFactory1(ERROR, "Class {0} must have a constructor in order to be able to initialize supertypes") { @Override @@ -121,7 +120,7 @@ public interface Errors { return e.getClass().getSimpleName() + ": " + e.getMessage(); } }; - PsiElementOnlyDiagnosticFactory3 VIRTUAL_METHOD_HIDDEN = PsiElementOnlyDiagnosticFactory3.create(ERROR, "Function ''{0}'' hides ''{1}'' in class {2} and needs 'override' modifier"); + PsiElementOnlyDiagnosticFactory3 VIRTUAL_MEMBER_HIDDEN = PsiElementOnlyDiagnosticFactory3.create(ERROR, "''{0}'' hides ''{1}'' in class {2} and needs 'override' modifier"); SimpleDiagnosticFactory UNREACHABLE_CODE = SimpleDiagnosticFactory.create(ERROR, "Unreachable code"); ParameterizedDiagnosticFactory1 UNREACHABLE_BECAUSE_OF_NOTHING = ParameterizedDiagnosticFactory1.create(ERROR, "This code is unreachable, because ''{0}'' never terminates normally"); @@ -274,9 +273,10 @@ public interface Errors { return nameExpression.getReferencedName(); } }; - ParameterizedDiagnosticFactory2 OVERRIDING_FINAL_FUNCTION = new ParameterizedDiagnosticFactory2(ERROR, "Method {0} in {1} is final and cannot be overridden") { + ParameterizedDiagnosticFactory2 OVERRIDING_FINAL_MEMBER = new ParameterizedDiagnosticFactory2(ERROR, "{0} in {1} is final and cannot be overridden") { + @Override - protected String makeMessageForA(@NotNull FunctionDescriptor functionDescriptor) { + protected String makeMessageForA(@NotNull CallableMemberDescriptor functionDescriptor) { return functionDescriptor.getName(); } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java index 3048ec1503a..c33a4b7ac11 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java @@ -128,7 +128,7 @@ public class DeclarationResolver { } private void processPrimaryConstructor(MutableClassDescriptor classDescriptor, JetClass klass) { - if (!klass.hasPrimaryConstructor()) return; + if (!klass.hasPrimaryConstructor() && classDescriptor.getKind() != ClassKind.OBJECT) return; if (classDescriptor.getKind() == ClassKind.TRAIT) { // context.getTrace().getErrorHandler().genericError(klass.getPrimaryConstructorParameterList().getNode(), "A trait may not have a constructor"); 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 e2f0e6aa178..de1b8e91298 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java @@ -97,6 +97,7 @@ public class OverrideResolver { } protected void checkOverridesInAClass(MutableClassDescriptor classDescriptor, JetClassOrObject klass) { + // Everything from supertypes Set inheritedFunctions = Sets.newLinkedHashSet(); for (JetType supertype : classDescriptor.getSupertypes()) { for (DeclarationDescriptor descriptor : supertype.getMemberScope().getAllDescriptors()) { @@ -107,8 +108,10 @@ public class OverrideResolver { } } + // Only those actually inherited Set filteredMembers = OverridingUtil.filterOverrides(inheritedFunctions); + // Group members with "the same" signature Multimap factoredMembers = CommonSuppliers.newLinkedHashSetHashSetMultimap(); JetTypeChecker typeChecker = context.getSemanticServices().getTypeChecker(); for (CallableMemberDescriptor one : filteredMembers) { @@ -123,14 +126,11 @@ public class OverrideResolver { } } + // More than one implementation or no implementations at all Set mustBeOverridden = Sets.newLinkedHashSet(); - for (CallableMemberDescriptor key : factoredMembers.keySet()) { Collection mutuallyOverridable = factoredMembers.get(key); -// System.out.println(key); -// println(mutuallyOverridable); - int implementationCount = 0; for (CallableMemberDescriptor member : mutuallyOverridable) { if (member.getModality() != Modality.ABSTRACT) { @@ -142,22 +142,23 @@ public class OverrideResolver { mustBeOverridden.addAll(mutuallyOverridable); } } - + + // Members actually present (declared) in the class Set actuallyOverridden = Sets.newHashSet(); for (FunctionDescriptor declaredFunction : classDescriptor.getFunctions()) { actuallyOverridden.addAll(declaredFunction.getOverriddenDescriptors()); } - for (PropertyDescriptor declaredProperty : classDescriptor.getProperties()) { actuallyOverridden.addAll(declaredProperty.getOverriddenDescriptors()); } + // Those to be overridden that are actually not mustBeOverridden.removeAll(actuallyOverridden); - System.out.println(classDescriptor); - println(mustBeOverridden); - System.out.println("Actually overridden:"); - println(actuallyOverridden); +// System.out.println(classDescriptor); +// println(mustBeOverridden); +// System.out.println("Actually overridden:"); +// println(actuallyOverridden); for (FunctionDescriptor declaredFunction : classDescriptor.getFunctions()) { checkOverrideForFunction(declaredFunction); @@ -208,34 +209,40 @@ public class OverrideResolver { } - private void checkOverrideForFunction(FunctionDescriptor declaredFunction) { - JetFunction function = (JetFunction) context.getTrace().get(BindingContext.DESCRIPTOR_TO_DECLARATION, declaredFunction); - assert function != null; - JetModifierList modifierList = function.getModifierList(); + private void checkOverrideForFunction(CallableMemberDescriptor declared) { + JetNamedDeclaration member = (JetNamedDeclaration) context.getTrace().get(BindingContext.DESCRIPTOR_TO_DECLARATION, declared); + assert member != null; + JetModifierList modifierList = member.getModifierList(); ASTNode overrideNode = modifierList != null ? modifierList.getModifierNode(JetTokens.OVERRIDE_KEYWORD) : null; boolean hasOverrideModifier = overrideNode != null; - boolean foundError = false; + boolean error = false; - for (FunctionDescriptor overridden : declaredFunction.getOverriddenDescriptors()) { + for (CallableMemberDescriptor overridden : declared.getOverriddenDescriptors()) { if (overridden != null) { - if (hasOverrideModifier && !overridden.getModality().isOpen() && !foundError) { -// context.getTrace().getErrorHandler().genericError(overrideNode, "Method " + overridden.getName() + " in " + overridden.getContainingDeclaration().getName() + " is final and cannot be overridden"); - context.getTrace().report(OVERRIDING_FINAL_FUNCTION.on(overrideNode, overridden, overridden.getContainingDeclaration())); - foundError = true; + if (hasOverrideModifier) { + if (!overridden.getModality().isOpen() && !error) { + // context.getTrace().getErrorHandler().genericError(overrideNode, "Method " + overridden.getName() + " in " + overridden.getContainingDeclaration().getName() + " is final and cannot be overridden"); + context.getTrace().report(OVERRIDING_FINAL_MEMBER.on(overrideNode, overridden, overridden.getContainingDeclaration())); + error = true; + } + + if (!OverridingUtil.isReturnTypeOkForOverride(JetTypeChecker.INSTANCE, overridden, declared).isSuccess()) { + + } } } } - if (hasOverrideModifier && declaredFunction.getOverriddenDescriptors().size() == 0) { -// context.getTrace().getErrorHandler().genericError(overrideNode, "Method " + declaredFunction.getName() + " overrides nothing"); - context.getTrace().report(NOTHING_TO_OVERRIDE.on(function, overrideNode, declaredFunction)); + if (hasOverrideModifier && declared.getOverriddenDescriptors().size() == 0) { +// context.getTrace().getErrorHandler().genericError(overrideNode, "Method " + declared.getName() + " overrides nothing"); + context.getTrace().report(NOTHING_TO_OVERRIDE.on(member, overrideNode, declared)); } - PsiElement nameIdentifier = function.getNameIdentifier(); - if (!hasOverrideModifier && declaredFunction.getOverriddenDescriptors().size() > 0 && nameIdentifier != null) { - FunctionDescriptor overriddenFunction = declaredFunction.getOverriddenDescriptors().iterator().next(); + PsiElement nameIdentifier = member.getNameIdentifier(); + if (!hasOverrideModifier && declared.getOverriddenDescriptors().size() > 0 && nameIdentifier != null) { + CallableMemberDescriptor overridden = declared.getOverriddenDescriptors().iterator().next(); // context.getTrace().getErrorHandler().genericError(nameIdentifier.getNode(), -// "Method '" + declaredFunction.getName() + "' overrides method '" + overriddenFunction.getName() + "' in class " + -// overriddenFunction.getContainingDeclaration().getName() + " and needs 'override' modifier"); - context.getTrace().report(VIRTUAL_METHOD_HIDDEN.on(function, nameIdentifier, declaredFunction, overriddenFunction, overriddenFunction.getContainingDeclaration())); +// "Method '" + declared.getName() + "' overrides method '" + overridden.getName() + "' in class " + +// overridden.getContainingDeclaration().getName() + " and needs 'override' modifier"); + context.getTrace().report(VIRTUAL_MEMBER_HIDDEN.on(member, nameIdentifier, declared, overridden, overridden.getContainingDeclaration())); } } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java index 087cbb7d6c4..524341baca4 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java @@ -99,14 +99,10 @@ public class OverridingUtil { List superTypeParameters = superDescriptor.getTypeParameters(); List subTypeParameters = subDescriptor.getTypeParameters(); - Map substitutionContext = Maps.newHashMap(); BiMap axioms = HashBiMap.create(); for (int i = 0, typeParametersSize = superTypeParameters.size(); i < typeParametersSize; i++) { TypeParameterDescriptor superTypeParameter = superTypeParameters.get(i); TypeParameterDescriptor subTypeParameter = subTypeParameters.get(i); - substitutionContext.put( - superTypeParameter.getTypeConstructor(), - new TypeProjection(subTypeParameter.getDefaultType())); axioms.put(superTypeParameter.getTypeConstructor(), subTypeParameter.getTypeConstructor()); } @@ -132,13 +128,29 @@ public class OverridingUtil { // TODO : Default values, varargs etc + return OverrideCompatibilityInfo.success(); + } + + @NotNull + public static OverrideCompatibilityInfo isReturnTypeOkForOverride(@NotNull JetTypeChecker typeChecker, @NotNull CallableDescriptor superDescriptor, @NotNull CallableDescriptor subDescriptor) { + List superTypeParameters = superDescriptor.getTypeParameters(); + List subTypeParameters = subDescriptor.getTypeParameters(); + Map substitutionContext = Maps.newHashMap(); + for (int i = 0, typeParametersSize = superTypeParameters.size(); i < typeParametersSize; i++) { + TypeParameterDescriptor superTypeParameter = superTypeParameters.get(i); + TypeParameterDescriptor subTypeParameter = subTypeParameters.get(i); + substitutionContext.put( + superTypeParameter.getTypeConstructor(), + new TypeProjection(subTypeParameter.getDefaultType())); + } + // This code compares return types, but they are not a part of the signature, so this code does not belong here -// TypeSubstitutor typeSubstitutor = TypeSubstitutor.create(substitutionContext); -// JetType substitutedSuperReturnType = typeSubstitutor.substitute(superDescriptor.getReturnType(), Variance.OUT_VARIANCE); -// assert substitutedSuperReturnType != null; -// if (!typeChecker.isSubtypeOf(subDescriptor.getReturnType(), substitutedSuperReturnType)) { -// return OverrideCompatibilityInfo.returnTypeMismatch(substitutedSuperReturnType, subDescriptor.getReturnType()); -// } + TypeSubstitutor typeSubstitutor = TypeSubstitutor.create(substitutionContext); + JetType substitutedSuperReturnType = typeSubstitutor.substitute(superDescriptor.getReturnType(), Variance.OUT_VARIANCE); + assert substitutedSuperReturnType != null; + if (!typeChecker.isSubtypeOf(subDescriptor.getReturnType(), substitutedSuperReturnType)) { + return OverrideCompatibilityInfo.returnTypeMismatch(substitutedSuperReturnType, subDescriptor.getReturnType()); + } return OverrideCompatibilityInfo.success(); } diff --git a/idea/src/org/jetbrains/jet/plugin/quickfix/QuickFixes.java b/idea/src/org/jetbrains/jet/plugin/quickfix/QuickFixes.java index 79b4184183b..be85717c4b2 100644 --- a/idea/src/org/jetbrains/jet/plugin/quickfix/QuickFixes.java +++ b/idea/src/org/jetbrains/jet/plugin/quickfix/QuickFixes.java @@ -13,7 +13,6 @@ import org.jetbrains.jet.lexer.JetToken; import org.jetbrains.jet.lexer.JetTokens; import java.util.Collection; -import java.util.Set; /** * @author svtk @@ -71,7 +70,7 @@ public class QuickFixes { add(Errors.NON_MEMBER_FUNCTION_NO_BODY, addFunctionBodyFactory); add(Errors.NOTHING_TO_OVERRIDE, RemoveModifierFix.createFactory(JetTokens.OVERRIDE_KEYWORD)); - add(Errors.VIRTUAL_METHOD_HIDDEN, AddModifierFix.createFactory(JetTokens.OVERRIDE_KEYWORD)); + add(Errors.VIRTUAL_MEMBER_HIDDEN, AddModifierFix.createFactory(JetTokens.OVERRIDE_KEYWORD)); add(Errors.VAL_WITH_SETTER, ChangeVariableMutabilityFix.createFactory()); diff --git a/idea/testData/checkerWithErrorTypes/full/Override.jet b/idea/testData/checkerWithErrorTypes/quick/Override.jet similarity index 93% rename from idea/testData/checkerWithErrorTypes/full/Override.jet rename to idea/testData/checkerWithErrorTypes/quick/Override.jet index fecb1c72304..4bc46e47790 100644 --- a/idea/testData/checkerWithErrorTypes/full/Override.jet +++ b/idea/testData/checkerWithErrorTypes/quick/Override.jet @@ -27,12 +27,12 @@ namespace normal { } class MyIllegalClass4 : MyTrait, MyAbstractClass { - fun foo() {} + fun foo() {} override fun other() {} } class MyChildClass1 : MyClass { - fun foo() {} + fun foo() {} override fun bar() {} } } @@ -54,7 +54,7 @@ namespace generics { class MyChildClass : MyGenericClass {} class MyChildClass1 : MyGenericClass {} class MyChildClass2 : MyGenericClass { - fun foo(t: T) = t + fun foo(t: T) = t override fun bar(t: T) = t }