From b02a6087e921352b98341a03fb166c6bb1e56fcc Mon Sep 17 00:00:00 2001 From: Jonathan Gerrish Date: Sun, 3 Jan 2021 05:52:19 -0800 Subject: [PATCH] Strict Kotlin dependency checking/enforcement. --- examples/android/bzl/BUILD.bazel | 1 + kotlin/internal/jvm/compile.bzl | 1 + kotlin/internal/toolchains.bzl | 12 +++++++ .../io/bazel/kotlin/builder/tasks/BUILD.bazel | 1 + .../kotlin/builder/tasks/KotlinBuilder.kt | 4 ++- .../tasks/jvm/KotlinJvmTaskExecutor.kt | 1 + .../kotlin/builder/utils/jars/BUILD.bazel | 27 ++++++++++++++ .../kotlin/builder/utils/jars/JarOwner.kt | 28 +++++++++++++++ .../io/bazel/kotlin/plugin/jdeps/BUILD.bazel | 1 + .../jdeps/JdepsGenCommandLineProcessor.kt | 5 ++- .../plugin/jdeps/JdepsGenConfigurationKeys.kt | 6 ++++ .../kotlin/plugin/jdeps/JdepsGenExtension.kt | 36 ++++++++++++++++--- src/main/protobuf/kotlin_model.proto | 2 ++ .../builder/tasks/jvm/KotlinWorkerTest.kt | 1 + .../io/bazel/kotlin/builder/utils/BUILD.bazel | 3 ++ 15 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 src/main/kotlin/io/bazel/kotlin/builder/utils/jars/BUILD.bazel create mode 100644 src/main/kotlin/io/bazel/kotlin/builder/utils/jars/JarOwner.kt diff --git a/examples/android/bzl/BUILD.bazel b/examples/android/bzl/BUILD.bazel index 6bb41ae93..1625e06f2 100644 --- a/examples/android/bzl/BUILD.bazel +++ b/examples/android/bzl/BUILD.bazel @@ -13,6 +13,7 @@ define_kt_toolchain( name = "experimental_toolchain", api_version = "1.4", experimental_use_abi_jars = True, + experimental_strict_kotlin_deps = "warn", javac_options = ":default_javac_options", kotlinc_options = ":default_kotlinc_options", language_version = "1.4", diff --git a/kotlin/internal/jvm/compile.bzl b/kotlin/internal/jvm/compile.bzl index a964a4660..c86bc6ddb 100644 --- a/kotlin/internal/jvm/compile.bzl +++ b/kotlin/internal/jvm/compile.bzl @@ -378,6 +378,7 @@ def _run_kt_builder_action( # TODO: Implement Strict Kotlin deps: (https://github.com/bazelbuild/rules_kotlin/issues/419) # This flag is currently unused by the builder but required for the unused_deps tool args.add_all("--direct_dependencies", _java_infos_to_compile_jars(compile_deps.deps)) + args.add("--strict_kotlin_deps", toolchains.kt.experimental_strict_kotlin_deps) args.add_all("--classpath", compile_deps.compile_jars) args.add_all("--sources", srcs.all_srcs, omit_if_empty = True) args.add_all("--source_jars", srcs.src_jars + generated_src_jars, omit_if_empty = True) diff --git a/kotlin/internal/toolchains.bzl b/kotlin/internal/toolchains.bzl index a0d969317..eb33cf42a 100644 --- a/kotlin/internal/toolchains.bzl +++ b/kotlin/internal/toolchains.bzl @@ -78,6 +78,7 @@ def _kotlin_toolchain_impl(ctx): jvm_stdlibs = java_common.merge(compile_time_providers + runtime_providers), js_stdlibs = ctx.attr.js_stdlibs, experimental_use_abi_jars = ctx.attr.experimental_use_abi_jars, + experimental_strict_kotlin_deps = ctx.attr.experimental_strict_kotlin_deps, javac_options = ctx.attr.javac_options[JavacOptions] if ctx.attr.javac_options else None, kotlinc_options = ctx.attr.kotlinc_options[KotlincOptions] if ctx.attr.kotlinc_options else None, empty_jar = ctx.file._empty_jar, @@ -187,6 +188,15 @@ _kt_toolchain = rule( `kt_abi_plugin_incompatible`""", default = False, ), + "experimental_strict_kotlin_deps": attr.string( + doc = "Report strict deps violations", + default = "off", + values = [ + "off", + "warn", + "error", + ], + ), "javac_options": attr.label( doc = "Compiler options for javac", providers = [JavacOptions], @@ -224,6 +234,7 @@ def define_kt_toolchain( api_version = None, jvm_target = None, experimental_use_abi_jars = False, + experimental_strict_kotlin_deps = None, javac_options = None, kotlinc_options = None): """Define the Kotlin toolchain.""" @@ -248,6 +259,7 @@ def define_kt_toolchain( absolute_target("//kotlin/internal:noexperimental_use_abi_jars"): False, "//conditions:default": experimental_use_abi_jars, }), + experimental_strict_kotlin_deps = experimental_strict_kotlin_deps, javac_options = javac_options, kotlinc_options = kotlinc_options, visibility = ["//visibility:public"], diff --git a/src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel b/src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel index b07e40672..4f6c620b3 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel +++ b/src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel @@ -23,6 +23,7 @@ kt_bootstrap_library( deps = [ "//src/main/kotlin/io/bazel/kotlin/builder/toolchain", "//src/main/kotlin/io/bazel/kotlin/builder/utils", + "//src/main/kotlin/io/bazel/kotlin/builder/utils/jars", "//src/main/protobuf:deps_java_proto", "//src/main/protobuf:kotlin_model_java_proto", "//src/main/protobuf:worker_protocol_java_proto", diff --git a/src/main/kotlin/io/bazel/kotlin/builder/tasks/KotlinBuilder.kt b/src/main/kotlin/io/bazel/kotlin/builder/tasks/KotlinBuilder.kt index c0f4025fb..eb0901549 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/tasks/KotlinBuilder.kt +++ b/src/main/kotlin/io/bazel/kotlin/builder/tasks/KotlinBuilder.kt @@ -115,7 +115,8 @@ class KotlinBuilder @Inject internal constructor( GENERATED_JAVA_SRC_JAR("--generated_java_srcjar"), GENERATED_JAVA_STUB_JAR("--kapt_generated_stub_jar"), GENERATED_CLASS_JAR("--kapt_generated_class_jar"), - BUILD_KOTLIN("--build_kotlin"); + BUILD_KOTLIN("--build_kotlin"), + STRICT_KOTLIN_DEPS("--strict_kotlin_deps"), } } @@ -180,6 +181,7 @@ class KotlinBuilder @Inject internal constructor( argMap.mandatorySingle(KotlinBuilderFlags.API_VERSION) toolchainInfoBuilder.commonBuilder.languageVersion = argMap.mandatorySingle(KotlinBuilderFlags.LANGUAGE_VERSION) + strictKotlinDeps = argMap.mandatorySingle(KotlinBuilderFlags.STRICT_KOTLIN_DEPS) this } diff --git a/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt b/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt index d32297bc8..10b2b737a 100644 --- a/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt +++ b/src/main/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinJvmTaskExecutor.kt @@ -69,6 +69,7 @@ class KotlinJvmTaskExecutor @Inject internal constructor( inputs.directDependenciesList.forEach { flag("direct_dependencies", it) } + flag("strict_kotlin_deps", info.strictKotlinDeps) } .given(outputs.jar).notEmpty { append(codeGenArgs()) diff --git a/src/main/kotlin/io/bazel/kotlin/builder/utils/jars/BUILD.bazel b/src/main/kotlin/io/bazel/kotlin/builder/utils/jars/BUILD.bazel new file mode 100644 index 000000000..0e5cc1509 --- /dev/null +++ b/src/main/kotlin/io/bazel/kotlin/builder/utils/jars/BUILD.bazel @@ -0,0 +1,27 @@ +# Copyright 2020 The Bazel Authors. All rights reserved. +# +# 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. +load("@rules_java//java:defs.bzl", "java_library") +load("//src/main/kotlin:bootstrap.bzl", "kt_bootstrap_library") + +kt_bootstrap_library( + name = "jars", + srcs = glob([ + "*.kt", + "**/*.kt", + ]), + visibility = ["//src:__subpackages__"], + deps = [ + "//src/main/protobuf:deps_java_proto", + ], +) diff --git a/src/main/kotlin/io/bazel/kotlin/builder/utils/jars/JarOwner.kt b/src/main/kotlin/io/bazel/kotlin/builder/utils/jars/JarOwner.kt new file mode 100644 index 000000000..2f98b7d16 --- /dev/null +++ b/src/main/kotlin/io/bazel/kotlin/builder/utils/jars/JarOwner.kt @@ -0,0 +1,28 @@ +package io.bazel.kotlin.builder.utils.jars + +import io.bazel.kotlin.builder.utils.jars.JarHelper.Companion.INJECTING_RULE_KIND +import io.bazel.kotlin.builder.utils.jars.JarHelper.Companion.TARGET_LABEL +import java.io.IOException +import java.io.UncheckedIOException +import java.nio.file.Path +import java.util.jar.JarFile + +data class JarOwner(val jar: Path, val label: String? = null, val aspect: String? = null) { + companion object { + fun readJarOwnerFromManifest(jarPath: Path): JarOwner { + try { + JarFile(jarPath.toFile()).use { jarFile -> + val manifest = jarFile.manifest ?: return JarOwner(jarPath) + val attributes = manifest.mainAttributes + val label = attributes[TARGET_LABEL] as String? + ?: return JarOwner(jarPath) + val injectingRuleKind = attributes[INJECTING_RULE_KIND] as String? + return JarOwner(jarPath, label, injectingRuleKind) + } + } catch (e: IOException) { + // This jar file pretty much has to exist. + throw UncheckedIOException(e) + } + } + } +} diff --git a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel index 3bb8b904e..28248aa05 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel @@ -21,6 +21,7 @@ kt_bootstrap_library( srcs = glob(["*.kt"]), visibility = ["//src:__subpackages__"], deps = [ + "//src/main/kotlin/io/bazel/kotlin/builder/utils/jars", "//src/main/protobuf:deps_java_proto", "@com_github_jetbrains_kotlin//:kotlin-compiler", "@kotlin_rules_maven//:com_google_protobuf_protobuf_java", diff --git a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenCommandLineProcessor.kt b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenCommandLineProcessor.kt index d492ba69c..b5b6fefb3 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenCommandLineProcessor.kt +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenCommandLineProcessor.kt @@ -16,18 +16,21 @@ class JdepsGenCommandLineProcessor : CommandLineProcessor { CliOption("target_label", "", "Label of target being analyzed", required = true) val DIRECT_DEPENDENCIES_OPTION: CliOption = CliOption("direct_dependencies", "", "List of targets direct dependencies", required = false, allowMultipleOccurrences = true) + val STRICT_KOTLIN_DEPS_OPTION: CliOption = + CliOption("strict_kotlin_deps", "", "Report strict deps violations", required = true) } override val pluginId: String get() = COMPILER_PLUGIN_ID override val pluginOptions: Collection - get() = listOf(OUTPUT_JDEPS_FILE_OPTION, TARGET_LABEL_OPTION, DIRECT_DEPENDENCIES_OPTION) + get() = listOf(OUTPUT_JDEPS_FILE_OPTION, TARGET_LABEL_OPTION, DIRECT_DEPENDENCIES_OPTION, STRICT_KOTLIN_DEPS_OPTION) override fun processOption(option: AbstractCliOption, value: String, configuration: CompilerConfiguration) { when (option) { OUTPUT_JDEPS_FILE_OPTION -> configuration.put(JdepsGenConfigurationKeys.OUTPUT_JDEPS, value) TARGET_LABEL_OPTION -> configuration.put(JdepsGenConfigurationKeys.TARGET_LABEL, value) DIRECT_DEPENDENCIES_OPTION -> configuration.appendList(JdepsGenConfigurationKeys.DIRECT_DEPENDENCIES, value) + STRICT_KOTLIN_DEPS_OPTION -> configuration.put(JdepsGenConfigurationKeys.STRICT_KOTLIN_DEPS, value) else -> throw CliOptionProcessingException("Unknown option: ${option.optionName}") } } diff --git a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenConfigurationKeys.kt b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenConfigurationKeys.kt index 4d238b678..ff7b2d8c9 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenConfigurationKeys.kt +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenConfigurationKeys.kt @@ -15,6 +15,12 @@ object JdepsGenConfigurationKeys { val TARGET_LABEL: CompilerConfigurationKey = CompilerConfigurationKey.create(JdepsGenCommandLineProcessor.TARGET_LABEL_OPTION.description) + /** + * Label of the Bazel target being analyzed. + */ + val STRICT_KOTLIN_DEPS: CompilerConfigurationKey = + CompilerConfigurationKey.create(JdepsGenCommandLineProcessor.STRICT_KOTLIN_DEPS_OPTION.description) + /** * List of direct dependencies of the target. */ diff --git a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt index bd57a1622..d0493376c 100644 --- a/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt +++ b/src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt @@ -4,14 +4,13 @@ import com.google.devtools.build.lib.view.proto.Deps import com.intellij.mock.MockProject import com.intellij.openapi.project.Project import com.intellij.psi.PsiElement +import io.bazel.kotlin.builder.utils.jars.JarOwner import org.jetbrains.kotlin.analyzer.AnalysisResult import org.jetbrains.kotlin.codegen.ClassBuilder import org.jetbrains.kotlin.codegen.ClassBuilderFactory import org.jetbrains.kotlin.codegen.extensions.ClassBuilderInterceptorExtension import org.jetbrains.kotlin.codegen.inline.RemappingClassBuilder -import org.jetbrains.kotlin.compiler.plugin.ComponentRegistrar import org.jetbrains.kotlin.config.CompilerConfiguration -import org.jetbrains.kotlin.config.JVMConfigurationKeys import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.ModuleDescriptor import org.jetbrains.kotlin.descriptors.SourceElement @@ -22,11 +21,9 @@ import org.jetbrains.kotlin.load.java.structure.impl.classFiles.BinaryJavaClass import org.jetbrains.kotlin.load.kotlin.KotlinJvmBinarySourceElement import org.jetbrains.kotlin.load.kotlin.VirtualFileKotlinClass import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.BindingTrace -import org.jetbrains.kotlin.resolve.descriptorUtil.classId import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe import org.jetbrains.kotlin.resolve.descriptorUtil.getAllSuperclassesWithoutAny import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperInterfaces @@ -35,6 +32,7 @@ import org.jetbrains.kotlin.resolve.jvm.extensions.AnalysisHandlerExtension import org.jetbrains.org.objectweb.asm.commons.Remapper import java.io.BufferedOutputStream import java.io.File +import java.nio.file.Paths /** * Kotlin compiler extension that tracks classes (and corresponding classpath jars) needed to @@ -192,6 +190,36 @@ class JdepsGenExtension( BufferedOutputStream(File(jdepsOutput).outputStream()).use { it.write(rootBuilder.build().toByteArray()) } + + when (configuration.getNotNull(JdepsGenConfigurationKeys.STRICT_KOTLIN_DEPS)) { + "warn" -> checkStrictDeps(result, directDeps, targetLabel) + "error" -> { + if(checkStrictDeps(result, directDeps, targetLabel)) error("Strict Deps Violations - please fix") + } + } + } + + /** + * Prints strict deps warnings and returns true if violations were found. + */ + private fun checkStrictDeps(result: HashMap>, directDeps: List, targetLabel: String): Boolean { + val missingStrictDeps = result.keys + .filter { !directDeps.contains(it) } + .map { JarOwner.readJarOwnerFromManifest(Paths.get(it)).label } + + if (missingStrictDeps.isNotEmpty()) { + val open = "\u001b[35m\u001b[1m" + val close = "\u001b[0m" + val command = + """ + |$open ** Please add the following dependencies:$close ${missingStrictDeps.joinToString(" ")} to $targetLabel + |$open ** You can use the following buildozer command:$close buildozer 'add deps ${missingStrictDeps.joinToString(" ")}' $targetLabel + """.trimMargin() + + println(command) + return true + } + return false } private fun isExternalModuleClass(className: ClassId): Boolean { diff --git a/src/main/protobuf/kotlin_model.proto b/src/main/protobuf/kotlin_model.proto index de53cd6a0..e4d1f9602 100644 --- a/src/main/protobuf/kotlin_model.proto +++ b/src/main/protobuf/kotlin_model.proto @@ -78,6 +78,8 @@ message CompilationTaskInfo { // trace: enables trace logging. // timings: causes timing information to be printed at the of an action. repeated string debug = 9; + // Enable strict dependency checking for Kotlin + string strict_kotlin_deps = 10; } // Mested messages not marked with stable could be refactored. diff --git a/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinWorkerTest.kt b/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinWorkerTest.kt index 85a8ba7ed..d3daa1c4f 100644 --- a/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinWorkerTest.kt +++ b/src/test/kotlin/io/bazel/kotlin/builder/tasks/jvm/KotlinWorkerTest.kt @@ -114,6 +114,7 @@ class KotlinWorkerTest { flag(KotlinBuilderFlags.GENERATED_CLASSDIR, "generated_classes") flag(JavaBuilderFlags.TEMPDIR, "tmp") flag(JavaBuilderFlags.SOURCEGEN_DIR, "generated_sources") + flag(KotlinBuilderFlags.STRICT_KOTLIN_DEPS, "off") cp(JavaBuilderFlags.CLASSPATH, KotlinJvmTestBuilder.KOTLIN_STDLIB.singleCompileJar()) cp(JavaBuilderFlags.DIRECT_DEPENDENCIES, "") info.passthroughFlagsList.forEach { pf -> diff --git a/src/test/kotlin/io/bazel/kotlin/builder/utils/BUILD.bazel b/src/test/kotlin/io/bazel/kotlin/builder/utils/BUILD.bazel index 941b13b29..410bc2ab2 100644 --- a/src/test/kotlin/io/bazel/kotlin/builder/utils/BUILD.bazel +++ b/src/test/kotlin/io/bazel/kotlin/builder/utils/BUILD.bazel @@ -19,6 +19,9 @@ load("//kotlin:kotlin.bzl", "kt_jvm_test") kt_rules_test( name = "SourceJarCreatorTest", srcs = ["jars/SourceJarCreatorTest.java"], + deps = [ + "//src/main/kotlin/io/bazel/kotlin/builder/utils/jars", + ] ) kt_jvm_test(