Aspect-based scrooge implementation

scala/private/common.bzl

@@ -1,10 +1,14 @@
 def write_manifest(ctx):
+  main_class = getattr(ctx.attr, "main_class", None)
+  write_manifest_file(ctx.actions, ctx.outputs.manifest, main_class)
+
+def write_manifest_file(actions, output_file, main_class):
   # TODO(bazel-team): I don't think this classpath is what you want
   manifest = "Class-Path: \n"
-  if getattr(ctx.attr, "main_class", ""):
-    manifest += "Main-Class: %s\n" % ctx.attr.main_class
+  if main_class:
+    manifest += "Main-Class: %s\n" % main_class
 
-  ctx.actions.write(output = ctx.outputs.manifest, content = manifest)
+  actions.write(output = output_file, content = manifest)
 
 def collect_srcjars(targets):
   srcjars = []

scala/private/rule_impls.bzl

@@ -545,28 +545,9 @@ def _lib(ctx, non_macro_lib):
       scala = scalaattr,
       providers = [java_provider],
       runfiles = runfiles,
-      # This is a free monoid given to the graph for the purpose of
-      # extensibility. This is necessary when one wants to create
-      # new targets which want to leverage a scala_library. For example,
-      # new_target1 -> scala_library -> new_target2. There might be
-      # information that new_target2 needs to get from new_target1,
-      # but we do not want to have to change scala_library to pass
-      # this information through. extra_information allows passing
-      # this information through, and it is up to the new_targets
-      # to filter and make sense of this information.
-      # unfortunately, we need to see this for scrooge and protobuf to work,
-      # but those are generating srcjar, so they should really come in via srcs
-      extra_information=_collect_extra_information(ctx.attr.deps + ctx.attr.srcs),
       jars_to_labels = jars.jars2labels,
     )
 
-def _collect_extra_information(targets):
-  r = []
-  for target in targets:
-    if hasattr(target, "extra_information"):
-      r.extend(target.extra_information)
-  return r
-
 def scala_library_impl(ctx):
   return _lib(ctx, True)
 

src/scala/scripts/TwitterScroogeGenerator.scala

