Add helper to create closure_js_library metadata (#298)

The helper will make it is easier for other rules to act like a
closure_js_library. Note that the returned struct from helper is
not a proper provider since existing contract of closure_js_library
is not compatible with it and requires a large incompatible
refactoring.

This patch also fixes handling of zip files and adds corresponding
tests.

closure/compiler/closure_js_binary.bzl

@@ -16,14 +16,12 @@
 
 load(
     "//closure/private:defs.bzl",
-    "CLOSURE_LIBRARY_BASE_ATTR",
-    "CLOSURE_WORKER_ATTR",
+    "CLOSURE_JS_TOOLCHAIN_ATTRS",
     "JS_LANGUAGES",
     "JS_LANGUAGE_IN",
     "JS_LANGUAGE_OUT_DEFAULT",
     "collect_js",
     "collect_runfiles",
-    "create_argfile",
     "difference",
     "find_js_module_roots",
     "get_jsfile_path",
@@ -210,7 +208,7 @@ def _impl(ctx):
     for src in js.srcs:
         inputs.append(src)
         if src.path.endswith(".zip"):
-            all_args.append("--jszip")
+            all_args.add("--jszip")
         all_args.add_all(
             [src],
             map_each = get_jsfile_path,
@@ -277,7 +275,7 @@ def _validate_css_graph(ctx, js):
 
 closure_js_binary = rule(
     implementation = _impl,
-    attrs = {
+    attrs = dict({
         "compilation_level": attr.string(default = "ADVANCED"),
         "css": attr.label(providers = ["closure_css_binary"]),
         "debug": attr.bool(default = False),
@@ -301,9 +299,7 @@ closure_js_binary = rule(
         # internal only
         "internal_expect_failure": attr.bool(default = False),
         "internal_expect_warnings": attr.bool(default = False),
-        "_ClosureWorker": CLOSURE_WORKER_ATTR,
-        "_closure_library_base": CLOSURE_LIBRARY_BASE_ATTR,
-    },
+    }, **CLOSURE_JS_TOOLCHAIN_ATTRS),
     outputs = {
         "bin": "%{name}.js",
         "map": "%{name}.js.map",

closure/compiler/closure_js_library.bzl

@@ -16,8 +16,7 @@
 
 load(
     "//closure/private:defs.bzl",
-    "CLOSURE_LIBRARY_BASE_ATTR",
-    "CLOSURE_WORKER_ATTR",
+    "CLOSURE_JS_TOOLCHAIN_ATTRS",
     "JS_FILE_TYPE",
     "JS_LANGUAGE_IN",
     "collect_js",
@@ -40,7 +39,59 @@ def _maybe_declare_file(actions, file, name):
         return file
     return actions.declare_file(name)
 
-def closure_js_library_impl(
+def create_closure_js_library(
+        ctx,
+        srcs = [],
+        deps = [],
+        exports = [],
+        suppress = [],
+        lenient = False):
+    """ Returns closure_js_library metadata with provided attributes.
+
+    Note that the returned struct is not a proper provider since existing contract
+    of closure_js_library is not compatible with it. The rule calling this method
+    could assign properties of the returned struct to its own result to fullfil
+    the contract expected by the bazel closure rules.
+
+    Note that rules using this helper should extend its attribute set with
+    CLOSURE_JS_TOOLCHAIN_ATTRS in order to make the closure toolchain available.
+
+    Args:
+      ctx: ctx for the rule
+      srcs: JavaScript srcs for the library
+      deps: deps of the library
+      exports: exports for the library
+      suppress: list of strings containing DiagnosticGroup (coarse grained) or
+        DiagnosticType (fine grained) codes. These apply not only to JsChecker,
+        but also propagate up to closure_js_binary.
+      lenient: makes the library lenient which suppresses handful of checkings in
+        one shot.
+
+    Returns:
+      A closure_js_library metadata struct with exports and closure_js_library attribute
+    """
+
+    if not hasattr(ctx.files, "_ClosureWorker") or not hasattr(ctx.files, "_closure_library_base"):
+        fail("Closure toolchain undefined; rule should include CLOSURE_JS_TOOLCHAIN_ATTRS")
+
+    # testonly exist for all rules but if it is an aspect it need to accessed over ctx.rule.
+    testonly = ctx.attr.testonly if hasattr(ctx.attr, "testonly") else ctx.rule.attr.testonly
+
+    return _closure_js_library_impl(
+        ctx.actions,
+        ctx.label,
+        ctx.workspace_name,
+        srcs = srcs,
+        deps = deps,
+        exports = exports,
+        suppress = suppress,
+        lenient = lenient,
+        testonly = testonly,
+        closure_library_base = ctx.files._closure_library_base,
+        closure_worker = ctx.executable._ClosureWorker,
+    )
+
+def _closure_js_library_impl(
         actions,
         label,
         workspace_name,
@@ -50,7 +101,7 @@ def closure_js_library_impl(
         suppress,
         lenient,
         closure_library_base,
-        _ClosureWorker,
+        closure_worker,
         includes = (),
         exports = depset(),
         internal_descriptors = depset(),
@@ -224,7 +275,7 @@ def closure_js_library_impl(
     actions.run(
         inputs = inputs,
         outputs = [info_file, stderr_file, ijs_file],
-        executable = _ClosureWorker,
+        executable = closure_worker,
         arguments = ["@@" + argfile.path],
         mnemonic = "Closure",
         execution_requirements = {"supports-workers": "1"},
@@ -236,7 +287,7 @@ def closure_js_library_impl(
         label = label,
         ijs_deps = js.ijs_files,
         srcs = srcs,
-        executable = _ClosureWorker,
+        executable = closure_worker,
         output = _maybe_declare_file(
             actions,
             deprecated_typecheck_file,
@@ -332,7 +383,7 @@ def _closure_js_library(ctx):
         print("closure_js_library 'externs' is deprecated; just use 'srcs'")
         srcs = ctx.files.externs + srcs
 
-    library = closure_js_library_impl(
+    library = _closure_js_library_impl(
         ctx.actions,
         ctx.label,
         ctx.workspace_name,
@@ -377,7 +428,7 @@ def _closure_js_library(ctx):
 
 closure_js_library = rule(
     implementation = _closure_js_library,
-    attrs = {
+    attrs = dict({
         "convention": attr.string(
             default = "CLOSURE",
             # TODO(yannic): Define valid values.
@@ -405,9 +456,7 @@ closure_js_library = rule(
         # internal only
         "internal_descriptors": attr.label_list(allow_files = True),
         "internal_expect_failure": attr.bool(default = False),
-        "_ClosureWorker": CLOSURE_WORKER_ATTR,
-        "_closure_library_base": CLOSURE_LIBRARY_BASE_ATTR,
-    },
+    }, **CLOSURE_JS_TOOLCHAIN_ATTRS),
     # TODO(yannic): Deprecate.
     #     https://docs.bazel.build/versions/master/skylark/lib/globals.html#rule.outputs
     outputs = {

closure/compiler/test/js_zip/BUILD

@@ -0,0 +1,37 @@
+# Copyright 2018 The Closure Rules 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.
+
+package(default_testonly = True)
+
+licenses(["notice"])  # Apache 2.0
+
+load("//closure/private:file_test.bzl", "file_test")
+load("//closure:defs.bzl", "closure_js_binary")
+load(":zip_file_test_library.bzl", "zip_file_test_library")
+
+zip_file_test_library(
+    name = "main_lib",
+    srcs = ["main.js.zip"],
+)
+
+closure_js_binary(
+    name = "main_bin",
+    deps = [":main_lib"],
+)
+
+file_test(
+    name = "zip_files_worksCorrectly",
+    content = "console.log(\"hi from zip\");\n",
+    file = "main_bin.js",
+)

closure/compiler/test/js_zip/zip_file_test_library.bzl

@@ -0,0 +1,11 @@
+load("//closure:defs.bzl", "CLOSURE_JS_TOOLCHAIN_ATTRS", "create_closure_js_library")
+
+def _impl_zip_file_test_library(ctx):
+    return create_closure_js_library(ctx, ctx.files.srcs)
+
+zip_file_test_library = rule(
+    implementation = _impl_zip_file_test_library,
+    attrs = dict(CLOSURE_JS_TOOLCHAIN_ATTRS, **{
+        "srcs": attr.label_list(allow_files = [".js.zip"]),
+    }),
+)

closure/defs.bzl

@@ -15,7 +15,8 @@
 load("//closure/compiler:closure_js_aspect.bzl", "closure_js_aspect")
 load("//closure/compiler:closure_js_binary.bzl", "closure_js_binary")
 load("//closure/compiler:closure_js_deps.bzl", "closure_js_deps")
-load("//closure/compiler:closure_js_library.bzl", "closure_js_library")
+load("//closure/compiler:closure_js_library.bzl", "closure_js_library", "create_closure_js_library")
+load("//closure/private:defs.bzl", "CLOSURE_JS_TOOLCHAIN_ATTRS")
 load("//closure/private:files_equal_test.bzl", "files_equal_test")
 load("//closure/private:java_import_external.bzl", "java_import_external")
 load("//closure/protobuf:closure_js_proto_library.bzl", "closure_js_proto_library")

closure/private/defs.bzl

@@ -42,6 +42,11 @@ CLOSURE_WORKER_ATTR = attr.label(
     cfg = "host",
 )
 
+CLOSURE_JS_TOOLCHAIN_ATTRS = {
+    "_closure_library_base": CLOSURE_LIBRARY_BASE_ATTR,
+    "_ClosureWorker": CLOSURE_WORKER_ATTR,
+}
+
 def get_jsfile_path(f):
     return f.path
 

closure/protobuf/closure_proto_library.bzl

@@ -15,13 +15,8 @@
 """Utilities for building JavaScript Protocol Buffers.
 """
 
-load("//closure/compiler:closure_js_library.bzl", "closure_js_library_impl")
-load(
-    "//closure/private:defs.bzl",
-    "CLOSURE_LIBRARY_BASE_ATTR",
-    "CLOSURE_WORKER_ATTR",
-    "unfurl",
-)
+load("//closure/compiler:closure_js_library.bzl", "create_closure_js_library")
+load("//closure/private:defs.bzl", "CLOSURE_JS_TOOLCHAIN_ATTRS", "unfurl")
 
 # This was borrowed from Rules Go, licensed under Apache 2.
 # https://github.com/bazelbuild/rules_go/blob/67f44035d84a352cffb9465159e199066ecb814c/proto/compiler.bzl#L72
@@ -101,18 +96,7 @@ def _closure_proto_aspect_impl(target, ctx):
         "unusedLocalVariables",
     ]
 
-    library = closure_js_library_impl(
-        ctx.actions,
-        ctx.label,
-        ctx.workspace_name,
-        srcs,
-        deps,
-        ctx.rule.attr.testonly,
-        suppress,
-        True,
-        ctx.files._closure_library_base,
-        ctx.executable._ClosureWorker,
-    )
+    library = create_closure_js_library(ctx, srcs, deps, [], suppress, True)
     return struct(
         exports = library.exports,
         closure_js_library = library.closure_js_library,
@@ -122,22 +106,20 @@ def _closure_proto_aspect_impl(target, ctx):
 
 closure_proto_aspect = aspect(
     attr_aspects = ["deps"],
-    attrs = {
+    attrs = dict({
         # internal only
         "_protoc": attr.label(
             default = Label("@com_google_protobuf//:protoc"),
             executable = True,
             cfg = "host",
         ),
-        "_ClosureWorker": CLOSURE_WORKER_ATTR,
-        "_closure_library_base": CLOSURE_LIBRARY_BASE_ATTR,
         "_closure_library": attr.label(
             default = Label("//closure/library/array"),
         ),
         "_closure_protobuf_jspb": attr.label(
             default = Label("//closure/protobuf:jspb"),
         ),
-    },
+    }, **CLOSURE_JS_TOOLCHAIN_ATTRS),
     implementation = _closure_proto_aspect_impl,
 )