From 70a5fd2ba7bf60bad94225cf5de5617147519d06 Mon Sep 17 00:00:00 2001 From: "Rodrigo B. de Oliveira" Date: Tue, 29 Oct 2019 21:43:05 -0300 Subject: [PATCH] Fix Kotlin DSL accessor for `android.kotlinOptions` And introduce a stricter accessor type precedence order. The accessor type is the first public Kotlin type found in the given extension type hierarchy considering all the types in the following order: * the extension type itsef * its superclasses excluding `java.lang.Object` * all supported interfaces ordered by: - subtyping first: subtypes before supertypes - where it's found in the class hierarchy: subclass interfaces before superclass interfaces Fixes #11083 --- .../kotlin-dsl-provider-plugins.gradle.kts | 1 + .../plugins/DefaultProjectSchemaProvider.kt | 88 ++++++++++++------ .../precompiled/AccessorTypePrecedenceTest.kt | 90 +++++++++++++++++++ .../DefaultProjectSchemaProviderTest.kt | 89 ++++++++++++++++++ 4 files changed, 241 insertions(+), 27 deletions(-) create mode 100644 subprojects/kotlin-dsl-provider-plugins/src/test/kotlin/org/gradle/kotlin/dsl/provider/plugins/precompiled/AccessorTypePrecedenceTest.kt create mode 100644 subprojects/kotlin-dsl-provider-plugins/src/test/kotlin/org/gradle/kotlin/dsl/provider/plugins/precompiled/DefaultProjectSchemaProviderTest.kt diff --git a/subprojects/kotlin-dsl-provider-plugins/kotlin-dsl-provider-plugins.gradle.kts b/subprojects/kotlin-dsl-provider-plugins/kotlin-dsl-provider-plugins.gradle.kts index bf3c195b0b84..b4d5a890e810 100644 --- a/subprojects/kotlin-dsl-provider-plugins/kotlin-dsl-provider-plugins.gradle.kts +++ b/subprojects/kotlin-dsl-provider-plugins/kotlin-dsl-provider-plugins.gradle.kts @@ -48,4 +48,5 @@ dependencies { implementation(library("slf4j_api")) testImplementation(project(":kotlinDslTestFixtures")) + testImplementation(testLibrary("mockito_kotlin2")) } diff --git a/subprojects/kotlin-dsl-provider-plugins/src/main/kotlin/org/gradle/kotlin/dsl/provider/plugins/DefaultProjectSchemaProvider.kt b/subprojects/kotlin-dsl-provider-plugins/src/main/kotlin/org/gradle/kotlin/dsl/provider/plugins/DefaultProjectSchemaProvider.kt index 688d1180e71d..f227ac2e6e5b 100644 --- a/subprojects/kotlin-dsl-provider-plugins/src/main/kotlin/org/gradle/kotlin/dsl/provider/plugins/DefaultProjectSchemaProvider.kt +++ b/subprojects/kotlin-dsl-provider-plugins/src/main/kotlin/org/gradle/kotlin/dsl/provider/plugins/DefaultProjectSchemaProvider.kt @@ -33,13 +33,8 @@ import org.gradle.kotlin.dsl.accessors.ProjectSchemaEntry import org.gradle.kotlin.dsl.accessors.ProjectSchemaProvider import org.gradle.kotlin.dsl.accessors.SchemaType import org.gradle.kotlin.dsl.accessors.TypedProjectSchema - -import org.jetbrains.kotlin.utils.addToStdlib.firstNotNullResult - -import kotlin.reflect.KClass -import kotlin.reflect.KVisibility - import java.lang.reflect.Modifier +import kotlin.reflect.KVisibility class DefaultProjectSchemaProvider : ProjectSchemaProvider { @@ -57,7 +52,7 @@ class DefaultProjectSchemaProvider : ProjectSchemaProvider { } -private +internal data class TargetTypedSchema( val extensions: List>>, val conventions: List>>, @@ -66,7 +61,7 @@ data class TargetTypedSchema( ) -private +internal fun targetSchemaFor(target: Any, targetType: TypeOf<*>): TargetTypedSchema { val extensions = mutableListOf>>() @@ -127,42 +122,86 @@ fun accessibleContainerSchema(collectionSchema: NamedDomainObjectCollectionSchem private fun NamedDomainObjectSchema.toFirstKotlinPublicOrSelf() = - publicType.concreteClass.kotlin.let { kotlinType -> + publicType.concreteClass.let { schemaType -> // Because a public Java class might not correspond necessarily to a // public Kotlin type due to Kotlin `internal` semantics, we check // whether the public Java class is also the first public Kotlin type, // otherwise we compute a new schema entry with the correct Kotlin type. - val firstPublicKotlinType = kotlinType.firstKotlinPublicOrSelf + val firstPublicKotlinType = schemaType.firstPublicKotlinAccessorTypeOrSelf when { - firstPublicKotlinType === kotlinType -> this + firstPublicKotlinType === schemaType -> this else -> ProjectSchemaNamedDomainObjectSchema( name, - firstPublicKotlinType.asTypeOf() + TypeOf.typeOf(firstPublicKotlinType) ) } } +internal +val Class<*>.firstPublicKotlinAccessorTypeOrSelf: Class<*> + get() = firstPublicKotlinAccessorType ?: this + + private -val KClass<*>.firstKotlinPublicOrSelf - get() = firstKotlinPublicOrNull ?: this +val Class<*>.firstPublicKotlinAccessorType: Class<*>? + get() = accessorTypePrecedenceSequence().find { it.isKotlinPublic } + + +internal +fun Class<*>.accessorTypePrecedenceSequence(): Sequence> = sequence { + + // First, all the classes in the hierarchy, subclasses before superclasses + val classes = ancestorClassesIncludingSelf.toList() + yieldAll(classes) + + // Then all supported interfaces sorted by subtyping (subtypes before supertypes) + val interfaces = mutableListOf>() + classes.forEach { `class` -> + `class`.interfaces.forEach { `interface` -> + when (val indexOfSupertype = interfaces.indexOfFirst { it.isAssignableFrom(`interface`) }) { + -1 -> interfaces.add(`interface`) + else -> if (interfaces[indexOfSupertype] != `interface`) { + interfaces.add(indexOfSupertype, `interface`) + } + } + } + } + yieldAll(interfaces) +} + + +internal +val Class<*>.ancestorClassesIncludingSelf: Sequence> + get() = sequence { + + yield(this@ancestorClassesIncludingSelf) + + var superclass: Class<*>? = superclass + while (superclass != null) { + val thisSuperclass: Class<*> = superclass + val nextSuperclass = thisSuperclass.superclass + if (nextSuperclass != null) { // skip java.lang.Object + yield(thisSuperclass) + } + superclass = nextSuperclass + } + } private -val KClass<*>.firstKotlinPublicOrNull: KClass<*>? - get() = takeIf { isJavaPublic && isKotlinVisible && visibility == KVisibility.PUBLIC } - ?: (java.superclass as Class<*>?)?.kotlin?.firstKotlinPublicOrNull - ?: java.interfaces.firstNotNullResult { it.kotlin.firstKotlinPublicOrNull } +val Class<*>.isKotlinPublic: Boolean + get() = isKotlinVisible && kotlin.visibility == KVisibility.PUBLIC private -val KClass<*>.isJavaPublic - get() = Modifier.isPublic(java.modifiers) +val Class<*>.isKotlinVisible: Boolean + get() = isPublic && !isLocalClass && !isAnonymousClass && !isSynthetic private -val KClass<*>.isKotlinVisible: Boolean - get() = !java.isLocalClass && !java.isAnonymousClass && !java.isSynthetic +val Class<*>.isPublic + get() = Modifier.isPublic(modifiers) private @@ -227,8 +266,3 @@ val typeOfTaskContainer = typeOf() internal inline fun typeOf(): TypeOf = object : TypeOf() {} - - -private -fun KClass.asTypeOf() = - TypeOf.typeOf(java) diff --git a/subprojects/kotlin-dsl-provider-plugins/src/test/kotlin/org/gradle/kotlin/dsl/provider/plugins/precompiled/AccessorTypePrecedenceTest.kt b/subprojects/kotlin-dsl-provider-plugins/src/test/kotlin/org/gradle/kotlin/dsl/provider/plugins/precompiled/AccessorTypePrecedenceTest.kt new file mode 100644 index 000000000000..d1f859a2b3a9 --- /dev/null +++ b/subprojects/kotlin-dsl-provider-plugins/src/test/kotlin/org/gradle/kotlin/dsl/provider/plugins/precompiled/AccessorTypePrecedenceTest.kt @@ -0,0 +1,90 @@ +/* + * Copyright 2019 the original author or authors. + * + * 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.gradle.kotlin.dsl.provider.plugins.precompiled + +import org.gradle.api.plugins.ExtensionAware +import org.gradle.kotlin.dsl.provider.plugins.accessorTypePrecedenceSequence +import org.gradle.kotlin.dsl.provider.plugins.ancestorClassesIncludingSelf +import org.hamcrest.CoreMatchers.equalTo +import org.hamcrest.MatcherAssert.assertThat +import org.junit.Test +import kotlin.reflect.KClass + + +class AccessorTypePrecedenceTest { + + @Test + fun `classes before interfaces`() { + assertAccessorTypePrecedenceOf( + ChooseA::class, + Base::class, + Core::class, + A::class, // because it appears first than B when walking up the tree starting at ChooseA + B::class, + ExtensionAware::class + ) + } + + @Test + fun `classes before interfaces, subtypes before supertypes`() { + assertAccessorTypePrecedenceOf( + ChooseB::class, + Base::class, + Core::class, + B::class, // because B extends ExtensionAware, even though ExtensionAware appears first + ExtensionAware::class + ) + } + + @Test + fun `ancestorClassesIncludingSelf does not include Any`() { + assertTypeSequence( + ChooseA::class.java.ancestorClassesIncludingSelf, + ChooseA::class, + Base::class, + Core::class + ) + } + + private + inline fun assertAccessorTypePrecedenceOf(vararg types: KClass<*>) { + assertTypeSequence( + T::class.java.accessorTypePrecedenceSequence(), + *types + ) + } + + private + fun assertTypeSequence(sequence: Sequence>, vararg types: KClass<*>) { + assertThat( + sequence.mapTo(mutableListOf()) { it.simpleName }, + equalTo(types.map { it.simpleName }) + ) + } + + abstract class ChooseA : Base(), ExtensionAware, A + + abstract class ChooseB : Base(), ExtensionAware + + abstract class Base : Core(), B + + open class Core + + interface A : ExtensionAware + + interface B : ExtensionAware +} diff --git a/subprojects/kotlin-dsl-provider-plugins/src/test/kotlin/org/gradle/kotlin/dsl/provider/plugins/precompiled/DefaultProjectSchemaProviderTest.kt b/subprojects/kotlin-dsl-provider-plugins/src/test/kotlin/org/gradle/kotlin/dsl/provider/plugins/precompiled/DefaultProjectSchemaProviderTest.kt new file mode 100644 index 000000000000..ed88262a7fdd --- /dev/null +++ b/subprojects/kotlin-dsl-provider-plugins/src/test/kotlin/org/gradle/kotlin/dsl/provider/plugins/precompiled/DefaultProjectSchemaProviderTest.kt @@ -0,0 +1,89 @@ +/* + * Copyright 2019 the original author or authors. + * + * 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.gradle.kotlin.dsl.provider.plugins.precompiled + +import com.nhaarman.mockitokotlin2.doReturn +import com.nhaarman.mockitokotlin2.mock +import org.gradle.api.internal.plugins.ExtensionContainerInternal +import org.gradle.api.plugins.ExtensionAware +import org.gradle.api.plugins.ExtensionsSchema +import org.gradle.api.reflect.TypeOf +import org.gradle.internal.extensibility.DefaultExtensionsSchema +import org.gradle.kotlin.dsl.accessors.ProjectSchemaEntry +import org.gradle.kotlin.dsl.provider.plugins.targetSchemaFor +import org.gradle.kotlin.dsl.provider.plugins.typeOf +import org.hamcrest.CoreMatchers.equalTo +import org.hamcrest.MatcherAssert.assertThat +import org.junit.Test + + +class DefaultProjectSchemaProviderTest { + + @Test + fun `chooses first public interface in type hierarchy`() { + + val androidExtensionsSchema = DefaultExtensionsSchema.create( + listOf( + extensionSchema( + "kotlinOptions", + typeOf() + ) + ) + ) + + val androidExtensions = mock { + on { extensionsSchema } doReturn androidExtensionsSchema + on { getByName("kotlinOptions") } doReturn KotlinJvmOptionsImpl() + } + + val androidExtension = mock { + on { extensions } doReturn androidExtensions + } + + assertThat( + targetSchemaFor( + androidExtension, + typeOf() + ).extensions, + equalTo( + listOf( + ProjectSchemaEntry( + typeOf(), + "kotlinOptions", + typeOf() + ) + ) + ) + ) + } + + interface AndroidExtension : ExtensionAware + + internal + class KotlinJvmOptionsImpl : KotlinJvmOptionsBase() + + internal + open class KotlinJvmOptionsBase : KotlinJvmOptions + + interface KotlinJvmOptions + + private + fun extensionSchema(name: String, publicType: TypeOf): ExtensionsSchema.ExtensionSchema = mock { + on { getName() } doReturn name + on { getPublicType() } doReturn publicType + } +}