Allow to exclude certain target from nogo facts collection

go/nogo.rst

@@ -77,14 +77,20 @@ instead.
     go_register_toolchains(version = "1.23.1")
     go_register_nogo(
       nogo = "@//:my_nogo"  # my_nogo is in the top-level BUILD file of this workspace
-      includes = ["@//:__subpackages__"],  # Labels to lint. By default only lints code in workspace.
-      excludes = ["@//generated:__subpackages__"],  # Labels to exclude.
+      excludes = ["@//generated:__subpackages__"],  # Labels to exclude from nogo facts collection. This is unsafe. See notes below.
     )
 
 **NOTE**: You must include ``"@//"`` prefix when referring to targets in the local
 workspace. Also note that you cannot use this to refer to bzlmod repos, as the labels
 don't go though repo mapping.
 
+Although nogo only reports errors in those files in `only_files` is specified for analyzers,
+it still collects facts from all targets in the transitive dependency closure, because it needs
+those facts to accurately reports the errors. However, this can be slow, especially in large Go packages,
+which could be generated. For performance reasons, users can exclude them with `excludes`.
+This is unsafe, and should only be used to exclude packages that don't provide
+useful nogo facts for the configured set of analyzers.
+
 The `nogo`_ rule will generate a program that executes all the supplied
 analyzers at build-time. The generated ``nogo`` program will run alongside the
 compiler when building any Go target (e.g. `go_library`_) within your workspace,
@@ -278,6 +284,9 @@ contain the following key-value pairs:
 | Keys in ``exclude_files`` override keys in ``only_files``. If a .go file matches a key present   |
 | in both ``only_files`` and ``exclude_files``, the analyzer will not emit diagnostics for that    |
 | file.                                                                                            |
+| The key difference between ``excludes_files`` and ``excludes`` from ``go_register_nogo`` is that |
+| the former is used to exclude files from reporting errors, while the latter is to prevent        |
+| nogo from collecting facts from certain packages.                                                |
 +----------------------------+---------------------------------------------------------------------+
 | ``"analyzer_flags"``       | :type:`dictionary, string to string`                                |
 +----------------------------+---------------------------------------------------------------------+

go/private/actions/archive.bzl