@@ -97,7 +97,7 @@ class ScroogeGenerator extends Processor {
     val intersect = allFilesInZips(onlyTransitiveThriftSrcJars)
       .intersect(immediateThriftSrcs)
 
-    if (intersect.nonEmpty)
+    if (intersect.iterator.filter(_.endsWith(".thrift")).nonEmpty)
       sys.error("onlyTransitiveThriftSrcs and immediateThriftSrcs should " +
         s"have not intersection, found: ${intersect.mkString(",")}")
 

test/src/main/scala/scalarules/test/twitter_scrooge/BUILD

@@ -1,35 +1,47 @@
 load("//scala:scala.bzl", "scala_binary", "scala_library")
 load("//twitter_scrooge:twitter_scrooge.bzl", "scrooge_scala_library")
+load("//thrift:thrift.bzl", "thrift_library")
 
 scrooge_scala_library(
     name = "scrooge1",
     visibility = ["//visibility:public"],
-    deps = [
+    exports = [
         ":scrooge2_a",
         ":scrooge2_b",
         ":scrooge3",
-        "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift",
     ],
+    deps = ["//test/src/main/scala/scalarules/test/twitter_scrooge/thrift"],
 )
 
 scrooge_scala_library(
     name = "scrooge2_a",
     visibility = ["//visibility:public"],
-    deps = [
+    exports = [
         ":scrooge3",
+    ],
+    deps = [
         "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2:thrift2_a",
     ],
 )
 
 scrooge_scala_library(
     name = "scrooge2_b",
     visibility = ["//visibility:public"],
-    deps = [
+    exports = [
         ":scrooge3",
+    ],
+    deps = [
         "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2:thrift2_b",
     ],
 )
 
+scrooge_scala_library(
+    name = "scrooge2_b_imp",
+    deps = [
+        "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2:thrift2_b_imp",
+    ],
+)
+
 scrooge_scala_library(
     name = "scrooge3",
     visibility = ["//visibility:public"],
@@ -39,8 +51,10 @@ scrooge_scala_library(
 scrooge_scala_library(
     name = "scrooge2",
     visibility = ["//visibility:public"],
-    deps = [
+    exports = [
         ":scrooge3",
+    ],
+    deps = [
         "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2:thrift2_a",
         "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2:thrift2_b",
     ],
@@ -49,17 +63,21 @@ scrooge_scala_library(
 scrooge_scala_library(
     name = "scrooge4",
     visibility = ["//visibility:public"],
-    deps = [
+    exports = [
         ":scrooge2",
+    ],
+    deps = [
         "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2/thrift4",
     ],
 )
 
 scrooge_scala_library(
     name = "scrooge4a",
     visibility = ["//visibility:public"],
-    deps = [
+    exports = [
         ":scrooge4",
+    ],
+    deps = [
         "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2/thrift4:thrift4a",
     ],
 )
@@ -83,21 +101,12 @@ scrooge_scala_library(
 scrooge_scala_library(
     name = "bare_thrift_scrooge",
     visibility = ["//visibility:public"],
-    deps = [
+    exports = [
         ":scroogebarejar1",
         ":scroogebarejar2",
-        "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/bare_jar_thrifts",
     ],
-)
-
-scrooge_scala_library(
-    name = "thrift_with_remote_jar",
-    remote_jars = [
-        "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/bare_jar_thrifts:barejar_java_import",
-    ],
-    visibility = ["//visibility:public"],
     deps = [
-        "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/bare_jar_thrifts/bare_jar_2",
+        "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/bare_jar_thrifts",
     ],
 )
 
@@ -127,6 +136,14 @@ scala_library(
     deps = [":scrooge3"],
 )
 
+scala_library(
+    name = "justscrooge3_import",
+    srcs = ["JustScrooge3.scala"],
+    deps = [
+        "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2/thrift3:thrift3_import",
+    ],
+)
+
 scala_library(
     name = "scrooge2_both",
     srcs = ["Scrooge2.scala"],

test/src/main/scala/scalarules/test/twitter_scrooge/prefix_test/e/f/b/c/d/BUILD

@@ -15,8 +15,10 @@ thrift_library(
 scrooge_scala_library(
     name = "d",
     visibility = ["//visibility:public"],
+    exports = [
+        "//test/src/main/scala/scalarules/test/twitter_scrooge/prefix_test/a/b/c/d",
+    ],
     deps = [
         ":b_thrift",
-        "//test/src/main/scala/scalarules/test/twitter_scrooge/prefix_test/a/b/c/d",
     ],
 )

test/src/main/scala/scalarules/test/twitter_scrooge/thrift/bare_jar_thrifts/BUILD

@@ -1,20 +1,16 @@
 load("//thrift:thrift.bzl", "thrift_library")
 
-filegroup(
-    name = "barejar",
-    srcs = ["bare-thrift.jar"],
-    visibility = ["//visibility:public"],
-)
-
 java_import(
-    name = "barejar_java_import",
+    name = "barejar",
     jars = ["bare-thrift.jar"],
     visibility = ["//visibility:public"],
 )
 
 thrift_library(
     name = "bare_jar_thrifts",
-    external_jars = [":barejar"],
+    external_jars = [
+        ":barejar",
+    ],
     visibility = ["//visibility:public"],
     deps = [
         "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/bare_jar_thrifts/bare_jar_1",

test/src/main/scala/scalarules/test/twitter_scrooge/thrift/bare_jar_thrifts/bare_jar_1/BUILD

@@ -1,13 +1,9 @@
 load("//thrift:thrift.bzl", "thrift_library")
 
-filegroup(
-    name = "barejar1",
-    srcs = ["bare-thrift-1.jar"],
-    visibility = ["//visibility:public"],
-)
-
 thrift_library(
     name = "bare_jar_1",
-    external_jars = [":barejar1"],
+    external_jars = [
+        "bare-thrift-1.jar",
+    ],
     visibility = ["//visibility:public"],
 )

test/src/main/scala/scalarules/test/twitter_scrooge/thrift/bare_jar_thrifts/bare_jar_2/BUILD

@@ -1,13 +1,9 @@
 load("//thrift:thrift.bzl", "thrift_library")
 
-filegroup(
-    name = "barejar2",
-    srcs = ["bare-thrift-2.jar"],
-    visibility = ["//visibility:public"],
-)
-
 thrift_library(
     name = "bare_jar_2",
-    external_jars = [":barejar2"],
+    external_jars = [
+        "bare-thrift-2.jar",
+    ],
     visibility = ["//visibility:public"],
 )

test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2/BUILD

@@ -13,3 +13,10 @@ thrift_library(
     visibility = ["//visibility:public"],
     deps = ["//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2/thrift3"],
 )
+
+thrift_library(
+    name = "thrift2_b_imp",
+    srcs = ["Thrift2_B.thrift"],
+    visibility = ["//visibility:public"],
+    deps = ["//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2/thrift3:thrift3_import"],
+)

test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2/thrift3/BUILD

@@ -1,7 +1,15 @@
 load("//thrift:thrift.bzl", "thrift_library")
+load("//twitter_scrooge:twitter_scrooge.bzl", "scrooge_scala_import")
 
 thrift_library(
     name = "thrift3",
     srcs = ["Thrift3.thrift"],
     visibility = ["//visibility:public"],
 )
+
+scrooge_scala_import(
+    name = "thrift3_import",
+    scala_jars = ["thrift3_scrooge.jar"],
+    thrift_jars = ["libthrift3.jar"],
+    visibility = ["//visibility:public"],
+)

test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2/thrift4/BUILD

@@ -4,11 +4,17 @@ thrift_library(
     name = "thrift4",
     srcs = ["Thrift4.thrift"],
     visibility = ["//visibility:public"],
+    deps = [
+        "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2:thrift2_a",
+    ],
 )
 
 thrift_library(
     name = "thrift4a",
     srcs = ["Thrift4a.thrift"],
     visibility = ["//visibility:public"],
-    deps = [":thrift4"],
+    deps = [
+        ":thrift4",
+        "//test/src/main/scala/scalarules/test/twitter_scrooge/thrift/thrift2:thrift2_a",
+    ],
 )