From 1a7e8b06903e3f8d5de3af6155c3551b1e875753 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Tue, 20 Dec 2016 15:01:33 +0300 Subject: [PATCH] JS: allow to omit delegated constructor call for external classes in common FE. Prohibit delegated constructor call for external classes in JS FE. --- .../kotlin/resolve/BodyResolver.java | 6 ++-- .../module/wrongCallToModule.kt | 3 +- .../module/wrongCallToModule.txt | 8 ----- .../module/wrongCallToNonModule.kt | 3 +- .../module/wrongCallToNonModule.txt | 8 ----- .../native/delegatedConstructorCall.kt | 18 ++++++++++ .../native/delegatedConstructorCall.txt | 27 +++++++++++++++ .../testsWithJsStdLib/native/delegation.kt | 18 ++++++++++ .../testsWithJsStdLib/native/delegation.txt | 34 +++++++++++++++++++ .../testsWithJsStdLib/native/inheritance.kt | 4 +-- .../native/overrideOptionalParam.kt | 6 ++-- .../DiagnosticsTestWithJsStdLibGenerated.java | 12 +++++++ .../diagnostics/DefaultErrorMessagesJs.kt | 3 ++ .../js/resolve/diagnostics/ErrorsJs.java | 4 +++ .../resolve/diagnostics/JsExternalChecker.kt | 29 ++++++++++++++++ js/js.libraries/src/jquery/common.kt | 2 +- .../box/crossModuleRef/constructor.kt | 2 +- .../box/inheritance/nativeNativeKotlin.kt | 2 +- 18 files changed, 160 insertions(+), 29 deletions(-) create mode 100644 compiler/testData/diagnostics/testsWithJsStdLib/native/delegatedConstructorCall.kt create mode 100644 compiler/testData/diagnostics/testsWithJsStdLib/native/delegatedConstructorCall.txt create mode 100644 compiler/testData/diagnostics/testsWithJsStdLib/native/delegation.kt create mode 100644 compiler/testData/diagnostics/testsWithJsStdLib/native/delegation.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java index a776c83929d..414bf8e9c12 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java @@ -169,8 +169,8 @@ public class BodyResolver { @NotNull KtSecondaryConstructor constructor, @NotNull ClassConstructorDescriptor descriptor ) { - if (descriptor.isHeader()) { - // For header classes, we do not resolve constructor delegation calls because they are prohibited + if (descriptor.isHeader() || DescriptorUtils.isEffectivelyExternal(descriptor)) { + // For header and external classes, we do not resolve constructor delegation calls because they are prohibited return DataFlowInfo.Companion.getEMPTY(); } @@ -367,7 +367,7 @@ public class BodyResolver { descriptor.getUnsubstitutedPrimaryConstructor() != null && superClass.getKind() != ClassKind.INTERFACE && !superClass.getConstructors().isEmpty() && - !descriptor.isHeader() && + !descriptor.isHeader() && !DescriptorUtils.isEffectivelyExternal(descriptor) && !ErrorUtils.isError(superClass) ) { trace.report(SUPERTYPE_NOT_INITIALIZED.on(specifier)); diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToModule.kt b/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToModule.kt index d6ed0cd0476..e87532642e3 100644 --- a/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToModule.kt +++ b/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToModule.kt @@ -39,4 +39,5 @@ fun box() { baz() } -external class DerivedB : B() \ No newline at end of file +// TODO: fix and uncomment +// external class DerivedB : B< !> \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToModule.txt b/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToModule.txt index 9db3570a9f2..8c4a6a6bcd3 100644 --- a/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToModule.txt +++ b/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToModule.txt @@ -28,14 +28,6 @@ package package bar { public fun box(): kotlin.Unit - - public external final class DerivedB : foo.B { - public constructor DerivedB() - public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public final override /*1*/ /*fake_override*/ fun foo(): kotlin.Int - public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int - public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String - } } package foo { diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToNonModule.kt b/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToNonModule.kt index f767317d939..8914b077c72 100644 --- a/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToNonModule.kt +++ b/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToNonModule.kt @@ -33,4 +33,5 @@ fun box() { bar() } -external class DerivedB : B() \ No newline at end of file +// TODO: fix and uncomment +// external class DerivedB : B< !> \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToNonModule.txt b/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToNonModule.txt index 697330909bc..795baa56d40 100644 --- a/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToNonModule.txt +++ b/compiler/testData/diagnostics/testsWithJsStdLib/module/wrongCallToNonModule.txt @@ -2,14 +2,6 @@ package package bar { public fun box(): kotlin.Unit - - public external final class DerivedB : foo.B { - public constructor DerivedB() - public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public final override /*1*/ /*fake_override*/ fun foo(): kotlin.Int - public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int - public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String - } } package foo { diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/native/delegatedConstructorCall.kt b/compiler/testData/diagnostics/testsWithJsStdLib/native/delegatedConstructorCall.kt new file mode 100644 index 00000000000..84bebb6edf6 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithJsStdLib/native/delegatedConstructorCall.kt @@ -0,0 +1,18 @@ +// !DIAGNOSTICS: -DEBUG_INFO_MISSING_UNRESOLVED + +external open class Base(x: Int) { + constructor(x: String) : this(23) + + constructor(x: String, y: String) : this("") +} + +external open class Derived1() : Base(23) { + constructor(x: Byte) : super(23) + + constructor(x: String) : super("") + + constructor(x: String, y: String) : super("") +} + +external open class Derived2() : Base("") + diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/native/delegatedConstructorCall.txt b/compiler/testData/diagnostics/testsWithJsStdLib/native/delegatedConstructorCall.txt new file mode 100644 index 00000000000..36d27fc63e9 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithJsStdLib/native/delegatedConstructorCall.txt @@ -0,0 +1,27 @@ +package + +public external open class Base { + public constructor Base(/*0*/ x: kotlin.Int) + public constructor Base(/*0*/ x: kotlin.String) + public constructor Base(/*0*/ x: kotlin.String, /*1*/ y: kotlin.String) + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public external open class Derived1 : Base { + public constructor Derived1() + public constructor Derived1(/*0*/ x: kotlin.Byte) + public constructor Derived1(/*0*/ x: kotlin.String) + public constructor Derived1(/*0*/ x: kotlin.String, /*1*/ y: kotlin.String) + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public external open class Derived2 : Base { + public constructor Derived2() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/native/delegation.kt b/compiler/testData/diagnostics/testsWithJsStdLib/native/delegation.kt new file mode 100644 index 00000000000..dad99748ed0 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithJsStdLib/native/delegation.kt @@ -0,0 +1,18 @@ +external interface I + +external object O : I + + +class Delegate { + operator fun getValue(thisRef: Any?, property: Any): String = "" + + operator fun setValue(thisRef: Any?, property: Any, value: String) {} +} + +external class A : I by O { + val prop by Delegate() + + var mutableProp by Delegate() +} + +external val topLevelProp by Delegate() \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/native/delegation.txt b/compiler/testData/diagnostics/testsWithJsStdLib/native/delegation.txt new file mode 100644 index 00000000000..bd8b368e4df --- /dev/null +++ b/compiler/testData/diagnostics/testsWithJsStdLib/native/delegation.txt @@ -0,0 +1,34 @@ +package + +public external val topLevelProp: kotlin.String + +public external final class A : I { + public constructor A() + public final var mutableProp: kotlin.String + public final val prop: kotlin.String + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class Delegate { + public constructor Delegate() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final operator fun getValue(/*0*/ thisRef: kotlin.Any?, /*1*/ property: kotlin.Any): kotlin.String + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final operator fun setValue(/*0*/ thisRef: kotlin.Any?, /*1*/ property: kotlin.Any, /*2*/ value: kotlin.String): kotlin.Unit + public open override /*1*/ /*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 open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public external object O : I { + private constructor O() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/native/inheritance.kt b/compiler/testData/diagnostics/testsWithJsStdLib/native/inheritance.kt index a53f2814a24..c649b761956 100644 --- a/compiler/testData/diagnostics/testsWithJsStdLib/native/inheritance.kt +++ b/compiler/testData/diagnostics/testsWithJsStdLib/native/inheritance.kt @@ -6,9 +6,9 @@ interface I external open class B -external class C : A() +external class C : A -external class D : B(), I +external class D : B, I @native class N : A() diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt b/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt index 1bdb2180a1b..26fc47af435 100644 --- a/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt +++ b/compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt @@ -6,7 +6,7 @@ class B : A() { override fun f(x: Int) {} } -external class C : A() { +external class C : A { override fun f(x: Int) } @@ -31,7 +31,7 @@ class F : D(), I { override fun f(x: Int) {} } -external class G : D(), I { +external class G : D, I { override fun f(x: Int) } @@ -45,6 +45,6 @@ open external class XE { class Y : X(), I -external class YE: XE(), I +external class YE: XE, I class Z : X(), J \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithJsStdLibGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithJsStdLibGenerated.java index d22c9ee826a..08861a7dd61 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithJsStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithJsStdLibGenerated.java @@ -575,6 +575,18 @@ public class DiagnosticsTestWithJsStdLibGenerated extends AbstractDiagnosticsTes doTest(fileName); } + @TestMetadata("delegatedConstructorCall.kt") + public void testDelegatedConstructorCall() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithJsStdLib/native/delegatedConstructorCall.kt"); + doTest(fileName); + } + + @TestMetadata("delegation.kt") + public void testDelegation() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithJsStdLib/native/delegation.kt"); + doTest(fileName); + } + @TestMetadata("extensionFunctionAndProperty.kt") public void testExtensionFunctionAndProperty() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithJsStdLib/native/extensionFunctionAndProperty.kt"); 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 5796ee8de78..745c8b55c1c 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 @@ -75,10 +75,13 @@ private val DIAGNOSTIC_FACTORY_TO_RENDERER by lazy { put(ErrorsJs.INLINE_EXTERNAL_DECLARATION, "Inline external declaration") put(ErrorsJs.NON_ABSTRACT_MEMBER_OF_EXTERNAL_INTERFACE, "Only nullable properties of external interfaces are allowed to be non-abstract") + put(ErrorsJs.WRONG_BODY_OF_EXTERNAL_DECLARATION, "Wrong body of external declaration. Must be either ' = noImpl' or { noImpl }") put(ErrorsJs.WRONG_INITIALIZER_OF_EXTERNAL_DECLARATION, "Wrong initializer of external declaration. Must be ' = noImpl'") put(ErrorsJs.WRONG_DEFAULT_VALUE_FOR_EXTERNAL_FUN_PARAMETER, "Wrong default value for parameter of external function. Must be ' = noImpl'") + put(ErrorsJs.EXTERNAL_DELEGATED_CONSTRUCTOR_CALL, "Delegated constructor call in external class") + put(ErrorsJs.EXTERNAL_DELEGATION, "Using delegate on external declaration") 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 43c1d57818b..56139986e3b 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 @@ -80,9 +80,13 @@ public interface ErrorsJs { ERROR, PositioningStrategies.DECLARATION_SIGNATURE_OR_DEFAULT); DiagnosticFactory0 NON_ABSTRACT_MEMBER_OF_EXTERNAL_INTERFACE = DiagnosticFactory0.create( ERROR, PositioningStrategies.DECLARATION_SIGNATURE_OR_DEFAULT); + + // Diagnostics about exposing implementation detail in external declarations DiagnosticFactory0 WRONG_BODY_OF_EXTERNAL_DECLARATION = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 WRONG_INITIALIZER_OF_EXTERNAL_DECLARATION = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 WRONG_DEFAULT_VALUE_FOR_EXTERNAL_FUN_PARAMETER = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 EXTERNAL_DELEGATED_CONSTRUCTOR_CALL = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 EXTERNAL_DELEGATION = DiagnosticFactory0.create(ERROR); @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 9b94fc81df8..b19ac7799c6 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 @@ -91,6 +91,7 @@ object JsExternalChecker : SimpleDeclarationChecker { } checkBody(declaration, descriptor, diagnosticHolder, bindingContext) + checkDelegation(declaration, descriptor, diagnosticHolder) } private fun checkBody( @@ -114,6 +115,34 @@ object JsExternalChecker : SimpleDeclarationChecker { } } + private fun checkDelegation(declaration: KtDeclaration, descriptor: DeclarationDescriptor, diagnosticHolder: DiagnosticSink) { + if (descriptor !is MemberDescriptor || !DescriptorUtils.isEffectivelyExternal(descriptor)) return + + if (declaration is KtClassOrObject) { + for (superTypeEntry in declaration.superTypeListEntries) { + when (superTypeEntry) { + is KtSuperTypeCallEntry -> { + diagnosticHolder.report(ErrorsJs.EXTERNAL_DELEGATED_CONSTRUCTOR_CALL.on(superTypeEntry.valueArgumentList!!)) + } + is KtDelegatedSuperTypeEntry -> { + diagnosticHolder.report(ErrorsJs.EXTERNAL_DELEGATION.on(superTypeEntry)) + } + } + } + } + else if (declaration is KtSecondaryConstructor) { + val delegationCall = declaration.getDelegationCall() + if (!delegationCall.isImplicit) { + diagnosticHolder.report(ErrorsJs.EXTERNAL_DELEGATED_CONSTRUCTOR_CALL.on(delegationCall)) + } + } + else if (declaration is KtProperty && descriptor !is PropertyAccessorDescriptor) { + declaration.delegate?.let { delegate -> + diagnosticHolder.report(ErrorsJs.EXTERNAL_DELEGATION.on(delegate)) + } + } + } + private fun isDirectlyExternal(declaration: KtDeclaration, descriptor: DeclarationDescriptor): Boolean { if (declaration is KtProperty && descriptor is PropertyAccessorDescriptor) return false diff --git a/js/js.libraries/src/jquery/common.kt b/js/js.libraries/src/jquery/common.kt index e1532ecbf98..0942f12a9ec 100644 --- a/js/js.libraries/src/jquery/common.kt +++ b/js/js.libraries/src/jquery/common.kt @@ -49,7 +49,7 @@ open public external class MouseEvent() { public fun isDefaultPrevented(): Boolean } -public external class MouseClickEvent() : MouseEvent() { +public external class MouseClickEvent() : MouseEvent { public val which: Int } diff --git a/js/js.translator/testData/box/crossModuleRef/constructor.kt b/js/js.translator/testData/box/crossModuleRef/constructor.kt index 47595b09ed8..0f65d33629e 100644 --- a/js/js.translator/testData/box/crossModuleRef/constructor.kt +++ b/js/js.translator/testData/box/crossModuleRef/constructor.kt @@ -7,7 +7,7 @@ class A(val x: Int) { } external class B(x: Int) { - constructor(a: Int, b: Int) : this(0) { noImpl } + constructor(a: Int, b: Int) val x: Int } diff --git a/js/js.translator/testData/box/inheritance/nativeNativeKotlin.kt b/js/js.translator/testData/box/inheritance/nativeNativeKotlin.kt index 25a78feae2d..b8c3a327b30 100644 --- a/js/js.translator/testData/box/inheritance/nativeNativeKotlin.kt +++ b/js/js.translator/testData/box/inheritance/nativeNativeKotlin.kt @@ -6,7 +6,7 @@ external open class A { fun foo(): String } -external open class B : A() { +external open class B : A { fun bar(): String }