Make Buildifier Sanity Test Strict (grpc#27807)

* Fix all lint errors in repo.

* Use strict buildifier by default

* Whoops. That file does not exist

* Attempt fix to buildifier invocation

* Add missing copyright

Author: gnossen

bazel/BUILD

@@ -12,12 +12,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""
+Contains build targets used by Starlark files in the bazel/ directory.
+"""
+
 licenses(["notice"])
 
 package(default_visibility = ["//:__subpackages__"])
 
-load(":cc_grpc_library.bzl", "cc_grpc_library")
-
 filegroup(
     name = "_gevent_test_main",
     srcs = ["_gevent_test_main.py"],

bazel/copts.bzl

@@ -12,6 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""
+Includes default copts.
+"""
+
 # This is a list of llvm flags to be used when being built with use_strict_warning=1
 GRPC_LLVM_WARNING_FLAGS = [
     # Enable all & extra warnings

bazel/custom_exec_properties.bzl

@@ -12,6 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""
+Reimports constants from the grpc_custom_exec_properties repo.
+"""
+
 load("@grpc_custom_exec_properties//:constants.bzl", _LARGE_MACHINE = "LARGE_MACHINE")
 
 LARGE_MACHINE = _LARGE_MACHINE

bazel/generate_cc.bzl

@@ -54,7 +54,13 @@ def _join_directories(directories):
     return "/".join(massaged_directories)
 
 def generate_cc_impl(ctx):
-    """Implementation of the generate_cc rule."""
+    """Implementation of the generate_cc rule.
+
+    Args:
+      ctx: The context object.
+    Returns:
+      The provider for the generated files.
+    """
     protos = [f for src in ctx.attr.srcs for f in src[ProtoInfo].check_deps_sources.to_list()]
     includes = [
         f
@@ -118,7 +124,7 @@ def generate_cc_impl(ctx):
         )
         tools = [ctx.executable.plugin]
     else:
-        arguments += ["--cpp_out=" + ",".join(ctx.attr.flags) + ":" + dir_out]
+        arguments.append("--cpp_out=" + ",".join(ctx.attr.flags) + ":" + dir_out)
         tools = []
 
     arguments += [
@@ -128,7 +134,7 @@ def generate_cc_impl(ctx):
 
     # Include the output directory so that protoc puts the generated code in the
     # right directory.
-    arguments += ["--proto_path={0}{1}".format(dir_out, proto_root)]
+    arguments.append("--proto_path={0}{1}".format(dir_out, proto_root))
     arguments += [_get_srcs_file_path(proto) for proto in protos]
 
     # create a list of well known proto files if the argument is non-None
@@ -138,11 +144,11 @@ def generate_cc_impl(ctx):
         if f != "external/com_google_protobuf/src/google/protobuf":
             print(
                 "Error: Only @com_google_protobuf//:well_known_protos is supported",
-            )
+            )  # buildifier: disable=print
         else:
             # f points to "external/com_google_protobuf/src/google/protobuf"
             # add -I argument to protoc so it knows where to look for the proto files.
-            arguments += ["-I{0}".format(f + "/../..")]
+            arguments.append("-I{0}".format(f + "/../.."))
             well_known_proto_files = [
                 f
                 for f in ctx.attr.well_known_protos.files.to_list()
@@ -157,7 +163,7 @@ def generate_cc_impl(ctx):
         use_default_shell_env = True,
     )
 
-    return struct(files = depset(out_files))
+    return struct(files = depset(out_files))  # buildifier: disable=rule-impl-return
 
 _generate_cc = rule(
     attrs = {

bazel/generate_objc.bzl

@@ -11,6 +11,11 @@
 # 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 module contains build rules relating to gRPC Objective-C.
+"""
+
 load("@rules_proto//proto:defs.bzl", "ProtoInfo")
 load(
     "//bazel:protobuf.bzl",
@@ -44,13 +49,13 @@ def _generate_objc_impl(ctx):
 
     outs = []
     for proto in protos:
-        outs += [_get_output_file_name_from_proto(proto, _PROTO_HEADER_FMT)]
-        outs += [_get_output_file_name_from_proto(proto, _PROTO_SRC_FMT)]
+        outs.append(_get_output_file_name_from_proto(proto, _PROTO_HEADER_FMT))
+        outs.append(_get_output_file_name_from_proto(proto, _PROTO_SRC_FMT))
 
         file_path = _get_full_path_from_file(proto)
         if file_path in files_with_rpc:
-            outs += [_get_output_file_name_from_proto(proto, _GRPC_PROTO_HEADER_FMT)]
-            outs += [_get_output_file_name_from_proto(proto, _GRPC_PROTO_SRC_FMT)]
+            outs.append(_get_output_file_name_from_proto(proto, _GRPC_PROTO_HEADER_FMT))
+            outs.append(_get_output_file_name_from_proto(proto, _GRPC_PROTO_SRC_FMT))
 
     out_files = [ctx.actions.declare_file(out) for out in outs]
     dir_out = _join_directories([
@@ -60,6 +65,7 @@ def _generate_objc_impl(ctx):
     ])
 
     arguments = []
+    tools = []
     if ctx.executable.plugin:
         arguments += get_plugin_args(
             ctx.executable.plugin,
@@ -68,17 +74,17 @@ def _generate_objc_impl(ctx):
             False,
         )
         tools = [ctx.executable.plugin]
-    arguments += ["--objc_out=" + dir_out]
+    arguments.append("--objc_out=" + dir_out)
 
-    arguments += ["--proto_path=."]
+    arguments.append("--proto_path=.")
     arguments += [
         "--proto_path={}".format(get_include_directory(i))
         for i in protos
     ]
 
     # Include the output directory so that protoc puts the generated code in the
     # right directory.
-    arguments += ["--proto_path={}".format(dir_out)]
+    arguments.append("--proto_path={}".format(dir_out))
     arguments += ["--proto_path={}".format(_get_directory_from_proto(proto)) for proto in protos]
     arguments += [_get_full_path_from_file(proto) for proto in protos]
 
@@ -88,7 +94,7 @@ def _generate_objc_impl(ctx):
         f = ctx.attr.well_known_protos.files.to_list()[0].dirname
 
         # go two levels up so that #import "google/protobuf/..." is correct
-        arguments += ["-I{0}".format(f + "/../..")]
+        arguments.append("-I{0}".format(f + "/../.."))
         well_known_proto_files = ctx.attr.well_known_protos.files.to_list()
     ctx.actions.run(
         inputs = protos + well_known_proto_files,
@@ -98,7 +104,7 @@ def _generate_objc_impl(ctx):
         arguments = arguments,
     )
 
-    return struct(files = depset(out_files))
+    return struct(files = depset(out_files))  # buildifier: disable=rule-impl-return
 
 def _label_to_full_file_path(src, package):
     if not src.startswith("//"):
@@ -198,7 +204,7 @@ def _group_objc_files_impl(ctx):
         for file in ctx.attr.src.files.to_list()
         if file.basename.endswith(suffix)
     ]
-    return struct(files = depset(out_files))
+    return struct(files = depset(out_files))  # buildifier: disable=rule-impl-return
 
 generate_objc_hdrs = rule(
     attrs = {

bazel/gevent_test.bzl

@@ -11,6 +11,11 @@
 # 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.
+
+"""
+Houses py_grpc_gevent_test.
+"""
+
 load("@grpc_python_dependencies//:requirements.bzl", "requirement")
 
 _GRPC_LIB = "//src/python/grpcio/grpc:grpcio"
@@ -24,6 +29,16 @@ def py_grpc_gevent_test(
         deps = None,
         data = None,
         **kwargs):
+    """Runs a Python test with gevent monkeypatched in.
+
+    Args:
+      name: The name of the test.
+      srcs: The source files.
+      main: The main file of the test.
+      deps: The dependencies of the test.
+      data: The data dependencies of the test.
+      **kwargs: Any other test arguments.
+    """
     if main == None:
         if len(srcs) != 1:
             fail("When main is not provided, srcs must be of size 1.")

bazel/grpc_build_system.bzl

@@ -23,6 +23,10 @@
 # each change must be ported from one to the other.
 #
 
+"""
+Contains macros used throughout the repo.
+"""
+
 load("//bazel:cc_grpc_library.bzl", "cc_grpc_library")
 load("//bazel:copts.bzl", "GRPC_DEFAULT_COPTS")
 load("@upb//bazel:upb_proto_library.bzl", "upb_proto_library", "upb_proto_reflection_library")
@@ -48,20 +52,20 @@ def _get_external_deps(external_deps):
     ret = []
     for dep in external_deps:
         if dep == "address_sorting":
-            ret += ["//third_party/address_sorting"]
+            ret.append("//third_party/address_sorting")
         elif dep == "xxhash":
-            ret += ["//third_party/xxhash"]
+            ret.append("//third_party/xxhash")
         elif dep == "cares":
             ret += select({
                 "//:grpc_no_ares": [],
                 "//conditions:default": ["//external:cares"],
             })
         elif dep == "cronet_c_for_grpc":
-            ret += ["//third_party/objective_c/Cronet:cronet_c_for_grpc"]
+            ret.append("//third_party/objective_c/Cronet:cronet_c_for_grpc")
         elif dep.startswith("absl/"):
-            ret += ["@com_google_absl//" + dep]
+            ret.append("@com_google_absl//" + dep)
         else:
-            ret += ["//external:" + dep]
+            ret.append("//external:" + dep)
     return ret
 
 def _update_visibility(visibility):
@@ -120,6 +124,27 @@ def grpc_cc_library(
         use_cfstream = False,
         tags = [],
         linkstatic = False):
+    """An internal wrapper around cc_library.
+
+    Args:
+      name: The name of the library.
+      srcs: The source files.
+      public_hdrs: The public headers.
+      hdrs: The headers.
+      external_deps: External depdendencies to be resolved.
+      defines: Build defines to use.
+      deps: cc_library deps.
+      select_deps: deps included conditionally.
+      standalone: Unused.
+      language: The language of the library, e.g. C, C++.
+      testonly: Whether the target is for tests only.
+      visibility: The visibility of the target.
+      alwayslink: Whether to enable alwayslink on the cc_library.
+      data: Data dependencies.
+      use_cfstream: Whether to use cfstream.
+      tags: Tags to apply to the rule.
+      linkstatic: Whether to enable linkstatic on the cc_library.
+    """
     visibility = _update_visibility(visibility)
     copts = []
     if use_cfstream:
@@ -197,6 +222,13 @@ def ios_cc_test(
         name,
         tags = [],
         **kwargs):
+    """An ios C++ test target.
+
+    Args:
+      name: The name of the test.
+      tags: The tags to apply to the test.
+      **kwargs: All other arguments to apply.
+    """
     ios_test_adapter = "//third_party/objective_c/google_toolbox_for_mac:GTM_GoogleTestRunner_GTM_USING_XCTEST"
 
     test_lib_ios = name + "_test_lib_ios"
@@ -221,6 +253,28 @@ def ios_cc_test(
         )
 
 def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data = [], uses_polling = True, language = "C++", size = "medium", timeout = None, tags = [], exec_compatible_with = [], exec_properties = {}, shard_count = None, flaky = None, copts = []):
+    """A cc_test target for use in the gRPC repo.
+
+    Args:
+        name: The name of the test.
+        srcs: The source files.
+        deps: The target deps.
+        external_deps: The external deps.
+        args: The args to supply to the test binary.
+        data: Data dependencies.
+        uses_polling: Whether the test uses polling.
+        language: The language of the test, e.g C, C++.
+        size: The size of the test.
+        timeout: The test timeout.
+        tags: The tags for the test.
+        exec_compatible_with: A list of constraint values that must be
+            satisifed for the platform.
+        exec_properties: A dictionary of strings that will be added to the
+            exec_properties of a platform selected for this target.
+        shard_count: The number of shards for this test.
+        flaky: Whether this test is flaky.
+        copts: Add these to the compiler invocation.
+    """
     copts = copts + if_mac(["-DGRPC_CFSTREAM"])
     if language.upper() == "C":
         copts = copts + if_not_windows(["-std=c99"])
@@ -283,6 +337,22 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data
     )
 
 def grpc_cc_binary(name, srcs = [], deps = [], external_deps = [], args = [], data = [], language = "C++", testonly = False, linkshared = False, linkopts = [], tags = [], features = []):
+    """Generates a cc_binary for use in the gRPC repo.
+
+    Args:
+      name: The name of the target.
+      srcs: The source files.
+      deps: The dependencies.
+      external_deps: The external dependencies.
+      args: The arguments to supply to the binary.
+      data: The data dependencies.
+      language: The language of the binary, e.g. C, C++.
+      testonly: Whether the binary is for tests only.
+      linkshared: Enables linkshared on the binary.
+      linkopts: linkopts to supply to the cc_binary.
+      tags: Tags to apply to the target.
+      features: features to be supplied to the cc_binary.
+    """
     copts = []
     if language.upper() == "C":
         copts = ["-std=c99"]
@@ -300,6 +370,7 @@ def grpc_cc_binary(name, srcs = [], deps = [], external_deps = [], args = [], da
         features = features,
     )
 
+# buildifier: disable=unnamed-macro
 def grpc_generate_one_off_targets():
     # In open-source, grpc_objc* libraries depend directly on //:grpc
     native.alias(
@@ -345,6 +416,13 @@ def grpc_py_binary(
     )
 
 def grpc_package(name, visibility = "private", features = []):
+    """Creates a package.
+
+    Args:
+        name: The name of the target
+        visibility: The visibility of the target.
+        features: The features to enable.
+    """
     if visibility == "tests":
         visibility = ["//test:__subpackages__"]
     elif visibility == "public":
@@ -355,6 +433,7 @@ def grpc_package(name, visibility = "private", features = []):
         fail("Unknown visibility " + visibility)
 
     if len(visibility) != 0:
+        # buildifier: disable=native-package
         native.package(
             default_visibility = visibility,
             features = features,
@@ -402,6 +481,7 @@ def grpc_upb_proto_library(name, deps):
 def grpc_upb_proto_reflection_library(name, deps):
     upb_proto_reflection_library(name = name, deps = deps)
 
+# buildifier: disable=unnamed-macro
 def python_config_settings():
     native.config_setting(
         name = "python3",