From e2dab5ab5caf782bb8fab0c4b2dbfc92b7b06887 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Mon, 12 Dec 2016 17:43:47 +0300 Subject: [PATCH] JS: prohibit overriding `external` functions with optional parameters by non-external functions. See KT-13889 --- .../native/overrideOptionalParam.kt | 32 ++++++++++ .../native/overrideOptionalParam.txt | 64 +++++++++++++++++++ .../DiagnosticsTestWithJsStdLibGenerated.java | 6 ++ .../js/resolve/JsPlatformConfigurator.kt | 2 +- .../diagnostics/DefaultErrorMessagesJs.kt | 1 + .../js/resolve/diagnostics/ErrorsJs.java | 2 + .../resolve/diagnostics/JsExternalChecker.kt | 2 +- .../diagnostics/JsInheritanceChecker.kt | 51 +++++++++++++++ 8 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt create mode 100644 compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.txt create mode 100644 js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsInheritanceChecker.kt diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt b/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt new file mode 100644 index 00000000000..03030c4e8c6 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt @@ -0,0 +1,32 @@ +open external class A { + open fun f(x: Int = noImpl) +} + +class B : A() { + override fun f(x: Int) {} +} + +external class C : A() { + override fun f(x: Int) {} +} + + +external interface I { + fun f(x: Int = noImpl) +} + +open external class D { + open fun f(x: Int) +} + +class E : D() { + override fun f(x: Int) {} +} + +class F : D(), I { + override fun f(x: Int) {} +} + +external class G : D(), I { + override fun f(x: Int) {} +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.txt b/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.txt new file mode 100644 index 00000000000..85d1400a0ce --- /dev/null +++ b/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.txt @@ -0,0 +1,64 @@ +package + +public external open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open 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 B : A { + public constructor B() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ 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 final class C : A { + public constructor C() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ 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 D { + public constructor D() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open 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 E : D { + public constructor E() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ 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 F : D, I { + public constructor F() + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*2*/ 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 G : D, I { + public constructor G() + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*2*/ 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 interface I { + 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 +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithJsStdLibGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithJsStdLibGenerated.java index 3f5efe572f0..055cde41a52 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithJsStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithJsStdLibGenerated.java @@ -581,6 +581,12 @@ public class DiagnosticsTestWithJsStdLibGenerated extends AbstractDiagnosticsTes doTest(fileName); } + @TestMetadata("overrideOptionalParam.kt") + public void testOverrideOptionalParam() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt"); + doTest(fileName); + } + @TestMetadata("privateMembers.kt") public void testPrivateMembers() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithJsStdLib/native/privateMembers.kt"); diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/JsPlatformConfigurator.kt b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/JsPlatformConfigurator.kt index df870dfd1c0..70e289ee5cd 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/JsPlatformConfigurator.kt +++ b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/JsPlatformConfigurator.kt @@ -36,7 +36,7 @@ object JsPlatformConfigurator : PlatformConfigurator( additionalDeclarationCheckers = listOf( NativeInvokeChecker(), NativeGetterChecker(), NativeSetterChecker(), JsNameChecker, JsModuleChecker, - JsExternalChecker(), + JsExternalChecker, JsInheritanceChecker, JsRuntimeAnnotationChecker, HeaderImplDeclarationChecker() ), 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 42c2fa41f52..955008e3b13 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 @@ -68,6 +68,7 @@ private val DIAGNOSTIC_FACTORY_TO_RENDERER by lazy { put(ErrorsJs.RUNTIME_ANNOTATION_ON_EXTERNAL_DECLARATION, "Runtime annotation can't be put on external declaration") 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") 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 f56ed793245..74db5ad5237 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 @@ -68,6 +68,8 @@ public interface ErrorsJs { DiagnosticFactory0 RUNTIME_ANNOTATION_ON_EXTERNAL_DECLARATION = DiagnosticFactory0.create( ERROR, DECLARATION_SIGNATURE_OR_DEFAULT); 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); @SuppressWarnings("UnusedDeclaration") Object _initializer = new Object() { diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsExternalChecker.kt b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsExternalChecker.kt index 6d447761f57..5b28cc6f00b 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsExternalChecker.kt +++ b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsExternalChecker.kt @@ -33,7 +33,7 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperInterfaces import org.jetbrains.kotlin.resolve.descriptorUtil.isExtension import org.jetbrains.kotlin.utils.singletonOrEmptyList -class JsExternalChecker : SimpleDeclarationChecker { +object JsExternalChecker : SimpleDeclarationChecker { override fun check(declaration: KtDeclaration, descriptor: DeclarationDescriptor, diagnosticHolder: DiagnosticSink, bindingContext: BindingContext) { if (!AnnotationsUtils.isNativeObject(descriptor)) return 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 new file mode 100644 index 00000000000..0c070eead7d --- /dev/null +++ b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsInheritanceChecker.kt @@ -0,0 +1,51 @@ +/* + * Copyright 2010-2016 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.js.resolve.diagnostics + +import org.jetbrains.kotlin.descriptors.DeclarationDescriptor +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +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 + +object JsInheritanceChecker : SimpleDeclarationChecker { + override fun check( + declaration: KtDeclaration, + descriptor: DeclarationDescriptor, + diagnosticHolder: DiagnosticSink, + bindingContext: BindingContext + ) { + if (descriptor is FunctionDescriptor && !DescriptorUtils.isEffectivelyExternal(descriptor) && + isOverridingExternalWithOptionalParams(descriptor) + ) { + diagnosticHolder.report(ErrorsJs.OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS.on(declaration)) + } + } + + private fun isOverridingExternalWithOptionalParams(function: FunctionDescriptor): Boolean { + if (!function.kind.isReal) return false + + for (overriddenFunction in function.overriddenDescriptors.filter { DescriptorUtils.isEffectivelyExternal(it) }) { + if (overriddenFunction.valueParameters.any { it.hasDefaultValue() }) return true + } + + return false + } +}