Skip to content

[WIP] Add detection for unused classpath entries as part of strict mode #438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion scala/scala.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}
Expand Down
2 changes: 2 additions & 0 deletions src/java/io/bazel/rulesscala/scalac/CompileOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -56,6 +57,7 @@ public CompileOptions(List<String> args) {
resourceJars = getCommaList(argMap, "ResourceJars");

directJars = getCommaList(argMap, "DirectJars");
directTargets = getCommaList(argMap, "DirectTargets");
indirectJars = getCommaList(argMap, "IndirectJars");
indirectTargets = getCommaList(argMap, "IndirectTargets");

Expand Down
6 changes: 4 additions & 2 deletions src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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
Expand All @@ -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
Expand All @@ -54,16 +62,35 @@ 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]) = {
for (usedJar <- usedJars;
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down