From e7921cf7284e65daed8cea67fe799de9920aa68c Mon Sep 17 00:00:00 2001 From: James Judd Date: Tue, 6 Mar 2018 22:00:50 -0700 Subject: [PATCH] [WIP] Add detection for unused classpath entries as part of strict mode --- scala/scala.bzl | 5 ++- .../rulesscala/scalac/CompileOptions.java | 2 + .../rulesscala/scalac/ScalacProcessor.java | 6 ++- .../DependencyAnalyzer.scala | 39 ++++++++++++++++--- .../DependencyAnalyzerTest.scala | 6 ++- .../dependencyanalyzer/TestUtil.scala | 9 +++-- 6 files changed, 52 insertions(+), 15 deletions(-) diff --git a/scala/scala.bzl b/scala/scala.bzl index 791c931b9..69396f64b 100644 --- a/scala/scala.bzl +++ b/scala/scala.bzl @@ -171,17 +171,20 @@ def _compile(ctx, cjars, dep_srcjars, buildijar, transitive_compile_jars, labels compiler_classpath_jars = transitive_compile_jars direct_jars = ",".join([j.path for j in cjars]) + direct_targets = ",".join([labels[j.path] for j in cjars]) indirect_jars = ",".join([j.path for j in transitive_compile_jars]) indirect_targets = ",".join([labels[j.path] for j in transitive_compile_jars]) current_target = str(ctx.label) optional_scalac_args = """ DirectJars: {direct_jars} +DirectTargets: {direct_targets} IndirectJars: {indirect_jars} IndirectTargets: {indirect_targets} CurrentTarget: {current_target} """.format( direct_jars=direct_jars, + direct_targets=direct_targets, indirect_jars=indirect_jars, indirect_targets=indirect_targets, current_target = current_target @@ -193,7 +196,7 @@ CurrentTarget: {current_target} compiler_classpath = separator.join([j.path for j in compiler_classpath_jars]) toolchain = ctx.toolchains['@io_bazel_rules_scala//scala:toolchain_type'] - scalacopts = toolchain.scalacopts + ctx.attr.scalacopts + scalacopts = toolchain.scalacopts + ctx.attr.scalacopts scalac_args = """ Classpath: {cp} diff --git a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java index fbc5c8285..36f5e1173 100644 --- a/src/java/io/bazel/rulesscala/scalac/CompileOptions.java +++ b/src/java/io/bazel/rulesscala/scalac/CompileOptions.java @@ -21,6 +21,7 @@ public class CompileOptions { final public String resourceStripPrefix; final public String[] resourceJars; final public String[] directJars; + final public String[] directTargets; final public String[] indirectJars; final public String[] indirectTargets; final public String dependencyAnalyzerMode; @@ -56,6 +57,7 @@ public CompileOptions(List args) { resourceJars = getCommaList(argMap, "ResourceJars"); directJars = getCommaList(argMap, "DirectJars"); + directTargets = getCommaList(argMap, "DirectTargets"); indirectJars = getCommaList(argMap, "IndirectJars"); indirectTargets = getCommaList(argMap, "IndirectTargets"); diff --git a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java index 5df7873b8..dac2eb2e7 100644 --- a/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java +++ b/src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java @@ -196,13 +196,15 @@ private static String[] getPluginParamsFrom(CompileOptions ops) { String[] pluginParams; if (isModeEnabled(ops.dependencyAnalyzerMode)) { - String[] targets = encodeBazelTargets(ops.indirectTargets); + String[] indirectTargets = encodeBazelTargets(ops.indirectTargets); + String[] directTargets = encodeBazelTargets(ops.directTargets); String currentTarget = encodeBazelTarget(ops.currentTarget); String[] pluginParamsInUse = { "-P:dependency-analyzer:direct-jars:" + String.join(":", ops.directJars), + "-P:dependency-analyzer:direct-targets:" + String.join(":", directTargets), "-P:dependency-analyzer:indirect-jars:" + String.join(":", ops.indirectJars), - "-P:dependency-analyzer:indirect-targets:" + String.join(":", targets), + "-P:dependency-analyzer:indirect-targets:" + String.join(":", indirectTargets), "-P:dependency-analyzer:mode:" + ops.dependencyAnalyzerMode, "-P:dependency-analyzer:current-target:" + currentTarget, }; diff --git a/third_party/plugin/src/main/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzer.scala b/third_party/plugin/src/main/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzer.scala index 65144995f..0fcc3dea7 100644 --- a/third_party/plugin/src/main/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzer.scala +++ b/third_party/plugin/src/main/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzer.scala @@ -3,6 +3,11 @@ package third_party.plugin.src.main.io.bazel.rulesscala.dependencyanalyzer import scala.reflect.io.AbstractFile import scala.tools.nsc.plugins.{Plugin, PluginComponent} import scala.tools.nsc.{Global, Phase} +import scala.Console._ +import java.io.File +import java.net.URI +import scala.tools.nsc.Settings +import scala.tools.nsc.classpath.FlatClassPathFactory class DependencyAnalyzer(val global: Global) extends Plugin { @@ -13,18 +18,21 @@ class DependencyAnalyzer(val global: Global) extends Plugin { val components = List[PluginComponent](Component) var indirect: Map[String, String] = Map.empty - var direct: Set[String] = Set.empty + var direct: Map[String, String] = Map.empty var analyzerMode: String = "error" + var directJars: Seq[String] = Seq.empty var currentTarget: String = "NA" override def processOptions(options: List[String], error: (String) => Unit): Unit = { var indirectJars: Seq[String] = Seq.empty var indirectTargets: Seq[String] = Seq.empty + var directTargets: Seq[String] = Seq.empty for (option <- options) { option.split(":").toList match { - case "direct-jars" :: data => direct = data.toSet - case "indirect-jars" :: data => indirectJars = data; + case "direct-jars" :: data => directJars = data + case "direct-targets" :: data => directTargets = data.map(_.replace(";", ":")) + case "indirect-jars" :: data => indirectJars = data case "indirect-targets" :: data => indirectTargets = data.map(_.replace(";", ":")) case "current-target" :: target => currentTarget = target.map(_.replace(";", ":")).head case "mode" :: mode => analyzerMode = mode.head @@ -33,9 +41,9 @@ class DependencyAnalyzer(val global: Global) extends Plugin { } } indirect = indirectJars.zip(indirectTargets).toMap + direct = directJars.zip(directTargets).toMap } - private object Component extends PluginComponent { val global: DependencyAnalyzer.this.global.type = DependencyAnalyzer.this.global @@ -54,6 +62,25 @@ class DependencyAnalyzer(val global: Global) extends Plugin { val usedJars = findUsedJars warnOnIndirectTargetsFoundIn(usedJars) + warnOnUnusedTargetsFoundIn(usedJars) + } + + private def warnOnUnusedTargetsFoundIn(usedJars: Set[AbstractFile]) = { + val usedJarPaths = usedJars.map(_.path) + for { + directJar <- direct.keys if !usedJarPaths.contains(directJar) + target <- direct.get(directJar) + } { + // TODO: Can we get the correct jar label here? + val errorMessage = + s"""${currentTarget} depends on '${target}' which depends on '${directJar}'. + |${RESET}${GREEN}${directJar} is not used by ${currentTarget}. Please remove it from the deps.${RESET}""".stripMargin + + analyzerMode match { + case "error" => reporter.error(NoPosition, errorMessage) + case "warn" => reporter.warning(NoPosition, errorMessage) + } + } } private def warnOnIndirectTargetsFoundIn(usedJars: Set[AbstractFile]) = { @@ -61,9 +88,9 @@ class DependencyAnalyzer(val global: Global) extends Plugin { usedJarPath = usedJar.path; target <- indirect.get(usedJarPath) if !direct.contains(usedJarPath)) { val errorMessage = - s"""Target '$target' is used but isn't explicitly declared, please add it to the deps. + s"""Target '${target}' is used but isn't explicitly declared, please add it to the deps. |You can use the following buildozer command: - |buildozer 'add deps $target' $currentTarget""".stripMargin + |${RESET}${GREEN}buildozer 'add deps ${target}' ${currentTarget}${RESET}""".stripMargin analyzerMode match { case "error" => reporter.error(NoPosition, errorMessage) diff --git a/third_party/plugin/src/test/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzerTest.scala b/third_party/plugin/src/test/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzerTest.scala index 0701fec9b..d1c4df465 100644 --- a/third_party/plugin/src/test/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzerTest.scala +++ b/third_party/plugin/src/test/io/bazel/rulesscala/dependencyanalyzer/DependencyAnalyzerTest.scala @@ -45,9 +45,11 @@ class DependencyAnalyzerTest { """.stripMargin val commonsTarget = "commonsTarget" - val direct = Seq(apacheCommonsClasspath) + val direct = Map(apacheCommonsClasspath -> commonsTarget) val indirect = Map(apacheCommonsClasspath -> commonsTarget) - run(testCode, withDirect = direct, withIndirect = indirect).noErrorOn(commonsTarget) + val foo = run(testCode, withDirect = direct, withIndirect = indirect) + println(foo) + foo.noErrorOn(commonsTarget) } diff --git a/third_party/plugin/src/test/io/bazel/rulesscala/dependencyanalyzer/TestUtil.scala b/third_party/plugin/src/test/io/bazel/rulesscala/dependencyanalyzer/TestUtil.scala index dd126e854..e57953717 100644 --- a/third_party/plugin/src/test/io/bazel/rulesscala/dependencyanalyzer/TestUtil.scala +++ b/third_party/plugin/src/test/io/bazel/rulesscala/dependencyanalyzer/TestUtil.scala @@ -14,17 +14,18 @@ object TestUtil { final val defaultTarget = "//..." - def run(code: String, withDirect: Seq[String] = Seq.empty, withIndirect: Map[String, String] = Map.empty): Seq[String] = { + def run(code: String, withDirect: Map[String, String] = Map.empty, withIndirect: Map[String, String] = Map.empty): Seq[String] = { val compileOptions = Seq( - constructParam("direct-jars", withDirect), + constructParam("direct-jars", withDirect.keys), + constructParam("direct-targets", withDirect.values), constructParam("indirect-jars", withIndirect.keys), constructParam("indirect-targets", withIndirect.values), constructParam("current-target", Seq(defaultTarget)) ).mkString(" ") - val extraClasspath = withDirect ++ withIndirect.keys + val extraClasspath = withDirect.keys ++ withIndirect.keys - val reporter: StoreReporter = runCompilation(code, compileOptions, extraClasspath) + val reporter: StoreReporter = runCompilation(code, compileOptions, extraClasspath.toSeq) reporter.infos.collect({ case msg if msg.severity == reporter.ERROR => msg.msg }).toSeq }