Remove (deprecated) implicit outputs, part 1

The `outputs` parameter on the `rule()` function has been deprecated and will be removed in a future version of Bazel. See bazelbuild/bazel#7977 for context.
bazelbuild · Jun 27, 2020 · 6b00b4a · 6b00b4a
1 parent 62746bd
commit 6b00b4a
Show file tree

Hide file tree

Showing 33 changed files with 427 additions and 140 deletions.
diff --git a/closure/compiler/closure_js_binary.bzl b/closure/compiler/closure_js_binary.bzl
@@ -33,6 +33,23 @@ load(
     "closure_js_aspect",
 )
 
+JavaScriptBinaryInfo = provider(
+    doc = "Encapsulates information provided by `closure_js_binary`.",
+    fields = {
+        "output": """
+Primary output file.
+""".strip(),
+        "sourcemap": """
+The sourcemap file.
+""".strip(),
+        "property_renaming_report": """
+File containing a serialized version of the property renaming map.
+
+If `compilation_level` is not `ADVANCED`, this is an empty file.
+""".strip(),
+    },
+)
+
 _dependency_mode_warning = '\n'.join([
   "{target}: dependency_mode={old_mode} is deprecated and will be " +
       "removed soon; prefer to use its equivalent {new_mode}.",
@@ -77,11 +94,19 @@ def _impl(ctx):
 
     _validate_css_graph(ctx, js)
 
+    output = ctx.actions.declare_file("{}.js".format(ctx.attr.name))
+    sourcemap = ctx.actions.declare_file("{}.js.map".format(ctx.attr.name))
+
+    if ctx.outputs.property_renaming_report:
+        property_renaming_report = ctx.outputs.property_renaming_report
+    else:
+        property_renaming_report = ctx.actions.declare_file("{}.property_renaming_report.txt".format(ctx.attr.name))
+
     # This is the list of files we'll be generating.
-    outputs = [ctx.outputs.bin, ctx.outputs.map, ctx.outputs.stderr]
+    outputs = [output, sourcemap, ctx.outputs.stderr]
 
     # This is the subset of that list we'll report to parent rules.
-    files = [ctx.outputs.bin, ctx.outputs.map]
+    files = [output, sourcemap]
 
     # JsCompiler is thin veneer over the Closure compiler. It's configured with a
     # superset of its flags. It introduces a private testing API, allows per-file
@@ -91,9 +116,9 @@ def _impl(ctx):
     args = [
         "JsCompiler",
         "--js_output_file",
-        ctx.outputs.bin.path,
+        output.path,
         "--create_source_map",
-        ctx.outputs.map.path,
+        sourcemap.path,
         "--output_errors",
         ctx.outputs.stderr.path,
         "--language_in",
@@ -138,7 +163,7 @@ def _impl(ctx):
     js_module_roots = sort_roots(
         depset(transitive = [
             find_js_module_roots(
-                [ctx.outputs.bin],
+                [output],
                 ctx.workspace_name,
                 ctx.label,
                 getattr(ctx.attr, "includes", []),
@@ -155,7 +180,7 @@ def _impl(ctx):
     # stored within the same directory as the compiled JS binary; therefore, the
     # JSON sourcemap file should cite that file as relative to itself.
     args.append("--source_map_location_mapping")
-    args.append("%s|%s" % (ctx.outputs.bin.path, ctx.outputs.bin.basename))
+    args.append("%s|%s" % (output.path, output.basename))
 
     # By default we're going to include the raw sources in the .js.map file. This
     # can be disabled with the nodefs attribute.
@@ -205,12 +230,16 @@ def _impl(ctx):
         if ctx.attr.output_wrapper == "(function(){%output%}).call(this);":
             args.append("--assume_function_wrapper")
 
-    if ctx.outputs.property_renaming_report:
-        report = ctx.outputs.property_renaming_report
-        files.append(report)
-        outputs.append(report)
+    if  ("ADVANCED" == ctx.attr.compilation_level) and (not ctx.attr.internal_expect_failure):
+        files.append(property_renaming_report)
+        outputs.append(property_renaming_report)
         args.append("--property_renaming_report")
-        args.append(report.path)
+        args.append(property_renaming_report.path)
+    else:
+        ctx.actions.write(
+            output = property_renaming_report,
+            content = "",
+        )
 
     # All sources must conform to these protos.
     for config in ctx.files.conformance:
@@ -268,7 +297,7 @@ def _impl(ctx):
         execution_requirements = {"supports-workers": "1"},
         progress_message = "Compiling %d JavaScript files to %s" % (
             len(js.srcs.to_list()),
-            ctx.outputs.bin.short_path,
+            output.short_path,
         ),
     )
 
@@ -280,8 +309,8 @@ def _impl(ctx):
         files = depset(files),
         closure_js_library = js,
         closure_js_binary = struct(
-            bin = ctx.outputs.bin,
-            map = ctx.outputs.map,
+            bin = output,
+            map = sourcemap,
             language = ctx.attr.language,
         ),
         runfiles = ctx.runfiles(
@@ -292,6 +321,17 @@ def _impl(ctx):
                 collect_runfiles(ctx.attr.data),
             ]),
         ),
+
+        # Modern-stype providers.
+        #
+        # TODO(yannic): Remove legacy providers.
+        providers = [
+            JavaScriptBinaryInfo(
+                output = output,
+                sourcemap = sourcemap,
+                property_renaming_report = property_renaming_report,
+            ),
+        ],
     )
 
 def _validate_css_graph(ctx, js):
@@ -345,8 +385,6 @@ closure_js_binary = rule(
         "internal_expect_warnings": attr.bool(default = False),
     }, **CLOSURE_JS_TOOLCHAIN_ATTRS),
     outputs = {
-        "bin": "%{name}.js",
-        "map": "%{name}.js.map",
         "stderr": "%{name}-stderr.txt",
     },
 )
diff --git a/closure/compiler/closure_js_library.bzl b/closure/compiler/closure_js_library.bzl
@@ -104,7 +104,6 @@ def _closure_js_library_impl(
         # and will be replaced with |actions.declare_file()| soon.
         deprecated_info_file = None,
         deprecated_stderr_file = None,
-        deprecated_ijs_file = None,
         deprecated_typecheck_file = None):
     # TODO(yannic): Figure out how to modify |find_js_module_roots|
     # so that we won't need |workspace_name| anymore.
@@ -150,11 +149,8 @@ def _closure_js_library_impl(
         deprecated_stderr_file,
         "%s-stderr.txt" % label.name,
     )
-    ijs_file = _maybe_declare_file(
-        actions,
-        deprecated_ijs_file,
-        "%s.i.js" % label.name,
-    )
+
+    ijs_file = ctx.actions.declare_file("{}.i.js".format(label.name))
 
     if not no_closure_library:
         deps = deps + closure_library_base
@@ -403,24 +399,23 @@ def _closure_js_library(ctx):
         srcs = ctx.files.externs + srcs
 
     library = _closure_js_library_impl(
-        ctx,
-        srcs,
-        ctx.attr.deps,
-        ctx.attr.testonly,
-        ctx.attr.suppress,
-        ctx.attr.lenient,
-        ctx.attr.convention,
-        getattr(ctx.attr, "includes", []),
-        ctx.attr.exports,
-        ctx.files.internal_descriptors,
-        ctx.attr.no_closure_library,
-        ctx.attr.internal_expect_failure,
+        ctx = ctx,
+        srcs = srcs,
+        deps = ctx.attr.deps,
+        testonly = ctx.attr.testonly,
+        suppress = ctx.attr.suppress,
+        lenient = ctx.attr.lenient,
+        convention = ctx.attr.convention,
+        includes = getattr(ctx.attr, "includes", []),
+        exports = ctx.attr.exports,
+        internal_descriptors = ctx.files.internal_descriptors,
+        no_closure_library = ctx.attr.no_closure_library,
+        internal_expect_failure = ctx.attr.internal_expect_failure,
 
         # Deprecated output files.
-        ctx.outputs.info,
-        ctx.outputs.stderr,
-        ctx.outputs.ijs,
-        ctx.outputs.typecheck,
+        deprecated_info_file = ctx.outputs.info,
+        deprecated_stderr_file = ctx.outputs.stderr,
+        deprecated_typecheck_file = ctx.outputs.typecheck,
     )
 
     return struct(
@@ -476,7 +471,6 @@ closure_js_library = rule(
     outputs = {
         "info": "%{name}.pbtxt",
         "stderr": "%{name}-stderr.txt",
-        "ijs": "%{name}.i.js",
         "typecheck": "%{name}_typecheck",  # dummy output file
     },
 )
diff --git a/closure/compiler/test/BUILD b/closure/compiler/test/BUILD
@@ -16,6 +16,7 @@ package(default_testonly = True)
 
 licenses(["notice"])  # Apache 2.0
 
+load("//closure/private:expected_output_test.bzl", "closure_js_binary_output_test", "closure_js_binary_sourcemap_test")
 load("//closure/private:file_test.bzl", "file_test")
 load("//closure:defs.bzl", "closure_js_binary", "closure_js_library")
 
@@ -30,24 +31,16 @@ closure_js_binary(
     deps = [":hello_lib"],
 )
 
-file_test(
+closure_js_binary_output_test(
     name = "minification",
-    content = "console.log(\"hello world\");\n",
-    file = "hello_bin.js",
+    expected_output = "golden/minification.js",
+    target = ":hello_bin",
 )
 
-file_test(
+closure_js_binary_sourcemap_test(
     name = "sourcemap_doesntContainWeirdBazelDirectories",
-    content = ("{\n" +
-               "\"version\":3,\n" +
-               "\"file\":\"hello_bin.js\",\n" +
-               "\"lineCount\":1,\n" +
-               "\"mappings\":\"AAgBAA,OAAA,CAAUC,GAAV,CAAgB,aAAhB;\",\n" +
-               "\"sources\":[\"/closure/compiler/test/hello.js\"],\n" +
-               "\"sourcesContent\":[\"// Copyright 2016 The Closure Rules Authors. All rights reserved.\\n//\\n// Licensed under the Apache License, Version 2.0 (the \\\"License\\\");\\n// you may not use this file except in compliance with the License.\\n// You may obtain a copy of the License at\\n//\\n//    http://www.apache.org/licenses/LICENSE-2.0\\n//\\n// Unless required by applicable law or agreed to in writing, software\\n// distributed under the License is distributed on an \\\"AS IS\\\" BASIS,\\n// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\\n// See the License for the specific language governing permissions and\\n// limitations under the License.\\n\\n// hello world\\n\\nconsole . log ( 'hello world' ) ;\\n\"],\n" +
-               "\"names\":[\"console\",\"log\"]\n" +
-               "}\n"),
-    file = "hello_bin.js.map",
+    expected_output = "golden/sourcemap_doesn_contain_weird_bazel_directories.js.map",
+    target = ":hello_bin",
 )
 
 # Make sure bazel doesn't complain about some outputs not created.
@@ -64,24 +57,16 @@ closure_js_binary(
     deps = [":hello_lib"],
 )
 
-file_test(
-    name = "minificationWithWrapper",
-    content = "(function(){console.log(\"hello world\");}).call(this);\n",
-    file = "hello_wrap_bin.js",
+closure_js_binary_output_test(
+    name = "minification_with_wrapper",
+    expected_output = "golden/minification_with_wrapper.js",
+    target = ":hello_wrap_bin",
 )
 
-file_test(
+closure_js_binary_sourcemap_test(
     name = "sourcemapWithWrapper_hasDifferentMappingCodes",
-    content = "{\n" +
-              "\"version\":3,\n" +
-              "\"file\":\"hello_wrap_bin.js\",\n" +
-              "\"lineCount\":1,\n" +
-              "\"mappings\":\"A,YAgBAA,OAAA,CAAUC,GAAV,CAAgB,aAAhB;\",\n" +
-              "\"sources\":[\"/closure/compiler/test/hello.js\"],\n" +
-              "\"sourcesContent\":[\"// Copyright 2016 The Closure Rules Authors. All rights reserved.\\n//\\n// Licensed under the Apache License, Version 2.0 (the \\\"License\\\");\\n// you may not use this file except in compliance with the License.\\n// You may obtain a copy of the License at\\n//\\n//    http://www.apache.org/licenses/LICENSE-2.0\\n//\\n// Unless required by applicable law or agreed to in writing, software\\n// distributed under the License is distributed on an \\\"AS IS\\\" BASIS,\\n// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\\n// See the License for the specific language governing permissions and\\n// limitations under the License.\\n\\n// hello world\\n\\nconsole . log ( 'hello world' ) ;\\n\"],\n" +
-              "\"names\":[\"console\",\"log\"]\n" +
-              "}\n",
-    file = "hello_wrap_bin.js.map",
+    expected_output = "golden/sourcemap_with_wrapper_has_different_mapping_codes.js.map",
+    target = ":hello_wrap_bin",
 )
 
 closure_js_binary(
@@ -90,10 +75,10 @@ closure_js_binary(
     deps = [":hello_lib"],
 )
 
-file_test(
+closure_js_binary_output_test(
     name = "strictOutputLanguage_addsUseStrict",
-    content = "'use strict';console.log(\"hello world\");\n",
-    file = "hello_es5strict_bin.js",
+    expected_output = "golden/strict_output_language_adds_use_strict.js",
+    target = ":hello_es5strict_bin",
 )
 
 closure_js_library(
@@ -109,10 +94,10 @@ closure_js_binary(
     deps = [":es6const_lib"],
 )
 
-file_test(
+closure_js_binary_output_test(
     name = "es6WithConstKeyword_getsRemoved",
-    content = "var hello=\"hello world\";console.log(hello);\n",
-    file = "es6const_bin.js",
+    expected_output = "golden/es6_with_const_keyword_gets_removed.js",
+    target = ":es6const_bin",
 )
 
 closure_js_library(
@@ -128,10 +113,10 @@ closure_js_binary(
     deps = [":es6arrow_lib"],
 )
 
-file_test(
+closure_js_binary_output_test(
     name = "es6WithArrowFunction_getsExpanded",
-    content = "var hello=function(e){return e+\" world\"};console.log(hello(\"hello\"));\n",
-    file = "es6arrow_bin.js",
+    expected_output = "golden/es6_with_arrow_function_gets_expanded.js",
+    target = ":es6arrow_bin",
 )
 
 closure_js_library(
@@ -165,10 +150,10 @@ closure_js_binary(
     deps = [":hello_lib"],
 )
 
-file_test(
+closure_js_binary_output_test(
     name = "output_wrapper_dash_dash_space",
-    content = "-- console.log(\"hello world\");\n",
-    file = "hello_output_wrapper_dash_dash_space_bin.js",
+    expected_output = "golden/output_wrapper_dash_dash_space.js",
+    target = ":hello_output_wrapper_dash_dash_space_bin",
 )
 
 closure_js_binary(
@@ -177,10 +162,10 @@ closure_js_binary(
     deps = [":empty_lib"],
 )
 
-file_test(
+closure_js_binary_output_test(
     name = "multiline_output_wrapper_test",
-    content = "\n//# sourceMappingURL=/app.js.map\n",
-    file = "multiline_output_wrapper_bin.js",
+    expected_output = "golden/multiline_output_wrapper_test.js",
+    target = ":multiline_output_wrapper_bin",
 )
 
 ################################################################################
@@ -202,10 +187,10 @@ closure_js_binary(
     deps = [":extern_invoke_lib"],
 )
 
-file_test(
+closure_js_binary_output_test(
     name = "externFile_namesDidntCollapse",
-    content = "omg.im_an_extern(\"hello\");\n",
-    file = "extern_bin.js",
+    expected_output = "golden/extern_file_names_didnt_collapse.js",
+    target = ":extern_bin",
 )
 
 closure_js_library(

diff --git a/closure/compiler/test/closure_js_deps/BUILD b/closure/compiler/test/closure_js_deps/BUILD
@@ -157,5 +157,5 @@ genrule(
 file_test(
     name = "srcsReferencingJsBinary_getsJsBinaryAndMapNotData",
     file = "hyperion2_bin_srcs.txt",
-    regexp = "^.*/closure/compiler/test/closure_js_deps/hyperion2_bin.js .*/closure/compiler/test/closure_js_deps/hyperion2_bin.js.map$",
+    regexp = "^.*/closure/compiler/test/closure_js_deps/hyperion2_bin.js .*/closure/compiler/test/closure_js_deps/hyperion2_bin.js.map .*/closure/compiler/test/closure_js_deps/hyperion2_bin.property_renaming_report.txt$",
 )