Skip to content

Commit

Permalink
Strict Kotlin dependency checking/enforcement.
Browse files Browse the repository at this point in the history
  • Loading branch information
jongerrish committed Jan 4, 2021
1 parent 4fb6e0a commit 7f39b76
Show file tree
Hide file tree
Showing 16 changed files with 124 additions and 6 deletions.
1 change: 1 addition & 0 deletions examples/android/bzl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions kotlin/internal/js/impl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def kt_js_library_impl(ctx):
args.add("--kotlin_js_dir", out_dir.path)
args.add("--kotlin_output_js_jar", ctx.outputs.jar)
args.add("--kotlin_output_srcjar", ctx.outputs.srcjar)
args.add("--strict_kotlin_deps", "off")

args.add_all("--kotlin_js_libraries", libraries, omit_if_empty = False)
args.add_all("--sources", ctx.files.srcs)
Expand Down
1 change: 1 addition & 0 deletions kotlin/internal/jvm/compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions kotlin/internal/toolchains.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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."""
Expand All @@ -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"],
Expand Down
1 change: 1 addition & 0 deletions src/main/kotlin/io/bazel/kotlin/builder/tasks/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
}

Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
27 changes: 27 additions & 0 deletions src/main/kotlin/io/bazel/kotlin/builder/utils/jars/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
],
)
28 changes: 28 additions & 0 deletions src/main/kotlin/io/bazel/kotlin/builder/utils/jars/JarOwner.kt
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
}
1 change: 1 addition & 0 deletions src/main/kotlin/io/bazel/kotlin/plugin/jdeps/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,21 @@ class JdepsGenCommandLineProcessor : CommandLineProcessor {
CliOption("target_label", "<String>", "Label of target being analyzed", required = true)
val DIRECT_DEPENDENCIES_OPTION: CliOption =
CliOption("direct_dependencies", "<List>", "List of targets direct dependencies", required = false, allowMultipleOccurrences = true)
val STRICT_KOTLIN_DEPS_OPTION: CliOption =
CliOption("strict_kotlin_deps", "<String>", "Report strict deps violations", required = true)
}

override val pluginId: String
get() = COMPILER_PLUGIN_ID
override val pluginOptions: Collection<AbstractCliOption>
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}")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ object JdepsGenConfigurationKeys {
val TARGET_LABEL: CompilerConfigurationKey<String> =
CompilerConfigurationKey.create(JdepsGenCommandLineProcessor.TARGET_LABEL_OPTION.description)

/**
* Label of the Bazel target being analyzed.
*/
val STRICT_KOTLIN_DEPS: CompilerConfigurationKey<String> =
CompilerConfigurationKey.create(JdepsGenCommandLineProcessor.STRICT_KOTLIN_DEPS_OPTION.description)

/**
* List of direct dependencies of the target.
*/
Expand Down
36 changes: 32 additions & 4 deletions src/main/kotlin/io/bazel/kotlin/plugin/jdeps/JdepsGenExtension.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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<String, ArrayList<String>>, directDeps: List<String>, 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 {
Expand Down
2 changes: 2 additions & 0 deletions src/main/protobuf/kotlin_model.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down
3 changes: 3 additions & 0 deletions src/test/kotlin/io/bazel/kotlin/builder/utils/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 7f39b76

Please sign in to comment.