From 204873edf200eab19f59b0d7440782abe671e56e Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 25 Nov 2016 17:10:46 +0300 Subject: [PATCH] Support "-Xno-check-impl" argument, check only real declarations Use "-Xno-check-impl" to suppress checking whether the platform declaration implementation has the "impl" modifier. Do not check presence of fake overrides from platform class in the impl class, otherwise there would be a lot of errors about the fact that equals/hashCode/toString are not marked with the "impl" modifier --- .../arguments/CommonCompilerArguments.java | 3 ++ .../kotlin/cli/common/CLICompiler.java | 3 ++ .../PlatformImplDeclarationChecker.kt | 45 +++++++++++++------ compiler/testData/cli/js/jsExtraHelp.out | 1 + compiler/testData/cli/jvm/extraHelp.out | 1 + .../classScopes/fakeOverrides/common.kt | 7 +++ .../classScopes/fakeOverrides/jvm.kt | 7 +++ .../classScopes/fakeOverrides/output.txt | 7 +++ compiler/testData/multiplatform/simple/js.kt | 2 +- compiler/testData/multiplatform/simple/jvm.kt | 2 +- ...MultiPlatformIntegrationTestGenerated.java | 6 +++ .../kotlin/config/LanguageVersionSettings.kt | 1 + 12 files changed, 70 insertions(+), 15 deletions(-) create mode 100644 compiler/testData/multiplatform/classScopes/fakeOverrides/common.kt create mode 100644 compiler/testData/multiplatform/classScopes/fakeOverrides/jvm.kt create mode 100644 compiler/testData/multiplatform/classScopes/fakeOverrides/output.txt diff --git a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/CommonCompilerArguments.java b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/CommonCompilerArguments.java index 6e3fb842e9b..01924dde85e 100644 --- a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/CommonCompilerArguments.java +++ b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/CommonCompilerArguments.java @@ -67,6 +67,9 @@ public abstract class CommonCompilerArguments { @Argument(value = "Xmulti-platform", description = "Enable experimental language support for multi-platform projects") public boolean multiPlatform; + @Argument(value = "Xno-check-impl", description = "Do not check presence of 'impl' modifier in multi-platform projects") + public boolean noCheckImpl; + @Argument(value = "P", description = "Pass an option to a plugin") @ValueDescription(PLUGIN_OPTION_FORMAT) public String[] pluginOptions; diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/common/CLICompiler.java b/compiler/cli/src/org/jetbrains/kotlin/cli/common/CLICompiler.java index f4a8c7fa9c9..8fb63eda139 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/common/CLICompiler.java +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/common/CLICompiler.java @@ -281,6 +281,9 @@ public abstract class CLICompiler { if (arguments.multiPlatform) { extraLanguageFeatures.add(LanguageFeature.MultiPlatformProjects); } + if (arguments.noCheckImpl) { + extraLanguageFeatures.add(LanguageFeature.MultiPlatformDoNotCheckImpl); + } configuration.put( CommonConfigurationKeys.LANGUAGE_VERSION_SETTINGS, diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/PlatformImplDeclarationChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/PlatformImplDeclarationChecker.kt index 19db63ff4df..f6440f135c8 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/PlatformImplDeclarationChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/PlatformImplDeclarationChecker.kt @@ -56,7 +56,8 @@ class PlatformImplDeclarationChecker : DeclarationChecker { if (descriptor !is MemberDescriptor) return if (descriptor.isPlatform && declaration.hasModifier(KtTokens.PLATFORM_KEYWORD)) { - checkPlatformDeclarationHasDefinition(declaration, descriptor, diagnosticHolder) + val checkImpl = !languageVersionSettings.supportsFeature(LanguageFeature.MultiPlatformDoNotCheckImpl) + checkPlatformDeclarationHasDefinition(declaration, descriptor, diagnosticHolder, checkImpl) } else if (descriptor.isImpl && declaration.hasModifier(KtTokens.IMPL_KEYWORD)) { checkImplementationHasPlatformDeclaration(declaration, descriptor, diagnosticHolder) @@ -64,7 +65,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker { } private fun checkPlatformDeclarationHasDefinition( - reportOn: KtDeclaration, descriptor: MemberDescriptor, diagnosticHolder: DiagnosticSink + reportOn: KtDeclaration, descriptor: MemberDescriptor, diagnosticHolder: DiagnosticSink, checkImpl: Boolean ) { val compatibility = when (descriptor) { is CallableMemberDescriptor -> { @@ -73,7 +74,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker { // TODO: support non-source definitions (e.g. from Java) DescriptorToSourceUtils.getSourceFromDescriptor(impl) is KtElement }.groupBy { impl -> - areCompatibleCallables(descriptor, impl) + areCompatibleCallables(descriptor, impl, checkImpl) } } is ClassDescriptor -> { @@ -81,7 +82,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker { descriptor != impl && DescriptorToSourceUtils.getSourceFromDescriptor(impl) is KtElement }.groupBy { impl -> - areCompatibleClassifiers(descriptor, impl) + areCompatibleClassifiers(descriptor, impl, checkImpl) } } else -> null @@ -102,7 +103,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker { findClassifiersFromTheSameModule().firstOrNull { declaration -> this != declaration && declaration is ClassDescriptor && declaration.isPlatform && - areCompatibleClassifiers(declaration, this) == Compatible + areCompatibleClassifiers(declaration, this, checkImpl = false) == Compatible } as? ClassDescriptor val hasDeclaration = when (descriptor) { @@ -116,7 +117,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker { candidates.any { declaration -> descriptor != declaration && declaration.isPlatform && - areCompatibleCallables(declaration, descriptor) == Compatible + areCompatibleCallables(declaration, descriptor, checkImpl = false) == Compatible } } is ClassifierDescriptor -> descriptor.findDeclarationForClass() != null @@ -202,6 +203,8 @@ class PlatformImplDeclarationChecker : DeclarationChecker { object TypeParameterVariance : Incompatible("declaration-site variances of type parameters are different") object TypeParameterReified : Incompatible("some type parameter is reified in one declaration and non-reified in the other") + object NoImpl : Incompatible("the implementation is not marked with the 'impl' modifier (-Xno-check-impl)") + object Unknown : Incompatible(null) } @@ -209,7 +212,12 @@ class PlatformImplDeclarationChecker : DeclarationChecker { } // a is the declaration in common code, b is the definition in the platform-specific code - private fun areCompatibleCallables(a: CallableMemberDescriptor, b: CallableMemberDescriptor, parentSubstitutor: Substitutor? = null): Compatibility { + private fun areCompatibleCallables( + a: CallableMemberDescriptor, + b: CallableMemberDescriptor, + checkImpl: Boolean, + parentSubstitutor: Substitutor? = null + ): Compatibility { assert(a.name == b.name) { "This function should be invoked only for declarations with the same name: $a, $b" } assert(a.containingDeclaration is ClassDescriptor == b.containingDeclaration is ClassDescriptor) { "This function should be invoked only for declarations in the same kind of container (both members or both top level): $a, $b" @@ -250,7 +258,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker { else -> throw AssertionError("Unsupported declarations: $a, $b") } - // TODO: check 'impl' modifier + if (checkImpl && !b.isImpl) return Incompatible.NoImpl return Compatible } @@ -278,15 +286,17 @@ class PlatformImplDeclarationChecker : DeclarationChecker { return Compatible } - private fun areCompatibleClassifiers(a: ClassDescriptor, other: ClassifierDescriptor): Compatibility { + private fun areCompatibleClassifiers(a: ClassDescriptor, other: ClassifierDescriptor, checkImpl: Boolean): Compatibility { assert(a.fqNameUnsafe == other.fqNameUnsafe) { "This function should be invoked only for declarations with the same name: $a, $other" } var parentSubstitutor: Substitutor? = null + var implTypealias = false val b = when (other) { is ClassDescriptor -> other is TypeAliasDescriptor -> { val classDescriptor = other.classDescriptor ?: return Compatible // do not report extra error on erroneous typealias + implTypealias = true // If a platform class test.C is implemented by a typealias test.C = test.CImpl, we must now state that any occurrence // of the type "test.C" in the platform class scope should be replaced with "test.CImpl". // Otherwise the types would not be equal and e.g. test.C's constructor is not going to be found in test.CImpl's scope. @@ -317,20 +327,29 @@ class PlatformImplDeclarationChecker : DeclarationChecker { if (!b.typeConstructor.supertypes.containsAll(a.typeConstructor.supertypes.map(substitutor))) return Incompatible.Supertypes - areCompatibleClassScopes(a, b, substitutor).let { if (it != Compatible) return it } + areCompatibleClassScopes(a, b, checkImpl && !implTypealias, substitutor).let { if (it != Compatible) return it } - // TODO: check 'impl' modifier + if (checkImpl && !b.isImpl && !implTypealias) return Incompatible.NoImpl return Compatible } - private fun areCompatibleClassScopes(a: ClassDescriptor, b: ClassDescriptor, substitutor: Substitutor): Compatibility { + private fun areCompatibleClassScopes( + a: ClassDescriptor, + b: ClassDescriptor, + checkImpl: Boolean, + substitutor: Substitutor + ): Compatibility { val unimplemented = arrayListOf>>>() val bMembersByName = b.getMembers().groupBy { it.name } outer@ for (aMember in a.getMembers()) { - val mapping = bMembersByName[aMember.name].orEmpty().keysToMap { bMember -> areCompatibleCallables(aMember, bMember, substitutor) } + if (!aMember.kind.isReal) continue + + val mapping = bMembersByName[aMember.name].orEmpty().keysToMap { bMember -> + areCompatibleCallables(aMember, bMember, checkImpl, substitutor) + } if (mapping.values.any { it == Compatible }) continue val incompatibilityMap = mutableMapOf>() diff --git a/compiler/testData/cli/js/jsExtraHelp.out b/compiler/testData/cli/js/jsExtraHelp.out index 6033fa4a18d..a7f20016bf5 100644 --- a/compiler/testData/cli/js/jsExtraHelp.out +++ b/compiler/testData/cli/js/jsExtraHelp.out @@ -4,6 +4,7 @@ where advanced options include: -Xrepeat Repeat compilation (for performance analysis) -Xplugin Load plugins from the given classpath -Xmulti-platform Enable experimental language support for multi-platform projects + -Xno-check-impl Do not check presence of 'impl' modifier in multi-platform projects Advanced options are non-standard and may be changed or removed without any notice. OK \ No newline at end of file diff --git a/compiler/testData/cli/jvm/extraHelp.out b/compiler/testData/cli/jvm/extraHelp.out index 4bf3faab216..290ad6e4a33 100644 --- a/compiler/testData/cli/jvm/extraHelp.out +++ b/compiler/testData/cli/jvm/extraHelp.out @@ -17,6 +17,7 @@ where advanced options include: -Xrepeat Repeat compilation (for performance analysis) -Xplugin Load plugins from the given classpath -Xmulti-platform Enable experimental language support for multi-platform projects + -Xno-check-impl Do not check presence of 'impl' modifier in multi-platform projects Advanced options are non-standard and may be changed or removed without any notice. OK \ No newline at end of file diff --git a/compiler/testData/multiplatform/classScopes/fakeOverrides/common.kt b/compiler/testData/multiplatform/classScopes/fakeOverrides/common.kt new file mode 100644 index 00000000000..03bf3dfa9fe --- /dev/null +++ b/compiler/testData/multiplatform/classScopes/fakeOverrides/common.kt @@ -0,0 +1,7 @@ +platform open class Base { + fun base() +} + +platform class Derived : Base { + fun derived() +} diff --git a/compiler/testData/multiplatform/classScopes/fakeOverrides/jvm.kt b/compiler/testData/multiplatform/classScopes/fakeOverrides/jvm.kt new file mode 100644 index 00000000000..1fb72b10b89 --- /dev/null +++ b/compiler/testData/multiplatform/classScopes/fakeOverrides/jvm.kt @@ -0,0 +1,7 @@ +impl open class Base { + impl fun base() {} +} + +impl class Derived : Base() { + impl fun derived() {} +} diff --git a/compiler/testData/multiplatform/classScopes/fakeOverrides/output.txt b/compiler/testData/multiplatform/classScopes/fakeOverrides/output.txt new file mode 100644 index 00000000000..b08950d503d --- /dev/null +++ b/compiler/testData/multiplatform/classScopes/fakeOverrides/output.txt @@ -0,0 +1,7 @@ +-- Common -- +Exit code: OK +Output: + +-- JVM -- +Exit code: OK +Output: diff --git a/compiler/testData/multiplatform/simple/js.kt b/compiler/testData/multiplatform/simple/js.kt index 82382628b9b..7849bab9754 100644 --- a/compiler/testData/multiplatform/simple/js.kt +++ b/compiler/testData/multiplatform/simple/js.kt @@ -1,5 +1,5 @@ impl class Printer { - fun print(message: String) { + impl fun print(message: String) { println("JS says: " + message) } } diff --git a/compiler/testData/multiplatform/simple/jvm.kt b/compiler/testData/multiplatform/simple/jvm.kt index d219f769753..6dc952b2562 100644 --- a/compiler/testData/multiplatform/simple/jvm.kt +++ b/compiler/testData/multiplatform/simple/jvm.kt @@ -1,5 +1,5 @@ impl class Printer { - fun print(message: String) { + impl fun print(message: String) { println("JVM says: " + message) } } diff --git a/compiler/tests/org/jetbrains/kotlin/multiplatform/MultiPlatformIntegrationTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/multiplatform/MultiPlatformIntegrationTestGenerated.java index dbcad671508..b44efed9dc9 100644 --- a/compiler/tests/org/jetbrains/kotlin/multiplatform/MultiPlatformIntegrationTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/multiplatform/MultiPlatformIntegrationTestGenerated.java @@ -98,6 +98,12 @@ public class MultiPlatformIntegrationTestGenerated extends AbstractMultiPlatform doTest(fileName); } + @TestMetadata("fakeOverrides") + public void testFakeOverrides() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/multiplatform/classScopes/fakeOverrides/"); + doTest(fileName); + } + @TestMetadata("functionIncorrectSignature") public void testFunctionIncorrectSignature() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/multiplatform/classScopes/functionIncorrectSignature/"); diff --git a/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt b/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt index ed315134ccf..a8943d7a6f5 100644 --- a/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt +++ b/compiler/util/src/org/jetbrains/kotlin/config/LanguageVersionSettings.kt @@ -36,6 +36,7 @@ enum class LanguageFeature(val sinceVersion: LanguageVersion?) { // Experimental features MultiPlatformProjects(null), + MultiPlatformDoNotCheckImpl(null), ; val presentableText: String