From b08afb0e933c9f10c2c0e29fb2648c8e5931046b Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Tue, 13 Dec 2016 14:45:03 +0300 Subject: [PATCH] JS: prohibit overriding `external` functions with optional parameters by non-external fake functions --- .../native/overrideOptionalParam.kt | 20 +++++++- .../native/overrideOptionalParam.txt | 47 +++++++++++++++++++ .../diagnostics/DefaultErrorMessagesJs.kt | 2 + .../js/resolve/diagnostics/ErrorsJs.java | 3 ++ .../diagnostics/JsInheritanceChecker.kt | 21 +++++++-- 5 files changed, 89 insertions(+), 4 deletions(-) diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt b/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt index 03030c4e8c6..9a8ccb00f13 100644 --- a/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt +++ b/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt @@ -15,6 +15,10 @@ external interface I { fun f(x: Int = noImpl) } +interface J { + fun f(x: Int = 23) +} + open external class D { open fun f(x: Int) } @@ -29,4 +33,18 @@ class F : D(), I { external class G : D(), I { override fun f(x: Int) {} -} \ No newline at end of file +} + +open class X { + fun f(x: Int) {} +} + +open external class XE { + fun f(x: Int) +} + +class Y : X(), I + +external class YE: XE(), I + +class Z : X(), J \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.txt b/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.txt index 85d1400a0ce..db3a7949d40 100644 --- a/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.txt +++ b/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.txt @@ -62,3 +62,50 @@ public external interface I { public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } + +public interface J { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract fun f(/*0*/ x: kotlin.Int = ...): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public open class X { + public constructor X() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final fun f(/*0*/ x: kotlin.Int): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public external open class XE { + public constructor XE() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final fun f(/*0*/ x: kotlin.Int): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class Y : X, I { + public constructor Y() + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*2*/ /*fake_override*/ fun f(/*0*/ x: kotlin.Int = ...): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + +public external final class YE : XE, I { + public constructor YE() + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*2*/ /*fake_override*/ fun f(/*0*/ x: kotlin.Int = ...): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class Z : X, J { + public constructor Z() + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*2*/ /*fake_override*/ fun f(/*0*/ x: kotlin.Int = ...): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt index 955008e3b13..dfc3100a3b6 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt +++ b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt @@ -69,6 +69,8 @@ private val DIAGNOSTIC_FACTORY_TO_RENDERER by lazy { put(ErrorsJs.RUNTIME_ANNOTATION_NOT_SUPPORTED, "Reflection is not supported in JavaScript target, therefore you won't be able " + "to read this annotation in run-time") put(ErrorsJs.OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS, "Overriding `external` function with optional parameters") + put(ErrorsJs.OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS_WITH_FAKE, + "Overriding `external` function with optional parameters by declaration from superclass: {0}", Renderers.COMPACT) this } diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java index 74db5ad5237..a7c4b98f803 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java +++ b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java @@ -18,6 +18,7 @@ package org.jetbrains.kotlin.js.resolve.diagnostics; import com.intellij.psi.PsiElement; import org.jetbrains.kotlin.descriptors.DeclarationDescriptor; +import org.jetbrains.kotlin.descriptors.FunctionDescriptor; import org.jetbrains.kotlin.diagnostics.*; import org.jetbrains.kotlin.psi.KtDeclaration; import org.jetbrains.kotlin.psi.KtElement; @@ -70,6 +71,8 @@ public interface ErrorsJs { DiagnosticFactory0 RUNTIME_ANNOTATION_NOT_SUPPORTED = DiagnosticFactory0.create(WARNING, DECLARATION_SIGNATURE_OR_DEFAULT); DiagnosticFactory0 OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS = DiagnosticFactory0.create(ERROR, PositioningStrategies.DECLARATION_SIGNATURE_OR_DEFAULT); + DiagnosticFactory1 OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS_WITH_FAKE = + DiagnosticFactory1.create(ERROR, PositioningStrategies.DECLARATION_SIGNATURE_OR_DEFAULT); @SuppressWarnings("UnusedDeclaration") Object _initializer = new Object() { diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsInheritanceChecker.kt b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsInheritanceChecker.kt index 0c070eead7d..5b23952d09a 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsInheritanceChecker.kt +++ b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsInheritanceChecker.kt @@ -16,14 +16,14 @@ package org.jetbrains.kotlin.js.resolve.diagnostics -import org.jetbrains.kotlin.descriptors.DeclarationDescriptor -import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.diagnostics.DiagnosticSink import org.jetbrains.kotlin.psi.KtDeclaration import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.checkers.SimpleDeclarationChecker import org.jetbrains.kotlin.resolve.descriptorUtil.hasDefaultValue +import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter object JsInheritanceChecker : SimpleDeclarationChecker { override fun check( @@ -37,10 +37,17 @@ object JsInheritanceChecker : SimpleDeclarationChecker { ) { diagnosticHolder.report(ErrorsJs.OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS.on(declaration)) } + else if (descriptor is ClassDescriptor && !DescriptorUtils.isEffectivelyExternal(descriptor)) { + val fakeOverriddenMethod = findFakeMethodOverridingExternalWithOptionalParams(descriptor) + if (fakeOverriddenMethod != null) { + diagnosticHolder.report(ErrorsJs.OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS_WITH_FAKE.on( + declaration, fakeOverriddenMethod)) + } + } } private fun isOverridingExternalWithOptionalParams(function: FunctionDescriptor): Boolean { - if (!function.kind.isReal) return false + if (!function.kind.isReal && function.modality == Modality.ABSTRACT) return false for (overriddenFunction in function.overriddenDescriptors.filter { DescriptorUtils.isEffectivelyExternal(it) }) { if (overriddenFunction.valueParameters.any { it.hasDefaultValue() }) return true @@ -48,4 +55,12 @@ object JsInheritanceChecker : SimpleDeclarationChecker { return false } + + private fun findFakeMethodOverridingExternalWithOptionalParams(cls: ClassDescriptor): FunctionDescriptor? { + val members = cls.unsubstitutedMemberScope.getContributedDescriptors(DescriptorKindFilter.CALLABLES) + .mapNotNull { it as? FunctionDescriptor } + .filter { it.containingDeclaration == cls && !it.kind.isReal } + + return members.firstOrNull { isOverridingExternalWithOptionalParams(it) } + } }