From ca8cf6cb49a811fcc6920032293e4d74cd673c97 Mon Sep 17 00:00:00 2001 From: Stepan Koltsov Date: Sun, 19 Feb 2012 22:02:33 +0400 Subject: [PATCH] missing error report on incompatible declaration === open class Aaaa() { fun bb() = 1 } class Bbbb() : Aaaa() { fun bb() = 1 } === --- .../resolve/java/JavaDescriptorResolver.java | 7 +- .../FunctionSignatureDiagnosticFactory.java | 3 +- .../jet/lang/resolve/OverloadUtil.java | 12 ++-- .../jet/lang/resolve/OverrideResolver.java | 39 ++++++++--- .../jet/lang/resolve/OverridingUtil.java | 67 +++++++++++-------- .../override/ComplexValRedeclaration.jet | 2 +- ...lictingFunctionSignatureFromSuperclass.jet | 7 ++ ...lictingPropertySignatureFromSuperclass.jet | 7 ++ .../diagnostics/tests/override/Generics.jet | 6 +- .../jet/types/JetOverridingTest.java | 2 +- 10 files changed, 105 insertions(+), 47 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/override/ConflictingFunctionSignatureFromSuperclass.jet create mode 100644 compiler/testData/diagnostics/tests/override/ConflictingPropertySignatureFromSuperclass.jet diff --git a/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/JavaDescriptorResolver.java b/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/JavaDescriptorResolver.java index b7798696486..c75ec6c2089 100644 --- a/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/JavaDescriptorResolver.java +++ b/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/JavaDescriptorResolver.java @@ -1198,9 +1198,14 @@ public class JavaDescriptorResolver { OverrideResolver.generateOverridesInFunctionGroup(methodName, functionsFromSupertypes, functionsFromCurrent, classDescriptor, new OverrideResolver.DescriptorSink() { @Override - public void addToScope(NamedFunctionDescriptor fakeOverride) { + public void addToScope(@NotNull NamedFunctionDescriptor fakeOverride) { functions.add(fakeOverride); } + + @Override + public void conflict(@NotNull NamedFunctionDescriptor fromSuper, @NotNull NamedFunctionDescriptor fromCurrent) { + // nop + } }); } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/FunctionSignatureDiagnosticFactory.java b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/FunctionSignatureDiagnosticFactory.java index b3b83766369..e4a9dcdfe4c 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/FunctionSignatureDiagnosticFactory.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/FunctionSignatureDiagnosticFactory.java @@ -19,6 +19,7 @@ package org.jetbrains.jet.lang.diagnostics; import com.intellij.openapi.util.TextRange; import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; +import org.jetbrains.jet.lang.descriptors.CallableMemberDescriptor; import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; import org.jetbrains.jet.lang.psi.JetClass; import org.jetbrains.jet.lang.psi.JetDeclaration; @@ -63,7 +64,7 @@ public class FunctionSignatureDiagnosticFactory extends DiagnosticFactoryWithMes } @NotNull - public Diagnostic on(@NotNull JetDeclaration declaration, @NotNull FunctionDescriptor functionDescriptor, + public Diagnostic on(@NotNull JetDeclaration declaration, @NotNull CallableMemberDescriptor functionDescriptor, @NotNull String functionContainer) { TextRange rangeToMark = rangeToMark(declaration); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadUtil.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadUtil.java index acc823ac541..064569b2e3d 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadUtil.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadUtil.java @@ -28,10 +28,14 @@ public class OverloadUtil { */ public static OverloadCompatibilityInfo isOverloadble(FunctionDescriptor a, FunctionDescriptor b) { OverridingUtil.OverrideCompatibilityInfo overrideCompatibilityInfo = OverridingUtil.isOverridableByImpl(a, b, false); - if (!overrideCompatibilityInfo.isOverloadable()) { - return OverloadCompatibilityInfo.someError(); - } else { - return OverloadCompatibilityInfo.success(); + switch (overrideCompatibilityInfo.isOverridable()) { + case OVERRIDABLE: + return OverloadCompatibilityInfo.someError(); + case CONFLICT: + case INCOMPATIBLE: + return OverloadCompatibilityInfo.success(); + default: + throw new IllegalStateException(); } } 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 d8be90262fd..ef97644c6ae 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java @@ -24,6 +24,7 @@ import com.intellij.psi.PsiElement; import com.intellij.util.containers.MultiMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.jet.lang.descriptors.*; +import org.jetbrains.jet.lang.diagnostics.Errors; import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.resolve.scopes.JetScope; import org.jetbrains.jet.lang.types.JetType; @@ -109,9 +110,15 @@ public class OverrideResolver { classDescriptor, new DescriptorSink() { @Override - public void addToScope(NamedFunctionDescriptor fakeOverride) { + public void addToScope(@NotNull NamedFunctionDescriptor fakeOverride) { classDescriptor.getScopeForMemberLookupAsWritableScope().addFunctionDescriptor(fakeOverride); } + + @Override + public void conflict(@NotNull NamedFunctionDescriptor fromSuper, @NotNull NamedFunctionDescriptor fromCurrent) { + JetNamedFunction jetDeclaration = (JetNamedFunction) context.getTrace().get(BindingContext.DESCRIPTOR_TO_DECLARATION, fromCurrent); + context.getTrace().report(Errors.CONFLICTING_OVERLOADS.on(jetDeclaration, fromCurrent, fromCurrent.getContainingDeclaration().getName())); + } }); } @@ -122,15 +129,23 @@ public class OverrideResolver { classDescriptor, new DescriptorSink() { @Override - public void addToScope(PropertyDescriptor fakeOverride) { + public void addToScope(@NotNull PropertyDescriptor fakeOverride) { classDescriptor.getScopeForMemberLookupAsWritableScope().addPropertyDescriptor(fakeOverride); } + + @Override + public void conflict(@NotNull PropertyDescriptor fromSuper, @NotNull PropertyDescriptor fromCurrent) { + JetProperty jetProperty = (JetProperty) context.getTrace().get(BindingContext.DESCRIPTOR_TO_DECLARATION, fromCurrent); + context.getTrace().report(Errors.CONFLICTING_OVERLOADS.on(jetProperty, fromCurrent, fromCurrent.getContainingDeclaration().getName())); + } }); } } public interface DescriptorSink { - void addToScope(D fakeOverride); + void addToScope(@NotNull D fakeOverride); + + void conflict(@NotNull D fromSuper, @NotNull D fromCurrent); } public static void generateOverridesInFunctionGroup( @@ -147,14 +162,17 @@ public class OverrideResolver { boolean overrides = false; for (NamedFunctionDescriptor functionFromCurrent : functionsFromCurrent) { - if (OverridingUtil.isOverridableBy(functionFromSupertype, functionFromCurrent).isOverridable()) { + OverridingUtil.OverrideCompatibilityInfo.ErrorKind overridable = OverridingUtil.isOverridableBy(functionFromSupertype, functionFromCurrent).isOverridable(); + if (overridable == OverridingUtil.OverrideCompatibilityInfo.ErrorKind.OVERRIDABLE) { ((FunctionDescriptorImpl) functionFromCurrent).addOverriddenFunction(functionFromSupertype); overrides = true; + } else if (overridable == OverridingUtil.OverrideCompatibilityInfo.ErrorKind.CONFLICT) { + sink.conflict(functionFromSupertype, functionFromCurrent); } } for (NamedFunctionDescriptor fakeOverride : fakeOverrides) { - if (OverridingUtil.isOverridableBy(functionFromSupertype, fakeOverride).isOverridable()) { + if (OverridingUtil.isOverridableBy(functionFromSupertype, fakeOverride).isOverridable() == OverridingUtil.OverrideCompatibilityInfo.ErrorKind.OVERRIDABLE) { ((FunctionDescriptorImpl) fakeOverride).addOverriddenFunction(functionFromSupertype); overrides = true; } @@ -183,14 +201,17 @@ public class OverrideResolver { for (PropertyDescriptor propertyFromSupertype : propertiesFromSupertypes) { boolean overrides = false; for (PropertyDescriptor propertyFromCurrent : propertiesFromCurrent) { - if (OverridingUtil.isOverridableBy(propertyFromSupertype, propertyFromCurrent).isOverridable()) { + OverridingUtil.OverrideCompatibilityInfo.ErrorKind overridable = OverridingUtil.isOverridableBy(propertyFromSupertype, propertyFromCurrent).isOverridable(); + if (overridable == OverridingUtil.OverrideCompatibilityInfo.ErrorKind.OVERRIDABLE) { propertyFromCurrent.addOverriddenDescriptor(propertyFromSupertype); overrides = true; + } else if (overridable == OverridingUtil.OverrideCompatibilityInfo.ErrorKind.CONFLICT) { + sink.conflict(propertyFromSupertype, propertyFromCurrent); } } for (PropertyDescriptor fakeOverride : fakeOverrides) { - if (OverridingUtil.isOverridableBy(propertyFromSupertype, fakeOverride).isOverridable()) { + if (OverridingUtil.isOverridableBy(propertyFromSupertype, fakeOverride).isOverridable() == OverridingUtil.OverrideCompatibilityInfo.ErrorKind.OVERRIDABLE) { ((PropertyDescriptor) fakeOverride).addOverriddenDescriptor(propertyFromSupertype); overrides = true; } @@ -374,8 +395,8 @@ public class OverrideResolver { for (CallableMemberDescriptor another : filteredMembers) { // if (one == another) continue; factoredMembers.put(one, one); - if (OverridingUtil.isOverridableBy(one, another).isOverridable() - || OverridingUtil.isOverridableBy(another, one).isOverridable()) { + if (OverridingUtil.isOverridableBy(one, another).isOverridable() == OverridingUtil.OverrideCompatibilityInfo.ErrorKind.OVERRIDABLE + || OverridingUtil.isOverridableBy(another, one).isOverridable() == OverridingUtil.OverrideCompatibilityInfo.ErrorKind.OVERRIDABLE) { factoredMembers.put(one, another); } } 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 e990452d9e7..057915d8b2e 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java @@ -56,8 +56,8 @@ public class OverridingUtil { for (D otherD : candidates) { CallableDescriptor other = transform.fun(otherD); if (me.getOriginal() == other.getOriginal() - && isOverridableBy(other, me).isOverridable() - && isOverridableBy(me, other).isOverridable()) { + && isOverridableBy(other, me).isOverridable() == OverrideCompatibilityInfo.ErrorKind.OVERRIDABLE + && isOverridableBy(me, other).isOverridable() == OverrideCompatibilityInfo.ErrorKind.OVERRIDABLE) { continue outerLoop; } } @@ -133,12 +133,6 @@ public class OverridingUtil { // TODO : Visibility - if (forOverride) { - if (superDescriptor.getTypeParameters().size() != subDescriptor.getTypeParameters().size()) { - return OverrideCompatibilityInfo.typeParameterNumberMismatch(); - } - } - if (compiledValueParameterCount(superDescriptor) != compiledValueParameterCount(subDescriptor)) { return OverrideCompatibilityInfo.valueParameterNumberMismatch(); } @@ -146,6 +140,20 @@ public class OverridingUtil { List superValueParameters = compiledValueParameters(superDescriptor); List subValueParameters = compiledValueParameters(subDescriptor); + if (forOverride) { + if (superDescriptor.getTypeParameters().size() != subDescriptor.getTypeParameters().size()) { + for (int i = 0; i < superValueParameters.size(); ++i) { + JetType superValueParameterType = getUpperBound(superValueParameters.get(i)); + JetType subValueParameterType = getUpperBound(subValueParameters.get(i)); + // TODO: compare erasure + if (!JetTypeChecker.INSTANCE.equalTypes(superValueParameterType, subValueParameterType)) { + return OverrideCompatibilityInfo.typeParameterNumberMismatch(); + } + } + return OverrideCompatibilityInfo.valueParameterTypeMismatch(null, null, OverrideCompatibilityInfo.ErrorKind.CONFLICT); + } + } + if (forOverride) { List superTypeParameters = superDescriptor.getTypeParameters(); @@ -172,18 +180,21 @@ public class OverridingUtil { JetType subValueParameter = subValueParameters.get(i); if (!JetTypeChecker.INSTANCE.equalTypes(superValueParameter, subValueParameter, axioms)) { - return OverrideCompatibilityInfo.valueParameterTypeMismatch(superValueParameter, subValueParameter); + return OverrideCompatibilityInfo.valueParameterTypeMismatch(superValueParameter, subValueParameter, OverrideCompatibilityInfo.ErrorKind.INCOMPATIBLE); } } + } else { + for (int i = 0; i < superValueParameters.size(); ++i) { JetType superValueParameterType = getUpperBound(superValueParameters.get(i)); JetType subValueParameterType = getUpperBound(subValueParameters.get(i)); // TODO: compare erasure if (!JetTypeChecker.INSTANCE.equalTypes(superValueParameterType, subValueParameterType)) { - return OverrideCompatibilityInfo.valueParameterTypeMismatch(superValueParameterType, subValueParameterType); + return OverrideCompatibilityInfo.valueParameterTypeMismatch(superValueParameterType, subValueParameterType, OverrideCompatibilityInfo.ErrorKind.CONFLICT); } } + } // TODO : Default values, varargs etc @@ -250,7 +261,13 @@ public class OverridingUtil { public static class OverrideCompatibilityInfo { - private static final OverrideCompatibilityInfo SUCCESS = new OverrideCompatibilityInfo(true, "SUCCESS"); + public enum ErrorKind { + OVERRIDABLE, + INCOMPATIBLE, + CONFLICT, + } + + private static final OverrideCompatibilityInfo SUCCESS = new OverrideCompatibilityInfo(ErrorKind.OVERRIDABLE, "SUCCESS"); @NotNull public static OverrideCompatibilityInfo success() { @@ -259,62 +276,58 @@ public class OverridingUtil { @NotNull public static OverrideCompatibilityInfo nameMismatch() { - return new OverrideCompatibilityInfo(false, "nameMismatch"); // TODO + return new OverrideCompatibilityInfo(ErrorKind.INCOMPATIBLE, "nameMismatch"); // TODO } @NotNull public static OverrideCompatibilityInfo typeParameterNumberMismatch() { - return new OverrideCompatibilityInfo(false, "typeParameterNumberMismatch"); // TODO + return new OverrideCompatibilityInfo(ErrorKind.INCOMPATIBLE, "typeParameterNumberMismatch"); // TODO } @NotNull public static OverrideCompatibilityInfo valueParameterNumberMismatch() { - return new OverrideCompatibilityInfo(false, "valueParameterNumberMismatch"); // TODO + return new OverrideCompatibilityInfo(ErrorKind.INCOMPATIBLE, "valueParameterNumberMismatch"); // TODO } @NotNull public static OverrideCompatibilityInfo boundsMismatch(TypeParameterDescriptor superTypeParameter, TypeParameterDescriptor subTypeParameter) { - return new OverrideCompatibilityInfo(false, "boundsMismatch"); // TODO + return new OverrideCompatibilityInfo(ErrorKind.INCOMPATIBLE, "boundsMismatch"); // TODO } @NotNull - public static OverrideCompatibilityInfo valueParameterTypeMismatch(JetType superValueParameter, JetType subValueParameter) { - return new OverrideCompatibilityInfo(false, "valueParameterTypeMismatch"); // TODO + public static OverrideCompatibilityInfo valueParameterTypeMismatch(JetType superValueParameter, JetType subValueParameter, ErrorKind errorKind) { + return new OverrideCompatibilityInfo(errorKind, "valueParameterTypeMismatch"); // TODO } @NotNull public static OverrideCompatibilityInfo memberKindMismatch() { - return new OverrideCompatibilityInfo(false, "memberKindMismatch"); // TODO + return new OverrideCompatibilityInfo(ErrorKind.INCOMPATIBLE, "memberKindMismatch"); // TODO } @NotNull public static OverrideCompatibilityInfo returnTypeMismatch(JetType substitutedSuperReturnType, JetType unsubstitutedSubReturnType) { - return new OverrideCompatibilityInfo(true, "returnTypeMismatch: " + unsubstitutedSubReturnType + " >< " + substitutedSuperReturnType); // TODO + return new OverrideCompatibilityInfo(ErrorKind.CONFLICT, "returnTypeMismatch: " + unsubstitutedSubReturnType + " >< " + substitutedSuperReturnType); // TODO } @NotNull public static OverrideCompatibilityInfo varOverriddenByVal() { - return new OverrideCompatibilityInfo(false, "varOverriddenByVal"); // TODO + return new OverrideCompatibilityInfo(ErrorKind.INCOMPATIBLE, "varOverriddenByVal"); // TODO } //////////////////////////////////////////////////////////////////////////////////////////////////////////////// - private final boolean overridable; + private final ErrorKind overridable; private final String message; - public OverrideCompatibilityInfo(boolean success, String message) { + public OverrideCompatibilityInfo(ErrorKind success, String message) { this.overridable = success; this.message = message; } - public boolean isOverridable() { + public ErrorKind isOverridable() { return overridable; } - public boolean isOverloadable() { - return !overridable; - } - public String getMessage() { return message; } diff --git a/compiler/testData/diagnostics/tests/override/ComplexValRedeclaration.jet b/compiler/testData/diagnostics/tests/override/ComplexValRedeclaration.jet index a84a556ba5d..f484dc133b3 100644 --- a/compiler/testData/diagnostics/tests/override/ComplexValRedeclaration.jet +++ b/compiler/testData/diagnostics/tests/override/ComplexValRedeclaration.jet @@ -5,5 +5,5 @@ abstract class MyAbstractClass { } abstract class MyLegalAbstractClass2(t : T) : MyAbstractClass() { - val pr : T = t + val pr : T = t } diff --git a/compiler/testData/diagnostics/tests/override/ConflictingFunctionSignatureFromSuperclass.jet b/compiler/testData/diagnostics/tests/override/ConflictingFunctionSignatureFromSuperclass.jet new file mode 100644 index 00000000000..5bb76f9b84e --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/ConflictingFunctionSignatureFromSuperclass.jet @@ -0,0 +1,7 @@ +open class Aaa() { + fun foo() = 1 +} + +open class Bbb() : Aaa() { + fun foo() = 2 +} diff --git a/compiler/testData/diagnostics/tests/override/ConflictingPropertySignatureFromSuperclass.jet b/compiler/testData/diagnostics/tests/override/ConflictingPropertySignatureFromSuperclass.jet new file mode 100644 index 00000000000..b423eb0864f --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/ConflictingPropertySignatureFromSuperclass.jet @@ -0,0 +1,7 @@ +open class Aaa() { + val bar = 1 +} + +open class Bbb() : Aaa() { + val bar = "aa" +} diff --git a/compiler/testData/diagnostics/tests/override/Generics.jet b/compiler/testData/diagnostics/tests/override/Generics.jet index a5faaa386de..ec17533dbaf 100644 --- a/compiler/testData/diagnostics/tests/override/Generics.jet +++ b/compiler/testData/diagnostics/tests/override/Generics.jet @@ -43,7 +43,7 @@ abstract class MyAbstractClass1 : MyTrait, MyAbstractClass() { class MyIllegalGenericClass1 : MyTrait, MyAbstractClass() {} class MyIllegalGenericClass2(r : R) : MyTrait, MyAbstractClass() { override fun foo(r: R) = r - override val pr : R = r + override val pr : R = r } class MyIllegalClass1 : MyTrait, MyAbstractClass() {} abstract class MyLegalAbstractClass1 : MyTrait, MyAbstractClass() {} @@ -51,10 +51,10 @@ abstract class MyLegalAbstractClass1 : MyTrait, MyAbstractClass() { class MyIllegalClass2(t : T) : MyTrait, MyAbstractClass() { fun foo(t: T) = t fun bar(t: T) = t - val pr : T = t + val pr : T = t } abstract class MyLegalAbstractClass2(t : T) : MyTrait, MyAbstractClass() { fun foo(t: T) = t fun bar(t: T) = t - val pr : T = t + val pr : T = t } diff --git a/compiler/tests/org/jetbrains/jet/types/JetOverridingTest.java b/compiler/tests/org/jetbrains/jet/types/JetOverridingTest.java index fd1ab4848a9..af46ccdf6eb 100644 --- a/compiler/tests/org/jetbrains/jet/types/JetOverridingTest.java +++ b/compiler/tests/org/jetbrains/jet/types/JetOverridingTest.java @@ -156,7 +156,7 @@ public class JetOverridingTest extends JetLiteFixture { FunctionDescriptor a = makeFunction(superFun); FunctionDescriptor b = makeFunction(subFun); OverridingUtil.OverrideCompatibilityInfo overridableWith = OverridingUtil.isOverridableBy(a, b); - assertEquals(overridableWith.getMessage(), expectedIsError, !overridableWith.isOverridable()); + assertEquals(overridableWith.getMessage(), expectedIsError, overridableWith.isOverridable() != OverridingUtil.OverrideCompatibilityInfo.ErrorKind.OVERRIDABLE); } private FunctionDescriptor makeFunction(String funDecl) {