Add Bzlmod support for nogo (#3782)

* Add Bzlmod support for `nogo`

See `bzlmod.md` for the details. This change is backwards compatible for
WORKSPACE setups, but allows finer control over the set of targets nogo
is applied to when using Bzlmod.

* Add test for matches_scope

* Mention in nogo.rst

* Add go_bazel_test for nogo

Requires some changes to the `go_bazel_test` setup.

* Address review comments

.bazelci/presubmit.yml

@@ -16,6 +16,8 @@ tasks:
       - "//..."
     test_targets:
       - "//..."
+      # Bzlmod tests require Bazel 6+
+      - "-//tests/core/nogo/bzlmod/..."
   ubuntu2004:
     # enable some unflipped incompatible flags on this platform to ensure we don't regress.
     shell_commands:

BUILD.bazel

@@ -169,7 +169,10 @@ filegroup(
     testonly = True,
     srcs = [
         "BUILD.bazel",
+        "MODULE.bazel",
         "WORKSPACE",
+        "go.mod",
+        "go.sum",
         "//extras:all_files",
         "//go:all_files",
         "//proto:all_files",

MODULE.bazel

@@ -11,18 +11,17 @@ bazel_dep(name = "platforms", version = "0.0.4")
 bazel_dep(name = "rules_proto", version = "4.0.0")
 bazel_dep(name = "protobuf", version = "3.19.2", repo_name = "com_google_protobuf")
 
-non_module_dependencies = use_extension("//go/private:extensions.bzl", "non_module_dependencies")
-use_repo(
-    non_module_dependencies,
-    "io_bazel_rules_nogo",
-)
-
 go_sdk = use_extension("//go:extensions.bzl", "go_sdk")
 go_sdk.download(
     name = "go_default_sdk",
     version = "1.21.1",
 )
-use_repo(go_sdk, "go_toolchains")
+use_repo(
+    go_sdk,
+    "go_toolchains",
+    # This name is ugly on purpose to avoid a conflict with a user-named SDK.
+    "io_bazel_rules_nogo",
+)
 
 register_toolchains("@go_toolchains//:all")
 
@@ -40,4 +39,5 @@ use_repo(
     "org_golang_google_grpc_cmd_protoc_gen_go_grpc",
     "org_golang_google_protobuf",
     "org_golang_x_net",
+    "org_golang_x_tools",
 )

docs/go/core/bzlmod.md

@@ -21,7 +21,7 @@ bazel_dep(name = "rules_go", version = "0.39.1", repo_name = "io_bazel_rules_go"
 bazel_dep(name = "gazelle", version = "0.31.0", repo_name = "bazel_gazelle")
 ```
 
-## Registering Go SDKs
+## Go SDKs
 
 rules_go automatically downloads and registers a recent Go SDK, so unless a particular version is required, no manual steps are required.
 
@@ -68,11 +68,39 @@ If you really do need direct access to a Go SDK, you can provide the `name` attr
 Note that modules using this attribute cannot be added to registries such as the Bazel Central Registry (BCR).
 If you have a use case that would require this, please explain it in an issue.
 
+### Configuring `nogo`
+
+The `nogo` tool is a static analyzer for Go code that is run as part of compilation.
+It is configured via an instance of the [`nogo`](/go/nogo.rst) rule, which can then be registered with the `go_sdk` extension:
+
+```starlark
+go_sdk = use_extension("@rules_go//go:extensions.bzl", "go_sdk")
+go_sdk.nogo(nogo = "//:my_nogo")
+```
+
+By default, the `nogo` tool is executed for all Go targets in the main repository, but not any external repositories.
+Each module can only provide at most one `go_sdk.nogo` tag and only the tag of the root module is honored.
+
+It is also possible to include only or exclude particular packages from `nogo` analysis, using syntax that matches the `visibility` attribute on rules:
+
+```starlark
+go_sdk = use_extension("@rules_go//go:extensions.bzl", "go_sdk")
+go_sdk.nogo(
+    nogo = "//:my_nogo",
+    includes = [
+        "//:__subpackages__",
+        "@my_own_go_dep//logic:__pkg__",
+    ],
+    excludes = [
+        "//third_party:__subpackages__",
+    ],
+)
+```
+
 ### Not yet supported
 
 * `go_local_sdk`
 * `go_wrap_sdk`
-* nogo ([#3529](https://github.com/bazelbuild/rules_go/issues/3529))
 
 ## Generating BUILD files
 

go/def.bzl

@@ -74,11 +74,7 @@ load(
     _go_cross_binary = "go_cross_binary",
 )
 
-# TOOLS_NOGO is a list of all analysis passes in
-# golang.org/x/tools/go/analysis/passes.
-# This is not backward compatible, so use caution when depending on this --
-# new analyses may discover issues in existing builds.
-TOOLS_NOGO = [
+_TOOLS_NOGO = [
     "@org_golang_x_tools//go/analysis/passes/asmdecl:go_default_library",
     "@org_golang_x_tools//go/analysis/passes/assign:go_default_library",
     "@org_golang_x_tools//go/analysis/passes/atomic:go_default_library",
@@ -117,6 +113,12 @@ TOOLS_NOGO = [
     "@org_golang_x_tools//go/analysis/passes/unusedresult:go_default_library",
 ]
 
+# TOOLS_NOGO is a list of all analysis passes in
+# golang.org/x/tools/go/analysis/passes.
+# This is not backward compatible, so use caution when depending on this --
+# new analyses may discover issues in existing builds.
+TOOLS_NOGO = [str(Label(l)) for l in _TOOLS_NOGO]
+
 # Current version or next version to be tagged. Gazelle and other tools may
 # check this to determine compatibility.
 RULES_GO_VERSION = "0.43.0"

go/nogo.rst

@@ -3,6 +3,7 @@
 
 .. _nogo: nogo.rst#nogo
 .. _configuring-analyzers: nogo.rst#configuring-analyzers
+.. _Bzlmod: /docs/go/core/bzlmod.md#configuring-nogo
 .. _go_library: /docs/go/core/rules.md#go_library
 .. _analysis: https://godoc.org/golang.org/x/tools/go/analysis
 .. _Analyzer: https://godoc.org/golang.org/x/tools/go/analysis#Analyzer
@@ -71,7 +72,8 @@ want to run.
     )
 
 Pass a label for your `nogo`_ target to ``go_register_toolchains`` in your
-``WORKSPACE`` file.
+``WORKSPACE`` file. When using ``MODULE.bazel``, see the Bzlmod_ documentation
+instead.
 
 .. code:: bzl
 

go/private/BUILD.bazel

@@ -64,6 +64,7 @@ bzl_library(
         "@bazel_skylib//rules:common_settings",
         "@bazel_tools//tools/build_defs/cc:action_names.bzl",
         "@bazel_tools//tools/cpp:toolchain_utils.bzl",
+        "@io_bazel_rules_nogo//:scope.bzl",
     ],
 )
 

go/private/BUILD.nogo.bazel

@@ -4,3 +4,5 @@ alias(
     name = "nogo",
     actual = "{{nogo}}",
 )
+
+exports_files(["scope.bzl"])

go/private/actions/compilepkg.bzl

@@ -110,9 +110,10 @@ def emit_compilepkg(
 
     args.add("-o", out_lib)
     args.add("-x", out_export)
-    if go.nogo:
-        args.add("-nogo", go.nogo)
-        inputs.append(go.nogo)
+    nogo = go.get_nogo(go)
+    if nogo:
+        args.add("-nogo", nogo)
+        inputs.append(nogo)
     if out_cgo_export_h:
         args.add("-cgoexport", out_cgo_export_h)
         outputs.append(out_cgo_export_h)

go/private/context.bzl

@@ -66,6 +66,11 @@ load(
     "//go/private/rules:transition.bzl",
     "request_nogo_transition",
 )
+load(
+    "@io_bazel_rules_nogo//:scope.bzl",
+    NOGO_EXCLUDES = "EXCLUDES",
+    NOGO_INCLUDES = "INCLUDES",
+)
 
 # cgo requires a gcc/clang style compiler.
 # We use a denylist instead of an allowlist:
@@ -392,6 +397,33 @@ def _infer_importpath(ctx, attr):
         importpath = importpath[1:]
     return importpath, importpath, INFERRED_PATH
 
+def matches_scope(label, scope):
+    if scope == "all":
+        return True
+    if scope.workspace_name != label.workspace_name:
+        return False
+    if scope.name == "__pkg__":
+        return scope.package == label.package
+    if scope.name == "__subpackages__":
+        if not scope.package:
+            return True
+        return scope.package == label.package or label.package.startswith(scope.package + "/")
+    fail("invalid scope '%s'" % scope.name)
+
+def _matches_scopes(label, scopes):
+    for scope in scopes:
+        if matches_scope(label, scope):
+            return True
+    return False
+
+def _get_nogo(go):
+    """Returns the nogo file for this target, if enabled and in scope."""
+    label = go._ctx.label
+    if _matches_scopes(label, NOGO_INCLUDES) and not _matches_scopes(label, NOGO_EXCLUDES):
+        return go.nogo
+    else:
+        return None
+
 def go_context(ctx, attr = None):
     """Returns an API used to build Go code.
 
@@ -551,6 +583,7 @@ def go_context(ctx, attr = None):
         library_to_source = _library_to_source,
         declare_file = _declare_file,
         declare_directory = _declare_directory,
+        get_nogo = _get_nogo,
 
         # Private
         # TODO: All uses of this should be removed

go/private/extensions.bzl

@@ -14,7 +14,7 @@
 
 load("@bazel_features//:features.bzl", "bazel_features")
 load("//go/private:sdk.bzl", "detect_host_platform", "go_download_sdk_rule", "go_host_sdk_rule", "go_multiple_toolchains")
-load("//go/private:repositories.bzl", "go_rules_dependencies")
+load("//go/private:nogo.bzl", "DEFAULT_NOGO", "go_register_nogo")
 
 def host_compatible_toolchain_impl(ctx):
     ctx.file("BUILD.bazel")
@@ -67,6 +67,35 @@ _host_tag = tag_class(
     },
 )
 
+_NOGO_DEFAULT_INCLUDES = ["@@//:__subpackages__"]
+_NOGO_DEFAULT_EXCLUDES = []
+
+_nogo_tag = tag_class(
+    attrs = {
+        "nogo": attr.label(
+            doc = "The nogo target to use when this module is the root module.",
+        ),
+        "includes": attr.label_list(
+            default = _NOGO_DEFAULT_INCLUDES,
+            # The special include "all" is undocumented on purpose: With it, adding a new transitive
+            # dependency to a Go module can cause a build failure if the new dependency has lint
+            # issues.
+            doc = """
+A Go target is checked with nogo if its package matches at least one of the entries in 'includes'
+and none of the entries in 'excludes'. By default, nogo is applied to all targets in the main
+repository.
+
+Uses the same format as 'visibility', i.e., every entry must be a label that ends with ':__pkg__' or
+':__subpackages__'.
+""",
+        ),
+        "excludes": attr.label_list(
+            default = _NOGO_DEFAULT_EXCLUDES,
+            doc = "See 'includes'.",
+        ),
+    },
+)
+
 # A list of (goos, goarch) pairs that are commonly used for remote executors in cross-platform
 # builds (where host != exec platform). By default, we register toolchains for all of these
 # platforms in addition to the host platform.
@@ -85,6 +114,39 @@ _MAX_NUM_TOOLCHAINS = 9999
 _TOOLCHAIN_INDEX_PAD_LENGTH = len(str(_MAX_NUM_TOOLCHAINS))
 
 def _go_sdk_impl(ctx):
+    nogo_tag = struct(
+        nogo = DEFAULT_NOGO,
+        includes = _NOGO_DEFAULT_INCLUDES,
+        excludes = _NOGO_DEFAULT_EXCLUDES,
+    )
+    for module in ctx.modules:
+        if not module.is_root or not module.tags.nogo:
+            continue
+        if len(module.tags.nogo) > 1:
+            # Make use of the special formatting applied to tags by fail.
+            fail(
+                "go_sdk.nogo: only one tag can be specified per module, got:\n",
+                *[t for p in zip(module.tags.nogo, len(module.tags.nogo) * ["\n"]) for t in p]
+            )
+        nogo_tag = module.tags.nogo[0]
+        for scope in nogo_tag.includes + nogo_tag.excludes:
+            # Validate that the scope references a valid, visible repository.
+            # buildifier: disable=no-effect
+            scope.workspace_name
+            if scope.name != "__pkg__" and scope.name != "__subpackages__":
+                fail(
+                    "go_sdk.nogo: all entries in includes and excludes must end with ':__pkg__' or ':__subpackages__', got '{}' in".format(scope.name),
+                    nogo_tag,
+                )
+    go_register_nogo(
+        name = "io_bazel_rules_nogo",
+        nogo = str(nogo_tag.nogo),
+        # Go through canonical label literals to avoid a dependency edge on the packages in the
+        # scope.
+        includes = [str(l) for l in nogo_tag.includes],
+        excludes = [str(l) for l in nogo_tag.excludes],
+    )
+
     multi_version_module = {}
     for module in ctx.modules:
         if module.name in multi_version_module:
@@ -268,13 +330,7 @@ go_sdk = module_extension(
     tag_classes = {
         "download": _download_tag,
         "host": _host_tag,
+        "nogo": _nogo_tag,
     },
     **go_sdk_extra_kwargs
 )
-
-def _non_module_dependencies_impl(_ctx):
-    go_rules_dependencies(force = True)
-
-non_module_dependencies = module_extension(
-    implementation = _non_module_dependencies_impl,
-)

go/private/nogo.bzl

@@ -14,6 +14,15 @@
 
 DEFAULT_NOGO = "@io_bazel_rules_go//:default_nogo"
 
+# repr(Label(...)) does not emit a canonical label literal.
+def _label_repr(label):
+    return "Label(\"{}\")".format(label)
+
+def _scope_list_repr(scopes):
+    if scopes == ["all"]:
+        return repr(["all"])
+    return "[" + ", ".join([_label_repr(Label(l)) for l in scopes]) + "]"
+
 def _go_register_nogo_impl(ctx):
     ctx.template(
         "BUILD.bazel",
@@ -23,14 +32,30 @@ def _go_register_nogo_impl(ctx):
         },
         executable = False,
     )
+    ctx.file(
+        "scope.bzl",
+        """
+INCLUDES = {includes}
+EXCLUDES = {excludes}
+""".format(
+            includes = _scope_list_repr(ctx.attr.includes),
+            excludes = _scope_list_repr(ctx.attr.excludes),
+        ),
+        executable = False,
+    )
 
 # go_register_nogo creates a repository with an alias that points
 # to the nogo rule that should be used globally by go rules in the workspace.
 # This may be called automatically by go_rules_dependencies or by
 # go_register_toolchains.
+# With Bzlmod, it is created by the go_sdk extension.
 go_register_nogo = repository_rule(
     _go_register_nogo_impl,
     attrs = {
         "nogo": attr.string(mandatory = True),
+        # Special sentinel value used to let nogo run on all targets when using
+        # WORKSPACE, for backwards compatibility.
+        "includes": attr.string_list(default = ["all"]),
+        "excludes": attr.string_list(),
     },
 )

go/private/rules/nogo.bzl

@@ -115,7 +115,7 @@ def nogo(name, visibility = None, **kwargs):
     native.alias(
         name = name,
         actual = select({
-            "@io_bazel_rules_go//go/private:nogo_active": actual_name,
+            str(Label("//go/private:nogo_active")): actual_name,
             "//conditions:default": Label("//:default_nogo"),
         }),
         visibility = visibility,
@@ -140,11 +140,11 @@ def nogo(name, visibility = None, **kwargs):
 def nogo_wrapper(**kwargs):
     if kwargs.get("vet"):
         kwargs["deps"] = kwargs.get("deps", []) + [
-            "@org_golang_x_tools//go/analysis/passes/atomic:go_default_library",
-            "@org_golang_x_tools//go/analysis/passes/bools:go_default_library",
-            "@org_golang_x_tools//go/analysis/passes/buildtag:go_default_library",
-            "@org_golang_x_tools//go/analysis/passes/nilfunc:go_default_library",
-            "@org_golang_x_tools//go/analysis/passes/printf:go_default_library",
+            Label("@org_golang_x_tools//go/analysis/passes/atomic:go_default_library"),
+            Label("@org_golang_x_tools//go/analysis/passes/bools:go_default_library"),
+            Label("@org_golang_x_tools//go/analysis/passes/buildtag:go_default_library"),
+            Label("@org_golang_x_tools//go/analysis/passes/nilfunc:go_default_library"),
+            Label("@org_golang_x_tools//go/analysis/passes/printf:go_default_library"),
         ]
         kwargs = {k: v for k, v in kwargs.items() if k != "vet"}
     nogo(**kwargs)