Skip to content
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

Use a scala compiler plugin to throw errors on missing direct-deps #243

Merged
merged 43 commits into from
Jul 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
bf3aba2
using classpath shrinker plugin to warn about indirect dependency usage
natansil Jun 28, 2017
59a98a4
fix escaping bugs
natansil Jun 29, 2017
69a5942
attempt to put dependency-analyzer inside rules_scala
natansil Jul 2, 2017
9dcbc6e
merge workspaces
natansil Jul 3, 2017
b177a6e
fix bugs
natansil Jul 3, 2017
fbaddba
remove hard coded jar paths
natansil Jul 4, 2017
c1e9e11
fix ci test
natansil Jul 4, 2017
185cb04
undo file formatting
natansil Jul 6, 2017
e2fe576
remove http_archive of old rules_scala version.
natansil Jul 6, 2017
e206014
remove compatiblity trait for scala 2.11/2.12
natansil Jul 7, 2017
c3118ed
rename package to reflect the hosting repo
natansil Jul 7, 2017
263242f
misc changes according to Oscar's CR
natansil Jul 7, 2017
5dfe1ab
fix bug: error message target was incorrect
natansil Jul 9, 2017
4d47aa3
make strict deps apply also to 'file' deps (e.g. '@com_google_guava_g…
natansil Jul 9, 2017
3b338a1
Update CONTRIBUTORS
nadavwe Jul 9, 2017
00f519b
changes according to @ittaiz CR
natansil Jul 9, 2017
32fc403
extract commonality of strict_deps E2E tests
natansil Jul 9, 2017
f75805c
refactor 'update_jars_to_labels'
natansil Jul 10, 2017
44d1942
replace ctx.attr.enable_dependency_analyzer with `True`
natansil Jul 10, 2017
61c2746
pass scala compiler only compiletime transitive jars, not runtime jars
natansil Jul 11, 2017
b5b2975
when strict mode is disabled, pass only direct jars to scala compiler
natansil Jul 11, 2017
df50732
add third_party aread and licensing statement for the dependency_anal…
natansil Jul 11, 2017
9b95403
make 'collection of [labels of jars]'' code more comprehensible
natansil Jul 11, 2017
ffa505c
extract to method
natansil Jul 11, 2017
c9a1826
add scalac plugin license file
natansil Jul 11, 2017
f2cba75
enable dependency analyser for scala_binary
shvar Jul 12, 2017
2654194
error/warn/off toggle for dependency analyzer
shvar Jul 12, 2017
abe4c99
cosmetics
shvar Jul 16, 2017
81062ab
remove coursier from tests. pre-fetch needed jars
natansil Jul 16, 2017
6c5f695
match file name to class name
natansil Jul 16, 2017
f49631f
make _scala_binary_common a bit clearer
natansil Jul 16, 2017
4e619ed
clarify name: provided_cjars -> compiler_classpath_jars
natansil Jul 16, 2017
d67195c
fix warning E2E test issue
natansil Jul 16, 2017
41e0bff
enable strict-deps check for all rules (move dep_analyzer_mode to com…
natansil Jul 16, 2017
36dfa9b
don't collect jars if mode=off
natansil Jul 20, 2017
4348623
add special jars to transitive_cjars to scala_test instead of adding …
natansil Jul 20, 2017
7c8c5f8
Oscar CR changes
natansil Jul 20, 2017
e5a091b
add test that no compilation triggers of target when transitive depen…
natansil Jul 23, 2017
4df8b0f
add buildozer command action to error/warn log for strict deps
natansil Jul 24, 2017
6d368ea
make it clear the strict_deps mode attribut will be replaced soon. ad…
natansil Jul 24, 2017
8a696a6
no_recompilation_on_internal_change CR changes
natansil Jul 24, 2017
25a6266
buildozer commands CR changes
natansil Jul 24, 2017
41b8fa5
expand on recompilation explanation
natansil Jul 24, 2017
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ bazel-*
*.idea
hash1
hash2
.DS_store
2 changes: 2 additions & 0 deletions CONTRIBUTORS
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ Laurent Le Brun <[email protected]>
Nathan Harmata <[email protected]>
Oscar Boykin <[email protected]>
Justine Alexandra Roberts Tunney <[email protected]>
Natan Silnitsky <[email protected]>
Nadav Wexler <[email protected]>
13 changes: 13 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
workspace(name = "io_bazel_rules_scala")



load("//scala:scala.bzl", "scala_repositories", "scala_mvn_artifact")
scala_repositories()

Expand Down Expand Up @@ -28,4 +30,15 @@ maven_jar(
artifact = scala_mvn_artifact("org.psywerx.hairyfotr:linter:0.1.13"),
sha1 = "e5b3e2753d0817b622c32aedcb888bcf39e275b4")

# test of strict deps (scalac plugin UT + E2E)
maven_jar(
name = "com_google_guava_guava_21_0",
artifact = "com.google.guava:guava:21.0",
sha1 = "3a3d111be1be1b745edfa7d91678a12d7ed38709"
)

maven_jar(
name = "org_apache_commons_commons_lang_3_5",
artifact = "org.apache.commons:commons-lang3:3.5",
sha1 = "6c6c702c89bfff3cd9e80b04d668c5e190d588c6"
)
230 changes: 186 additions & 44 deletions scala/scala.bzl

Large diffs are not rendered by default.

22 changes: 19 additions & 3 deletions src/java/io/bazel/rulesscala/scalac/CompileOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ public class CompileOptions {
final public Map<String, String> resourceFiles;
final public String resourceStripPrefix;
final public String[] resourceJars;
final public String[] directJars;
final public String[] indirectJars;
final public String[] indirectTargets;
final public String dependencyAnalyzerMode;
final public String currentTarget;

public CompileOptions(List<String> args) {
Map<String, String> argMap = buildArgMap(args);
Expand Down Expand Up @@ -52,6 +57,13 @@ public CompileOptions(List<String> args) {
resourceFiles = getResources(argMap);
resourceStripPrefix = getOrEmpty(argMap, "ResourceStripPrefix");
resourceJars = getCommaList(argMap, "ResourceJars");

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

dependencyAnalyzerMode = getOrElse(argMap, "DependencyAnalyzerMode", "off");
currentTarget = getOrElse(argMap, "CurrentTarget", "NA");
}

private static Map<String, String> getResources(Map<String, String> args) {
Expand Down Expand Up @@ -96,10 +108,14 @@ private static String[] getCommaList(Map<String, String> m, String k) {
}

private static String getOrEmpty(Map<String, String> m, String k) {
if(m.containsKey(k)) {
return m.get(k);
return getOrElse(m, k, "");
}

private static String getOrElse(Map<String, String> attrs, String key, String defaultValue) {
if(attrs.containsKey(key)) {
return attrs.get(key);
} else {
return "";
return defaultValue;
}
}

Expand Down
45 changes: 40 additions & 5 deletions src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@
import java.nio.file.Paths;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
import java.util.List;
import java.util.Map;
import java.util.*;
import java.util.Map.Entry;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
Expand Down Expand Up @@ -187,7 +183,45 @@ private static boolean matchesFileExtensions(String fileName, String[] extension
return false;
}

private static String[] encodeBazelTargets(String[] targets) {
return Arrays.stream(targets)
.map(ScalacProcessor::encodeBazelTarget)
.toArray(String[]::new);
}

private static String encodeBazelTarget(String target) {
return target.replace(":", ";");
}

private static boolean isModeEnabled(String mode) {
return !"off".equals(mode);
}

private static String[] getPluginParamsFrom(CompileOptions ops) {
String[] pluginParams;

if (isModeEnabled(ops.dependencyAnalyzerMode)) {
String[] targets = encodeBazelTargets(ops.indirectTargets);
String currentTarget = encodeBazelTarget(ops.currentTarget);

String[] pluginParamsInUse = {
"-P:dependency-analyzer:direct-jars:" + String.join(":", ops.directJars),
"-P:dependency-analyzer:indirect-jars:" + String.join(":", ops.indirectJars),
"-P:dependency-analyzer:indirect-targets:" + String.join(":", targets),
"-P:dependency-analyzer:mode:" + ops.dependencyAnalyzerMode,
"-P:dependency-analyzer:current-target:" + currentTarget,
};
pluginParams = pluginParamsInUse;
} else {
pluginParams = new String[0];
}
return pluginParams;
}

private static void compileScalaSources(CompileOptions ops, String[] scalaSources, Path tmpPath) throws IllegalAccessException {

String[] pluginParams = getPluginParamsFrom(ops);

String[] constParams = {
"-classpath",
ops.classpath,
Expand All @@ -199,6 +233,7 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource
ops.scalaOpts,
ops.pluginArgs,
constParams,
pluginParams,
scalaSources);

MainClass comp = new MainClass();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package scala.test.strict_deps.no_recompilation;

object A {
def foo = {
B.foo
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package scala.test.strict_deps.no_recompilation;

object B {
def foo = {
C.foo
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package(default_visibility = ["//visibility:public"])
load("//scala:scala.bzl", "scala_library", "scala_test", "scala_binary")

scala_library(
name="transitive_dependency_user",
srcs=[
"A.scala",
],
deps = ["direct_dependency"],
dependency_analyzer_mode_soon_to_be_removed="error",
)

scala_library(
name="direct_dependency",
srcs=[
"B.scala",
],
deps = ["transitive_dependency"],
dependency_analyzer_mode_soon_to_be_removed="error",
)

scala_library(
name="transitive_dependency",
srcs=[
"C.scala",
],
dependency_analyzer_mode_soon_to_be_removed="error",
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package scala.test.strict_deps.no_recompilation;

object C {
def foo = {
println("orig")
}
}
8 changes: 8 additions & 0 deletions test_expect_failure/dep_analyzer_modes/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package test_expect_failure.dep_analyzer_modes

object A {
def foo = {
B.foo
C.foo
}
}
7 changes: 7 additions & 0 deletions test_expect_failure/dep_analyzer_modes/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package test_expect_failure.dep_analyzer_modes

object B {
def foo = {
C.foo
}
}
56 changes: 56 additions & 0 deletions test_expect_failure/dep_analyzer_modes/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package(default_visibility = ["//visibility:public"])
load("//scala:scala.bzl", "scala_library", "scala_binary", "scala_test")

scala_library(
name="error_mode",
dependency_analyzer_mode_soon_to_be_removed="error",
srcs=[
"A.scala",
],
deps = ["direct_dependency"],
)

scala_library(
name="warn_mode",
dependency_analyzer_mode_soon_to_be_removed="warn",
srcs=[
"A.scala",
],
deps = ["direct_dependency"],
)

scala_library(
name="weird_mode",
dependency_analyzer_mode_soon_to_be_removed="kuki_buki",
srcs=[
"A.scala",
],
deps = ["direct_dependency"],
)

scala_library(
name="off_mode",
dependency_analyzer_mode_soon_to_be_removed="off",
srcs=[
"A.scala",
],
deps = ["direct_dependency"],
)


scala_library(
name="direct_dependency",
dependency_analyzer_mode_soon_to_be_removed="error",
srcs=[
"B.scala",
],
deps = ["transitive_dependency"],
)

scala_library(
name="transitive_dependency",
dependency_analyzer_mode_soon_to_be_removed="error",
srcs=[
"C.scala",
],
)
7 changes: 7 additions & 0 deletions test_expect_failure/dep_analyzer_modes/C.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package test_expect_failure.dep_analyzer_modes

object C {
def foo = {
println("in C")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package test_expect_failure.missing_direct_deps.external_deps

object A {
def foo = {
B.foo
com.google.common.base.Strings.commonPrefix("abc", "abcd")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package test_expect_failure.missing_direct_deps.external_deps

object B {
def foo = {
println("in B")
com.google.common.base.Strings.commonPrefix("abc", "abcd")
}
}
20 changes: 20 additions & 0 deletions test_expect_failure/missing_direct_deps/external_deps/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package(default_visibility = ["//visibility:public"])
load("//scala:scala.bzl", "scala_library", "scala_test")

scala_library(
name="transitive_external_dependency_user",
srcs=[
"A.scala",
],
deps = ["external_dependency_user"],
dependency_analyzer_mode_soon_to_be_removed="error",
)

scala_library(
name="external_dependency_user",
srcs=[
"B.scala",
],
deps = ["@com_google_guava_guava_21_0//jar"],
dependency_analyzer_mode_soon_to_be_removed="error",
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package test_expect_failure.missing_direct_deps.external_deps_file_group

object A {
def foo = {
B.foo
com.google.common.base.Strings.commonPrefix("abc", "abcd")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package test_expect_failure.missing_direct_deps.external_deps_file_group

object B {
def foo = {
println("in B")
com.google.common.base.Strings.commonPrefix("abc", "abcd")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package(default_visibility = ["//visibility:public"])
load("//scala:scala.bzl", "scala_library", "scala_test")

scala_library(
name="transitive_external_dependency_user",
srcs=[
"A.scala",
],
deps = ["external_dependency_user"],
dependency_analyzer_mode_soon_to_be_removed="error",
)

scala_library(
name="external_dependency_user",
srcs=[
"B.scala",
],
deps = ["@com_google_guava_guava_21_0//jar:file"],
dependency_analyzer_mode_soon_to_be_removed="error",
)
10 changes: 10 additions & 0 deletions test_expect_failure/missing_direct_deps/internal_deps/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package test_expect_failure.missing_direct_deps.internal_deps;

object A {
def foo = {
B.foo
C.foo
}

def main = foo
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package test_expect_failure.missing_direct_deps.internal_deps;

object B {
def foo = {
C.foo
}
}
Loading