Update to buildifier 0.22.0 and fix new lint warnings

Don’t check no-effect warning since doc attribute causes a build error in Ubuntu 14.04 only. Need to update to stardoc. See bazelbuild/buildtools#471 for more details.

e2e/ts_devserver/genrule/BUILD.bazel

@@ -54,8 +54,6 @@ ts_devserver(
     deps = [":app"],
 )
 
-""
-
 genrule(
     name = "say-hello",
     outs = ["say-hello.js"],

internal/common/node_module_info.bzl

@@ -38,6 +38,8 @@ NodeModuleInfo = provider(
 # NodeModuleSources provider is provided by targets that are npm dependencies by the
 # `node_module_library` rule as well as other targets that have direct or transitive deps on
 # `node_module_library` targets via the `collect_node_modules_aspect` below.
+# TODO: rename to NodeModuleSourcesInfo so name doesn't trigger name-conventions warning
+# buildozer: disable=name-conventions
 NodeModuleSources = provider(
     doc = "Provides sources for npm dependencies installed with yarn_install and npm_install rules",
     fields = {

internal/node/node.bzl

@@ -378,6 +378,16 @@ Now you can add `--config=debug` to any `bazel test` command line.
 The runtime will pause before executing the program, allowing you to connect a
 remote debugger.
 """
+# Adding the above nodejs_test & nodejs_binary docstrings as `doc` attributes
+# causes a build error but ONLY on Ubuntu 14.04 on BazelCI.
+# ```
+# File "internal/node/node.bzl", line 378, in <module>
+#     outputs = _NODEJS_EXECUTABLE_OUTPUTS,
+# TypeError: rule() got an unexpected keyword argument 'doc'
+# ```
+# This error does not occur on any other platform on BazelCI including Ubuntu 16.04.
+# TOOD(gregmagolan): Figure out why and/or file a bug to Bazel
+# See https://github.com/bazelbuild/buildtools/issues/471#issuecomment-485283200
 
 def nodejs_binary_macro(name, data = [], args = [], visibility = None, tags = [], testonly = 0, **kwargs):
     """This macro exists only to wrap the nodejs_binary as an .exe for Windows.

internal/node/node_repositories.bzl

@@ -513,23 +513,14 @@ def node_repositories(
                     when you manually run the package manager, e.g. with
                     `bazel run @nodejs//:yarn` or `bazel run @nodejs//:npm install`.
                     If you use bazel-managed dependencies, you can omit this attribute.
-
       node_version: optional; the specific version of NodeJS to install.
-
       yarn_version: optional; the specific version of Yarn to install.
-
       vendored_node: optional; the local path to a pre-installed NodeJS runtime.
-
       vendored_yarn: optional; the local path to a pre-installed yarn tool.
-
       node_repositories: optional; custom list of node repositories to use.
-
       yarn_repositories: optional; custom list of yarn repositories to use.
-
       node_urls: optional; custom list of URLs to use to download NodeJS.
-
       yarn_urls: optional; custom list of URLs to use to download Yarn.
-
       preserve_symlinks: Turn on --node_options=--preserve-symlinks for nodejs_binary and nodejs_test rules.
         The default for this is currently True but the options is deprecated and will be removed in the future.
         When this option is turned on, node will preserve the symlinked path for resolves instead of the default

internal/npm_install/node_module_library.bzl

@@ -12,6 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""Contains the node_module_library which is used by yarn_install & npm_install.
+"""
+
 load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo", "NodeModuleSources")
 
 def _node_module_library_impl(ctx):

internal/npm_install/npm_install.bzl

@@ -226,6 +226,9 @@ npm_install = repository_rule(
     implementation = _npm_install_impl,
 )
 """Runs npm install during workspace setup."""
+# Adding the above docstring as `doc` attribute causes a build
+# error since `doc` is not a valid attribute of repository_rule.
+# See https://github.com/bazelbuild/buildtools/issues/471#issuecomment-485278689.
 
 def _yarn_install_impl(repository_ctx):
     """Core implementation of yarn_install."""
@@ -321,3 +324,6 @@ yarn_install = repository_rule(
     implementation = _yarn_install_impl,
 )
 """Runs yarn install during workspace setup."""
+# Adding the above docstring as `doc` attribute causes a build
+# error since `doc` is not a valid attribute of repository_rule.
+# See https://github.com/bazelbuild/buildtools/issues/471#issuecomment-485278689.

internal/npm_package/npm_package.bzl

@@ -220,3 +220,13 @@ $ bazel run :my_package.publish
 
 You can pass arguments to npm by escaping them from Bazel using a double-hyphen `bazel run my_package.publish -- --tag=next`
 """
+# Adding the above docstring as `doc` attribute
+# causes a build error but ONLY on Ubuntu 14.04 on BazelCI.
+# ```
+# File "internal/npm_package/npm_package.bzl", line 221, in <module>
+#     outputs = NPM_PACKAGE_OUTPUTS,
+# TypeError: rule() got an unexpected keyword argument 'doc'
+# ```
+# This error does not occur on any other platform on BazelCI including Ubuntu 16.04.
+# TOOD(gregmagolan): Figure out why and/or file a bug to Bazel
+# See https://github.com/bazelbuild/buildtools/issues/471#issuecomment-485283200

internal/rollup/rollup_bundle.bzl

@@ -748,3 +748,13 @@ For debugging, note that the `rollup.config.js` and `terser.config.json` files c
 
 An example usage can be found in https://github.com/bazelbuild/rules_nodejs/tree/master/internal/e2e/rollup
 """
+# Adding the above docstring as `doc` attribute
+# causes a build error but ONLY on Ubuntu 14.04 on BazelCI.
+# ```
+# File "internal/npm_package/npm_package.bzl", line 221, in <module>
+#     outputs = NPM_PACKAGE_OUTPUTS,
+# TypeError: rule() got an unexpected keyword argument 'doc'
+# ```
+# This error does not occur on any other platform on BazelCI including Ubuntu 16.04.
+# TOOD(gregmagolan): Figure out why and/or file a bug to Bazel
+# See https://github.com/bazelbuild/buildtools/issues/471#issuecomment-485283200

internal/web_package/web_package.bzl

@@ -1,8 +1,38 @@
-def html_asset_inject(index_html, action_factory, injector, rootDirs, assets, output):
+# Copyright 2019 The Bazel 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.
+
+"""Contains the web_package rule.
+"""
+
+def html_asset_inject(index_html, action_factory, injector, root_dirs, assets, output):
+    """Injects JS and CSS resources into the index.html.
+
+    Args:
+      index_html: The input html file
+      action_factory: Bazel's actions module from ctx.actions - see https://docs.bazel.build/versions/master/skylark/lib/actions.html
+      injector: The injector executable
+      root_dirs: Path prefixes to strip off all assets. Longest wins.
+      assets: Asset files to inject
+      output: The output html file
+
+    Returns:
+      The output html file
+    """
     args = action_factory.args()
     args.add(output.path)
     args.add(index_html.path)
-    args.add_all(rootDirs)
+    args.add_all(root_dirs)
     args.add("--assets")
     args.add_all(assets)
     args.use_param_file("%s", use_always = True)
@@ -15,6 +45,18 @@ def html_asset_inject(index_html, action_factory, injector, rootDirs, assets, ou
     return output
 
 def move_files(output_name, files, action_factory, assembler, root_paths):
+    """Moves files into an output directory
+
+    Args:
+      output_name: The name of the output directory
+      files: The files to move
+      action_factory: Bazel's actions module from ctx.actions - see https://docs.bazel.build/versions/master/skylark/lib/actions.html
+      assembler: The assembler executable
+      root_paths: Path prefixes to strip off all assets. Longest wins.
+
+    Returns:
+      The output directory tree-artifact
+    """
     www_dir = action_factory.declare_directory(output_name)
     args = action_factory.args()
     args.add(www_dir.path)
@@ -91,9 +133,8 @@ web_package = rule(
             cfg = "host",
         ),
     },
-    doc = """
-Assembles a web application from source files.
+    doc = """Assembles a web application from source files.
 
-Injects JS and CSS resources into the index.html.
-""",
+    Injects JS and CSS resources into the index.html.
+    """,
 )

package.json

@@ -10,7 +10,7 @@
     "license": "Apache-2.0",
     "devDependencies": {
         "@bazel/bazel": "0.24.1",
-        "@bazel/buildifier": "^0.20.0",
+        "@bazel/buildifier": "0.22.0",
         "@bazel/ibazel": "0.10.1",
         "clang-format": "1.2.2",
         "hello": "file:./tools/npm_packages/hello",
@@ -48,7 +48,15 @@
         "test_legacy_e2e": "./scripts/test_legacy_e2e.sh",
         "test_packages_all": "./scripts/test_packages_all.sh",
         "test_packages": "./scripts/test_packages.sh",
-        "bazel:format": "find . -type f \\( -name \"*.bzl\" -or -name WORKSPACE -or -name BUILD -or -name BUILD.bazel \\) ! -path \"*/node_modules/*\" ! -path \"./dist/*\" ! -path \"./third_party/*\" | xargs buildifier -v --warnings=args-order,attr-cfg,attr-license,attr-non-empty,attr-output-default,attr-single-file,constant-glob,ctx-actions,ctx-args,depset-iteration,depset-union,dict-concatenation,duplicated-name,filetype,git-repository,http-archive,integer-division,load,load-on-top,native-build,native-package,out-of-order-load,output-group,package-name,package-on-top,positional-args,redefined-variable,repository-name,same-origin-load,string-iteration,unsorted-dict-items,unused-variable",
+        "// Unchecked warnings": "The following warnings are not checked as disabling them locally is broken",
+        "//   rule-impl-return": "./internal/common/typescript_mock_lib.bzl:29: rule-impl-return: Avoid using the legacy provider syntax. (https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#rule-impl-return)",
+        "//   rule-impl-return(2)": "./packages/typescript/internal/protobufjs/ts_proto_library.bzl:105: rule-impl-return: Avoid using the legacy provider syntax. (https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#rule-impl-return)",
+        "//   uninitialized": "./internal/node/node.bzl:95: uninitialized: Variable 'd' may not have been initialized. (https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#uninitialized)",
+        "//   uninitialized(2)": "./internal/node/node.bzl:95: uninitialized: Variable 'd' may not have been initialized. (https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#uninitialized)",
+        "//   uninitialized(3)": "./packages/typescript/internal/build_defs.bzl:95: uninitialized: Variable 'output' may not have been initialized. (https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#uninitialized)",
+        "//   no-effect": "./internal/npm_install/npm_install.bzl:228: no-effect: Expression result is not used. (https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#no-effect)",
+        "//   no-effect(2)": "./internal/npm_install/npm_install.bzl:325: no-effect: Expression result is not used. (https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#no-effect)",
+        "bazel:format": "find . -type f \\( -name \"*.bzl\" -or -name WORKSPACE -or -name BUILD -or -name BUILD.bazel \\) ! -path \"*/node_modules/*\" ! -path \"./dist/*\" ! -path \"./third_party/*\" | xargs buildifier -v --warnings=attr-cfg,attr-license,attr-non-empty,attr-output-default,attr-single-file,confusing-name,constant-glob,ctx-actions,ctx-args,depset-iteration,depset-union,dict-concatenation,duplicated-name,filetype,function-docstring,git-repository,http-archive,integer-division,load,load-on-top,module-docstring,name-conventions,native-build,native-package,out-of-order-load,output-group,package-name,package-on-top,positional-args,redefined-variable,repository-name,return-value,same-origin-load,string-iteration,unreachable,unsorted-dict-items,unused-variable",
         "bazel:lint": "yarn bazel:format --lint=warn",
         "bazel:lint-fix": "yarn bazel:format --lint=fix",
         "format": "git-clang-format",

packages/jasmine/index.bzl

@@ -1,3 +1,20 @@
+# Copyright 2019 The Bazel 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.
+
+""" Public API surface is re-exported here.
+"""
+
 # Public API surface re-exports
 # Users shouldn't import under src/
 

packages/karma/karma_web_test.bzl

@@ -16,8 +16,8 @@
 load("@build_bazel_rules_nodejs//internal/common:expand_into_runfiles.bzl", "expand_path_into_runfiles")
 load("@build_bazel_rules_nodejs//internal/common:sources_aspect.bzl", "sources_aspect")
 load("@build_bazel_rules_nodejs//internal/js_library:js_library.bzl", "write_amd_names_shim")
-load("@io_bazel_rules_webtesting//web/internal:constants.bzl", "DEFAULT_WRAPPED_TEST_TAGS")
 load("@io_bazel_rules_webtesting//web:web.bzl", "web_test_suite")
+load("@io_bazel_rules_webtesting//web/internal:constants.bzl", "DEFAULT_WRAPPED_TEST_TAGS")
 load(":web_test.bzl", "COMMON_WEB_TEST_ATTRS")
 
 _CONF_TMPL = "//:karma.conf.js"

packages/karma/ts_web_test.bzl

@@ -13,8 +13,8 @@
 # limitations under the License.
 "Unit testing in a browser"
 
-load("@io_bazel_rules_webtesting//web/internal:constants.bzl", "DEFAULT_WRAPPED_TEST_TAGS")
 load("@io_bazel_rules_webtesting//web:web.bzl", "web_test_suite")
+load("@io_bazel_rules_webtesting//web/internal:constants.bzl", "DEFAULT_WRAPPED_TEST_TAGS")
 load(":karma_web_test.bzl", "KARMA_GENERIC_WEB_TEST_ATTRS", "run_karma_web_test")
 
 # Using generic karma_web_test attributes under the hood

packages/labs/webpack/index.bzl

@@ -1,3 +1,20 @@
+# Copyright 2019 The Bazel 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.
+
+""" Public API surface is re-exported here.
+"""
+
 # Public API surface re-exports
 # Users shouldn't import under src/
 

packages/labs/webpack/src/webpack_bundle.bzl

@@ -1,3 +1,22 @@
+# Copyright 2019 The Bazel 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.
+
+"""Contains the webpack_bundle rule
+
+This rule is experimental, as part of Angular Labs! There may be breaking changes.
+"""
+
 WEBPACK_BUNDLE_ATTRS = {
     "srcs": attr.label_list(allow_files = True),
     "entry_point": attr.label(allow_single_file = True, mandatory = True),

packages/typescript/defs.bzl

@@ -19,11 +19,11 @@ New rules will be added to index.bzl.
 """
 
 load("//:version.bzl", _check_rules_typescript_version = "check_rules_typescript_version")
-load("//internal/devserver:ts_devserver.bzl", _ts_devserver = "ts_devserver_macro")
-load("//internal/protobufjs:ts_proto_library.bzl", _ts_proto_library = "ts_proto_library")
 load("//internal:build_defs.bzl", _ts_library = "ts_library_macro")
 load("//internal:ts_config.bzl", _ts_config = "ts_config")
 load("//internal:ts_repositories.bzl", _ts_setup_workspace = "ts_setup_workspace")
+load("//internal/devserver:ts_devserver.bzl", _ts_devserver = "ts_devserver_macro")
+load("//internal/protobufjs:ts_proto_library.bzl", _ts_proto_library = "ts_proto_library")
 
 check_rules_typescript_version = _check_rules_typescript_version
 ts_setup_workspace = _ts_setup_workspace

packages/typescript/index.bzl

@@ -18,11 +18,11 @@ Users should not load files under "/internal"
 """
 
 load("//:version.bzl", _check_rules_typescript_version = "check_rules_typescript_version")
-load("//internal/devserver:ts_devserver.bzl", _ts_devserver = "ts_devserver_macro")
-load("//internal/protobufjs:ts_proto_library.bzl", _ts_proto_library = "ts_proto_library")
 load("//internal:build_defs.bzl", _ts_library = "ts_library_macro")
 load("//internal:ts_config.bzl", _ts_config = "ts_config")
 load("//internal:ts_repositories.bzl", _ts_setup_workspace = "ts_setup_workspace")
+load("//internal/devserver:ts_devserver.bzl", _ts_devserver = "ts_devserver_macro")
+load("//internal/protobufjs:ts_proto_library.bzl", _ts_proto_library = "ts_proto_library")
 
 check_rules_typescript_version = _check_rules_typescript_version
 ts_setup_workspace = _ts_setup_workspace

packages/typescript/index.docs.bzl

@@ -1,13 +1,29 @@
-# This contains references to the symbols we want documented.
-# We can't point stardoc to the top-level index.bzl since then it will see macros rather than the rules they wrap.
-# So this is a copy of index.bzl with macro indirection removed.
+# Copyright 2019 The Bazel 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.
+
+"""This contains references to the symbols we want documented.
+
+We can't point stardoc to the top-level index.bzl since then it will see macros rather than the rules they wrap.
+So this is a copy of index.bzl with macro indirection removed.
+"""
 
 load("//:version.bzl", _check_rules_typescript_version = "check_rules_typescript_version")
-load("//internal/devserver:ts_devserver.bzl", _ts_devserver = "ts_devserver")
-load("//internal/protobufjs:ts_proto_library.bzl", _ts_proto_library = "ts_proto_library")
 load("//internal:build_defs.bzl", _ts_library = "ts_library")
 load("//internal:ts_config.bzl", _ts_config = "ts_config")
 load("//internal:ts_repositories.bzl", _ts_setup_workspace = "ts_setup_workspace")
+load("//internal/devserver:ts_devserver.bzl", _ts_devserver = "ts_devserver")
+load("//internal/protobufjs:ts_proto_library.bzl", _ts_proto_library = "ts_proto_library")
 
 check_rules_typescript_version = _check_rules_typescript_version
 ts_setup_workspace = _ts_setup_workspace