From 5218246d80855fbc2ebd96cfa333e3ce003be1ef Mon Sep 17 00:00:00 2001 From: Stepan Koltsov Date: Thu, 10 Nov 2011 20:00:15 +0400 Subject: [PATCH] KT-424: Check declarations for overload-compatibility === fun a() {} fun a() {} === must be an error. --- .../jet/lang/diagnostics/Errors.java | 13 ++ .../jet/lang/resolve/OverloadResolver.java | 86 ++++++++++ .../jet/lang/resolve/OverloadUtil.java | 52 ++++++ .../jet/lang/resolve/OverridingUtil.java | 7 + .../jet/lang/resolve/TopDownAnalyzer.java | 1 + .../quick/ConflictingOverloads.jet | 12 ++ .../jetbrains/jet/types/JetOverloadTest.java | 158 ++++++++++++++++++ .../jet/types/JetOverridingTest.java | 7 + 8 files changed, 336 insertions(+) create mode 100644 compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadResolver.java create mode 100644 compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadUtil.java create mode 100644 compiler/testData/checkerWithErrorTypes/quick/ConflictingOverloads.jet create mode 100644 compiler/tests/org/jetbrains/jet/types/JetOverloadTest.java 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 52b22dbcb3c..d18d4ca5db8 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java @@ -375,6 +375,19 @@ public interface Errors { } }; + ParameterizedDiagnosticFactory2 CONFLICTING_OVERLOADS = new ParameterizedDiagnosticFactory2(ERROR, "{1} is already defined in ''{0}''") { + @Override + protected String makeMessageForA(@NotNull JetClassOrObject jetClassOrObject) { + return jetClassOrObject.getName(); + } + + @Override + protected String makeMessageForB(@NotNull CallableMemberDescriptor memberDescriptor) { + return DescriptorRenderer.TEXT.render(memberDescriptor); + } + }; + + ParameterizedDiagnosticFactory3 RESULT_TYPE_MISMATCH = ParameterizedDiagnosticFactory3.create(ERROR, "{0} must return {1} but returns {2}"); ParameterizedDiagnosticFactory3 UNSAFE_INFIX_CALL = ParameterizedDiagnosticFactory3.create(ERROR, "Infix call corresponds to a dot-qualified call ''{0}.{1}({2})'' which is not allowed on a nullable receiver ''{0}''. Use '?.'-qualified call instead"); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadResolver.java new file mode 100644 index 00000000000..be18d1938de --- /dev/null +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadResolver.java @@ -0,0 +1,86 @@ +package org.jetbrains.jet.lang.resolve; + +import com.intellij.psi.PsiElement; +import com.intellij.util.containers.MultiMap; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; +import org.jetbrains.jet.lang.descriptors.MutableClassDescriptor; +import org.jetbrains.jet.lang.diagnostics.Errors; +import org.jetbrains.jet.lang.psi.*; + +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import static org.jetbrains.jet.lang.resolve.BindingContext.DELEGATED; + +/** + * @author Stepan Koltsov + */ +public class OverloadResolver { + private final TopDownAnalysisContext context; + + public OverloadResolver(@NotNull TopDownAnalysisContext context) { + this.context = context; + } + + public void process() { + checkOverloads(); + } + + private void checkOverloads() { + for (Map.Entry entry : context.getClasses().entrySet()) { + checkOverloadsInAClass(entry.getValue(), entry.getKey()); + } + for (Map.Entry entry : context.getObjects().entrySet()) { + checkOverloadsInAClass(entry.getValue(), entry.getKey()); + } + } + + private void checkOverloadsInAClass(MutableClassDescriptor classDescriptor, JetClassOrObject klass) { + MultiMap functionsByName = MultiMap.create(); + + for (FunctionDescriptor function : classDescriptor.getFunctions()) { + functionsByName.putValue(function.getName(), function); + } + + for (Map.Entry> e : functionsByName.entrySet()) { + checkOverloadsWithSameName(e.getKey(), e.getValue(), klass); + } + + // properties are checked elsewhere + + // Kotlin has no secondary constructors at this time + + } + + private void checkOverloadsWithSameName(String name, Collection functions, JetClassOrObject klass) { + if (functions.size() == 1) { + // microoptimization + return; + } + + for (FunctionDescriptor function : functions) { + for (FunctionDescriptor function2 : functions) { + if (function == function2) { + continue; + } + + OverloadUtil.OverloadCompatibilityInfo overloadble = OverloadUtil.isOverloadble(function, function2); + if (!overloadble.isSuccess()) { + JetNamedFunction member = (JetNamedFunction) context.getTrace().get(BindingContext.DESCRIPTOR_TO_DECLARATION, function); + if (member == null) { + assert context.getTrace().get(DELEGATED, function); + return; + } + + // XXX: mark "fun" keyword and parameters as in Java IDE // stepan.koltsov@ + PsiElement mark = member.getNameIdentifier(); + context.getTrace().report(Errors.CONFLICTING_OVERLOADS.on(mark, klass, function)); + } + } + } + } + +} diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadUtil.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadUtil.java new file mode 100644 index 00000000000..c33d452ada0 --- /dev/null +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverloadUtil.java @@ -0,0 +1,52 @@ +package org.jetbrains.jet.lang.resolve; + +import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; + +/** + * @author Stepan Koltsov + */ +public class OverloadUtil { + + public static OverloadCompatibilityInfo isOverloadble(FunctionDescriptor a, FunctionDescriptor b) { + OverridingUtil.OverrideCompatibilityInfo overrideCompatibilityInfo = OverridingUtil.isOverridableByImpl(a, b, false); + if (overrideCompatibilityInfo.isSuccess()) { + return OverloadCompatibilityInfo.someError(); + } else { + return OverloadCompatibilityInfo.success(); + } + } + + public static class OverloadCompatibilityInfo { + + private static final OverloadCompatibilityInfo SUCCESS = new OverloadCompatibilityInfo(true, "SUCCESS"); + + public static OverloadCompatibilityInfo success() { + return SUCCESS; + } + + public static OverloadCompatibilityInfo someError() { + return new OverloadCompatibilityInfo(false, "XXX"); + } + + + //////////////////////////////////////////////////////////////////////////////////////////////////////////////// + + private final boolean isSuccess; + private final String message; + + public OverloadCompatibilityInfo(boolean success, String message) { + isSuccess = success; + this.message = message; + } + + public boolean isSuccess() { + return isSuccess; + } + + public String getMessage() { + return message; + } + + } + +} 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 53b95a441f4..6b26f59f733 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverridingUtil.java @@ -93,6 +93,13 @@ public class OverridingUtil { @NotNull public static OverrideCompatibilityInfo isOverridableBy(@NotNull CallableDescriptor superDescriptor, @NotNull CallableDescriptor subDescriptor) { + return isOverridableByImpl(superDescriptor, subDescriptor, true); + } + + /** + * @param forOverride true for override, false for overload + */ + static OverrideCompatibilityInfo isOverridableByImpl(CallableDescriptor superDescriptor, CallableDescriptor subDescriptor, boolean forOverride) { if (superDescriptor instanceof FunctionDescriptor) { if (subDescriptor instanceof PropertyDescriptor) return OverrideCompatibilityInfo.memberKindMismatch(); } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java index 48a8733ee82..eefbe5a7e4b 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalyzer.java @@ -52,6 +52,7 @@ public class TopDownAnalyzer { new DeclarationResolver(context).process(); new DelegationResolver(context).process(); new OverrideResolver(context).process(); + new OverloadResolver(context).process(); new BodyResolver(context).resolveBehaviorDeclarationBodies(); new ControlFlowAnalyzer(context, flowDataTraceFactory, declaredLocally).process(); new DeclarationsChecker(context).process(); diff --git a/compiler/testData/checkerWithErrorTypes/quick/ConflictingOverloads.jet b/compiler/testData/checkerWithErrorTypes/quick/ConflictingOverloads.jet new file mode 100644 index 00000000000..e1d47e33868 --- /dev/null +++ b/compiler/testData/checkerWithErrorTypes/quick/ConflictingOverloads.jet @@ -0,0 +1,12 @@ +// http://youtrack.jetbrains.net/issue/KT-424 + +class A { + // XXX: error should be reported on the whole method signature + // not only on method name + fun a() { + } + + fun a() { + } +} + diff --git a/compiler/tests/org/jetbrains/jet/types/JetOverloadTest.java b/compiler/tests/org/jetbrains/jet/types/JetOverloadTest.java new file mode 100644 index 00000000000..30916fa528a --- /dev/null +++ b/compiler/tests/org/jetbrains/jet/types/JetOverloadTest.java @@ -0,0 +1,158 @@ +package org.jetbrains.jet.types; + +import org.jetbrains.jet.JetLiteFixture; +import org.jetbrains.jet.JetTestCaseBuilder; +import org.jetbrains.jet.JetTestUtils; +import org.jetbrains.jet.lang.JetSemanticServices; +import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; +import org.jetbrains.jet.lang.descriptors.ModuleDescriptor; +import org.jetbrains.jet.lang.psi.JetNamedFunction; +import org.jetbrains.jet.lang.psi.JetPsiFactory; +import org.jetbrains.jet.lang.resolve.ClassDescriptorResolver; +import org.jetbrains.jet.lang.resolve.OverloadUtil; +import org.jetbrains.jet.lang.types.JetStandardLibrary; + +/** + * @author Stepan Koltsov + */ +public class JetOverloadTest extends JetLiteFixture { + + private ModuleDescriptor root = new ModuleDescriptor("test_root"); + private JetStandardLibrary library; + private JetSemanticServices semanticServices; + private ClassDescriptorResolver classDescriptorResolver; + + @Override + public void setUp() throws Exception { + super.setUp(); + library = JetStandardLibrary.getJetStandardLibrary(getProject()); + semanticServices = JetSemanticServices.createSemanticServices(library); + classDescriptorResolver = semanticServices.getClassDescriptorResolver(JetTestUtils.DUMMY_TRACE); + } + + @Override + protected String getTestDataPath() { + return JetTestCaseBuilder.getTestDataPathBase(); + } + + public void testBasic() throws Exception { + + assertNotOverloadable( + "fun a() : Int", + "fun a() : Int"); + + assertNotOverloadable( + "fun a() : Int", + "fun a() : Any"); + + assertNotOverloadable( + "fun a() : T1", + "fun a() : T"); + + assertNotOverloadable( + "fun a(a : T1) : T1", + "fun a(a : T) : T"); + + assertNotOverloadable( + "fun a(a : T1) : T1", + "fun a(a : T) : T"); + + assertNotOverloadable( + "fun a(a : T1) : T1", + "fun a(a : T) : Y"); + + //////////////////////////////////////////////////////////////////////////////////////////////////////////////// + + + assertOverloadable( + "fun ab() : Int", + "fun a() : Int"); + + assertNotOverloadable( + "fun a() : Int", + "fun a() : Any"); + + assertOverloadable( + "fun a(a : Int) : Int", + "fun a() : Int"); + + assertOverloadable( + "fun a() : Int", + "fun a(a : Int) : Int"); + + assertOverloadable( + "fun a(a : Int?) : Int", + "fun a(a : Int) : Int"); + + // XXX: different from overridable + /* + assertNotOverloadable( + "fun a(a : Int) : Int", + "fun a(a : Int) : Int"); + */ + + assertOverloadable( + "fun a(a : T1) : T1", + "fun a(a : T) : T"); + + assertOverloadable( + "fun a(a : T1) : T1", + "fun a(a : Y) : T"); + + assertNotOverloadable( + "fun a(a : T1) : X", + "fun a(a : T) : T"); + + assertNotOverloadable( + "fun a>(a : Array) : T1", + "fun a>(a : Array) : T"); + + assertOverloadable( + "fun a>(a : Array) : T1", + "fun a>(a : Array) : T"); + + assertOverloadable( + "fun a>(a : Array) : T1", + "fun a>(a : Array) : T"); + + assertOverloadable( + "fun a>(a : Array) : T1", + "fun a>(a : Array) : T"); + + assertOverloadable( + "fun a>(a : Array) : T1", + "fun a>(a : Array) : T"); + + assertOverloadable( + "fun a>(a : Array<*>) : T1", + "fun a>(a : Array) : T"); + + } + + private void assertNotOverloadable(String funA, String funB) { + assertOverloadabilityRelation(funA, funB, true); + } + + private void assertOverloadable(String funA, String funB) { + assertOverloadabilityRelation(funA, funB, false); + } + + private void assertOverloadabilityRelation(String funA, String funB, boolean expectedIsError) { + FunctionDescriptor a = makeFunction(funA); + FunctionDescriptor b = makeFunction(funB); + { + OverloadUtil.OverloadCompatibilityInfo overloadableWith = OverloadUtil.isOverloadble(a, b); + assertEquals(overloadableWith.getMessage(), expectedIsError, !overloadableWith.isSuccess()); + } + { + OverloadUtil.OverloadCompatibilityInfo overloadableWith = OverloadUtil.isOverloadble(b, a); + assertEquals(overloadableWith.getMessage(), expectedIsError, !overloadableWith.isSuccess()); + } + } + + private FunctionDescriptor makeFunction(String funDecl) { + JetNamedFunction function = JetPsiFactory.createFunction(getProject(), funDecl); + return classDescriptorResolver.resolveFunctionDescriptor(root, library.getLibraryScope(), function); + } + +} diff --git a/compiler/tests/org/jetbrains/jet/types/JetOverridingTest.java b/compiler/tests/org/jetbrains/jet/types/JetOverridingTest.java index 4de8bd9a285..18b27921f61 100644 --- a/compiler/tests/org/jetbrains/jet/types/JetOverridingTest.java +++ b/compiler/tests/org/jetbrains/jet/types/JetOverridingTest.java @@ -67,6 +67,13 @@ public class JetOverridingTest extends JetLiteFixture { "fun a() : Int", "fun a() : Any"); + // return types are not cheked in the utility + /* + assertNotOverridable( + "fun a() : Any", + "fun a() : Int"); + */ + assertNotOverridable( "fun a(a : Int) : Int", "fun a() : Int");