From 97a5545bffb0a40da55aceeaf2a678117c5785bf Mon Sep 17 00:00:00 2001 From: Donat Csikos Date: Wed, 27 Oct 2021 13:47:35 +0200 Subject: [PATCH] Adjust script compilation to use jsr305 strict mode This is what we already do for Kotlin plugins. This change makes the Gradle DSL more consistent. --- .../mirrors.settings.gradle.kts | 2 +- .../api/internal/NullableTransformer.java | 38 +++++++++++++++++++ .../gradle/api/file/ContentFilterable.java | 7 +++- .../migration/upgrading_version_7.adoc | 4 ++ .../files/copy/kotlin/build.gradle.kts | 4 +- .../crossCompilation/kotlin/build.gradle.kts | 2 +- .../GradleKotlinDslIntegrationTest.groovy | 2 +- .../jacoco/plugins/JacocoTaskExtension.java | 2 +- .../KotlinScriptCompilerIntegrationTest.kt | 34 +++++++++++++++++ .../kotlin/dsl/ContentFilterableExtensions.kt | 19 ++++++++++ .../kotlin/dsl/support/KotlinCompiler.kt | 4 ++ 11 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 subprojects/base-services/src/main/java/org/gradle/api/internal/NullableTransformer.java create mode 100644 subprojects/kotlin-dsl/src/integTest/kotlin/org/gradle/kotlin/dsl/support/KotlinScriptCompilerIntegrationTest.kt diff --git a/gradle/shared-with-buildSrc/mirrors.settings.gradle.kts b/gradle/shared-with-buildSrc/mirrors.settings.gradle.kts index 48a05bcb7792..1d8cac5c6390 100644 --- a/gradle/shared-with-buildSrc/mirrors.settings.gradle.kts +++ b/gradle/shared-with-buildSrc/mirrors.settings.gradle.kts @@ -53,7 +53,7 @@ fun withMirrors(handler: RepositoryHandler) { if (this is MavenArtifactRepository) { originalUrls.forEach { name, originalUrl -> if (normalizeUrl(originalUrl) == normalizeUrl(this.url.toString()) && mirrorUrls.containsKey(name)) { - this.setUrl(mirrorUrls.get(name)) + mirrorUrls.get(name)?.let { this.setUrl(it) } } } } diff --git a/subprojects/base-services/src/main/java/org/gradle/api/internal/NullableTransformer.java b/subprojects/base-services/src/main/java/org/gradle/api/internal/NullableTransformer.java new file mode 100644 index 000000000000..638380123063 --- /dev/null +++ b/subprojects/base-services/src/main/java/org/gradle/api/internal/NullableTransformer.java @@ -0,0 +1,38 @@ +/* + * Copyright 2021 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.api.internal; + +import org.gradle.api.Transformer; + +import javax.annotation.Nullable; + +/** + *

{@code Transformer} extension that explicitly allows {@code null} return values

+ * + * @param The type the value is transformed to. + * @param The type of the value to be transformed. + */ +public abstract class NullableTransformer implements Transformer { + + @Nullable + @Override + public abstract OUT transform(IN in); + + public Transformer asTransformer() { + return this; + } +} diff --git a/subprojects/core-api/src/main/java/org/gradle/api/file/ContentFilterable.java b/subprojects/core-api/src/main/java/org/gradle/api/file/ContentFilterable.java index f60238ee788d..6cdc6f9d1015 100644 --- a/subprojects/core-api/src/main/java/org/gradle/api/file/ContentFilterable.java +++ b/subprojects/core-api/src/main/java/org/gradle/api/file/ContentFilterable.java @@ -76,7 +76,12 @@ public interface ContentFilterable { * Adds a content filter based on the provided transformer. The Closure will be called with each line (stripped of line * endings) and should return a String to replace the line or {@code null} to remove the line. If every line is * removed, the result will be an empty file, not an absent one. - * + *

+ * Note, that due to the nullability constraints clients written in Kotlin cannot return null values as it results in a + * compile-time error. To fix that, the {@code nullableTransformer} utility method should be used: + * 

+     *     filter(nullableTransformer { line -> ... })
+     *
* @param transformer to implement line based filtering * @return this */ diff --git a/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc b/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc index ade39fb5985c..de75f85ede72 100644 --- a/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc +++ b/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc @@ -38,6 +38,10 @@ Some plugins will break with this new version of Gradle, for example because the === Potential breaking changes +=== Kotlin build scripts compilation enforces non-null APIs + +If a Kotlin build script uses a Java API that has nullability annotations (JSR-305) then the compilation will enforce those constraints. Previous Gradle versions, however, forgot to add the same enforcement to the Kotlin build script compilation. Starting from Gradle 7.4 the nullability checks are enforced in the script compilation. + ==== Updates to default tool integration versions - PMD has been updated to https://github.com/pmd/pmd/releases/tag/pmd_releases%2F6.39.0[PMD 6.39.0]. diff --git a/subprojects/docs/src/snippets/files/copy/kotlin/build.gradle.kts b/subprojects/docs/src/snippets/files/copy/kotlin/build.gradle.kts index 37dfb87aac59..eaa7c7f33796 100644 --- a/subprojects/docs/src/snippets/files/copy/kotlin/build.gradle.kts +++ b/subprojects/docs/src/snippets/files/copy/kotlin/build.gradle.kts @@ -218,9 +218,9 @@ tasks.register("filter") { "[$line]" } // Use a closure to remove lines - filter { line: String -> + filter(nullableTransformer { line: String -> if (line.startsWith('-')) null else line - } + }) filteringCharset = "UTF-8" } // end::filter-files[] diff --git a/subprojects/docs/src/snippets/java/crossCompilation/kotlin/build.gradle.kts b/subprojects/docs/src/snippets/java/crossCompilation/kotlin/build.gradle.kts index fdf7a1732b7e..27a5906b7d6a 100644 --- a/subprojects/docs/src/snippets/java/crossCompilation/kotlin/build.gradle.kts +++ b/subprojects/docs/src/snippets/java/crossCompilation/kotlin/build.gradle.kts @@ -21,7 +21,7 @@ java { // end::java-cross-compilation[] tasks.withType().configureEach { - systemProperty("targetJavaVersion", project.findProperty("targetJavaVersion")) + project.findProperty("targetJavaVersion")?.let { systemProperty("targetJavaVersion", it) } } tasks.register("checkJavadocOutput") { diff --git a/subprojects/integ-test/src/integTest/groovy/org/gradle/integtests/GradleKotlinDslIntegrationTest.groovy b/subprojects/integ-test/src/integTest/groovy/org/gradle/integtests/GradleKotlinDslIntegrationTest.groovy index 89fff3ad02d3..9cb836769783 100644 --- a/subprojects/integ-test/src/integTest/groovy/org/gradle/integtests/GradleKotlinDslIntegrationTest.groovy +++ b/subprojects/integ-test/src/integTest/groovy/org/gradle/integtests/GradleKotlinDslIntegrationTest.groovy @@ -147,7 +147,7 @@ import org.gradle.tooling.provider.model.ToolingModelBuilderRegistry task("dumpKotlinBuildScriptModelClassPath") { doLast { - val modelName = KotlinBuildScriptModel::class.qualifiedName + val modelName = KotlinBuildScriptModel::class.qualifiedName!! val builderRegistry = (project as ProjectInternal).services[ToolingModelBuilderRegistry::class.java] val builder = builderRegistry.getBuilder(modelName) val model = builder.buildAll(modelName, project) as KotlinBuildScriptModel diff --git a/subprojects/jacoco/src/main/java/org/gradle/testing/jacoco/plugins/JacocoTaskExtension.java b/subprojects/jacoco/src/main/java/org/gradle/testing/jacoco/plugins/JacocoTaskExtension.java index cf34d17a6ab1..4323b9879dc9 100644 --- a/subprojects/jacoco/src/main/java/org/gradle/testing/jacoco/plugins/JacocoTaskExtension.java +++ b/subprojects/jacoco/src/main/java/org/gradle/testing/jacoco/plugins/JacocoTaskExtension.java @@ -123,7 +123,7 @@ public void setDestinationFile(Provider destinationFile) { this.destinationFile.set(destinationFile); } - public void setDestinationFile(File destinationFile) { + public void setDestinationFile(@Nullable File destinationFile) { // nullability must match on getter and setter argument to end up with a writable Kotlin property this.destinationFile.set(destinationFile); } diff --git a/subprojects/kotlin-dsl/src/integTest/kotlin/org/gradle/kotlin/dsl/support/KotlinScriptCompilerIntegrationTest.kt b/subprojects/kotlin-dsl/src/integTest/kotlin/org/gradle/kotlin/dsl/support/KotlinScriptCompilerIntegrationTest.kt new file mode 100644 index 000000000000..6e6c8b24eb4a --- /dev/null +++ b/subprojects/kotlin-dsl/src/integTest/kotlin/org/gradle/kotlin/dsl/support/KotlinScriptCompilerIntegrationTest.kt @@ -0,0 +1,34 @@ +package org.gradle.kotlin.dsl.support + +import org.gradle.kotlin.dsl.fixtures.AbstractKotlinIntegrationTest +import org.junit.Test +import spock.lang.Issue + + +class KotlinScriptCompilerIntegrationTest : AbstractKotlinIntegrationTest() { + + @Test + @Issue("https://github.com/gradle/gradle/issues/18714") + fun `Build scripts are compiled with jsr305 strict mode`() { + + // given: + withFile( + "buildSrc/src/main/java/StringTransformer.java", + """ + @org.gradle.api.NonNullApi + public interface StringTransformer { + String transform(String input); + } + """ + ) + + withBuildScript("StringTransformer { input -> null }") + + // when: + val result = buildAndFail("help") + + // then: + result.assertHasDescription("Script compilation error") + result.hasErrorOutput("Null can not be a value of a non-null type String") + } +} diff --git a/subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/ContentFilterableExtensions.kt b/subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/ContentFilterableExtensions.kt index 13029d8d7dee..e256a2234e38 100644 --- a/subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/ContentFilterableExtensions.kt +++ b/subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/ContentFilterableExtensions.kt @@ -15,7 +15,10 @@ */ package org.gradle.kotlin.dsl +import org.gradle.api.Incubating +import org.gradle.api.Transformer import org.gradle.api.file.ContentFilterable +import org.gradle.api.internal.NullableTransformer import java.io.FilterReader import kotlin.reflect.KClass @@ -111,3 +114,19 @@ fun ContentFilterable.filter(filterType: KClass, vararg pr fun ContentFilterable.filter(filterType: KClass, properties: Map) = if (properties.isEmpty()) filter(filterType.java) else filter(properties, filterType.java) + + +/** + * Creates a new transformer that can return null and can be used in the context of [ContentFilterable.filter]. + * + * @param transformer the spec of the transformer to be returned. + * @since 7.4 + */ +@Incubating +fun ContentFilterable.nullableTransformer(transformer: (String) -> String?): Transformer { + return object : NullableTransformer() { + override fun transform(input: String): String? { + return transformer.invoke(input) + } + }.asTransformer() +} diff --git a/subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/support/KotlinCompiler.kt b/subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/support/KotlinCompiler.kt index e8e13a8928af..3b59b0aaade5 100644 --- a/subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/support/KotlinCompiler.kt +++ b/subprojects/kotlin-dsl/src/main/kotlin/org/gradle/kotlin/dsl/support/KotlinCompiler.kt @@ -57,6 +57,9 @@ import org.jetbrains.kotlin.config.LanguageVersion import org.jetbrains.kotlin.config.LanguageVersionSettingsImpl import org.jetbrains.kotlin.extensions.StorageComponentContainerContributor.Companion.registerExtension +import org.jetbrains.kotlin.load.java.JavaTypeEnhancementState +import org.jetbrains.kotlin.load.java.Jsr305Settings +import org.jetbrains.kotlin.load.java.ReportLevel import org.jetbrains.kotlin.name.NameUtils @@ -352,6 +355,7 @@ val gradleKotlinDslLanguageVersionSettings = LanguageVersionSettingsImpl( apiVersion = ApiVersion.KOTLIN_1_4, analysisFlags = mapOf( AnalysisFlags.skipMetadataVersionCheck to true, + JvmAnalysisFlags.javaTypeEnhancementState to JavaTypeEnhancementState(Jsr305Settings(ReportLevel.STRICT, ReportLevel.STRICT)) { ReportLevel.STRICT }, JvmAnalysisFlags.jvmDefaultMode to JvmDefaultMode.ENABLE, ), specificFeatures = mapOf(