@@ -14,7 +14,7 @@
 
 load(
     "//go/private:context.bzl",
-    "validate_nogo",
+    "get_nogo",
 )
 load(
     "//go/private:mode.bzl",
@@ -59,16 +59,13 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
     out_export = go.declare_file(go, name = source.name, ext = pre_ext + ".x")
     out_cgo_export_h = None  # set if cgo used in c-shared or c-archive mode
 
-    nogo = go.nogo
+    nogo = get_nogo(go)
 
     # nogo is a FilesToRunProvider and some targets don't have it, some have it but no executable.
     if nogo != None and nogo.executable != None:
         out_facts = go.declare_file(go, name = source.name, ext = pre_ext + ".facts")
         out_diagnostics = go.declare_directory(go, name = source.name, ext = pre_ext + "_nogo")
-        if validate_nogo(go):
-            out_nogo_validation = go.declare_file(go, name = source.name, ext = pre_ext + ".nogo")
-        else:
-            out_nogo_validation = None
+        out_nogo_validation = go.declare_file(go, name = source.name, ext = pre_ext + ".nogo")
     else:
         out_facts = None
         out_diagnostics = None

go/private/context.bzl

@@ -38,7 +38,6 @@ load("@io_bazel_rules_go_bazel_features//:features.bzl", "bazel_features")
 load(
     "@io_bazel_rules_nogo//:scope.bzl",
     NOGO_EXCLUDES = "EXCLUDES",
-    NOGO_INCLUDES = "INCLUDES",
 )
 load(
     "//go/platform:apple.bzl",
@@ -429,11 +428,13 @@ def _matches_scopes(label, scopes):
             return True
     return False
 
-def validate_nogo(go):
-    """Whether nogo should be run as a validation action rather than just to generate fact files for the current
-    target."""
+def get_nogo(go):
+    """Returns the nogo file for this target, if enabled and in scope."""
     label = go.label
-    return _matches_scopes(label, NOGO_INCLUDES) and not _matches_scopes(label, NOGO_EXCLUDES)
+    if not _matches_scopes(label, NOGO_EXCLUDES):
+        return go.nogo
+    else:
+        return None
 
 default_go_config_info = GoConfigInfo(
     static = False,

go/private/extensions.bzl

@@ -83,17 +83,17 @@ _nogo_tag = tag_class(
             # 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__'.
+Deprecated. Use the JSON config file to specify the include list.
 """,
         ),
         "excludes": attr.label_list(
             default = NOGO_DEFAULT_EXCLUDES,
-            doc = "See 'includes'.",
+            doc = """
+Nogo will generate facts from all Go targets, unless excluded here. 
+
+Uses the same format as 'visibility', i.e., every entry must be a label that ends with ':__pkg__' or
+':__subpackages__'.
+            """,
         ),
     },
 )
@@ -159,7 +159,7 @@ def _go_sdk_impl(ctx):
                 *[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:
+        for scope in nogo_tag.excludes:
             # Validate that the scope references a valid, visible repository.
             # buildifier: disable=no-effect
             scope.repo_name
@@ -168,6 +168,7 @@ def _go_sdk_impl(ctx):
                     "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/private/nogo.bzl

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 DEFAULT_NOGO = "@io_bazel_rules_go//:default_nogo"
-NOGO_DEFAULT_INCLUDES = ["@@//:__subpackages__"]
+NOGO_DEFAULT_INCLUDES = ["all"]
 NOGO_DEFAULT_EXCLUDES = []
 
 # repr(Label(...)) does not emit a canonical label literal.
@@ -34,13 +34,14 @@ def _go_register_nogo_impl(ctx):
         },
         executable = False,
     )
+    if ctx.attr.includes != NOGO_DEFAULT_INCLUDES:
+        print("go_register_nogo's include attribute is no-op. Nogo now collect facts from all targets by default. To include files in nogo validation, please use only_files in the JSON: https://github.com/bazel-contrib/rules_go/blob/master/go/nogo.rst#example")
+
     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,
@@ -55,8 +56,6 @@ 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/tools/builders/nogo_main.go

@@ -512,6 +512,8 @@ func (g *goPackage) String() string {
 	return g.types.Path()
 }
 
+var _externalFilePattern = regexp.MustCompile("^external/")
+
 // checkAnalysisResults checks the analysis diagnostics in the given actions
 // and returns a string containing all the diagnostics that should be printed
 // to the build log.
@@ -542,6 +544,10 @@ func checkAnalysisResults(actions []*action, pkg *goPackage) ([]diagnosticEntry,
 		if baseConfig, ok := configs[nogoBaseConfigName]; ok {
 			currentConfig = baseConfig
 		}
+		if currentConfig.excludeFiles == nil {
+			// TODO(linzhp): add a test to verify the case of [].
+			currentConfig.excludeFiles = []*regexp.Regexp{_externalFilePattern}
+		}
 		// Overwrite the config with the desired config. Any unset fields
 		// in the config will default to the base config.
 		if actionConfig, ok := configs[act.a.Name]; ok {

tests/core/nogo/bzlmod/includes_excludes_test.go

@@ -30,6 +30,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo", "TOOLS_NOGO")
 
 nogo(
     name = "my_nogo",
+    config = ":nogo.json",
     visibility = ["//visibility:public"],
     deps = TOOLS_NOGO,
 )
@@ -93,13 +94,23 @@ func useless() string {
 	}
 	return foo
 }
+
+-- nogo.json --
+{
+  "_base": {
+    "only_files": {
+      "^go/": "go files"
+    },
+    "exclude_files": {
+      "^go/third_party/": "vendored"
+    }
+  }
+}
 `,
 		ModuleFileSuffix: `
 go_sdk = use_extension("@io_bazel_rules_go//go:extensions.bzl", "go_sdk")
 go_sdk.nogo(
     nogo = "//:my_nogo",
-    includes = ["//go:__subpackages__"],
-    excludes = ["//go/third_party:__subpackages__"],
 )
 `,
 	})

tests/core/nogo/includes_excludes/includes_excludes_test.go

@@ -23,15 +23,14 @@ import (
 
 func TestMain(m *testing.M) {
 	bazel_testing.TestMain(m, bazel_testing.Args{
-		Nogo:         "@//:my_nogo",
-		NogoIncludes: []string{"@//go:__subpackages__"},
-		NogoExcludes: []string{"@//go/third_party:__subpackages__"},
+		Nogo: "@io_bazel_rules_go//:tools_nogo",
 		Main: `
 -- BUILD.bazel --
 load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo", "TOOLS_NOGO")
 
 nogo(
     name = "my_nogo",
+    config = ":nogo.json",
     visibility = ["//visibility:public"],
     deps = TOOLS_NOGO,
 )
@@ -95,6 +94,24 @@ func useless() string {
 	}
 	return foo
 }
+
+-- nogo.json --
+{
+  "_base": {
+    "only_files": {
+      "^go/": "go files"
+    },
+    "exclude_files": {
+      "^go/third_party/": "vendored"
+    }
+  }
+}
+`,
+		ModuleFileSuffix: `
+go_sdk = use_extension("@io_bazel_rules_go//go:extensions.bzl", "go_sdk")
+go_sdk.nogo(
+    nogo = "//:my_nogo",
+)
 `,
 	})
 }