rules_go improvement to externalize the nogo fix

go/private/actions/archive.bzl

@@ -64,10 +64,21 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
         out_facts = go.declare_file(go, name = source.library.name, ext = pre_ext + ".facts")
         out_nogo_log = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo.log")
         out_nogo_validation = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo")
+
+        # out_nogo_fix_tmp holds the fixes produced by the RunNogo action, out_nogo_fix holds the fixes produced by the ValidateNogo action.
+        # They have the same content, but ValidateNogo propagates the fixes and eventually externalizes the fixes via `_validation` in the OutputGroupInfo section.
+        # --run_validations (default=True) ensures nogo validation is applied to not only the input targets but also their dependent targets,
+        # thereby producing available fixes for all targets.
+        # Otherwise, if we externalize out_nogo_fix_tmp (not going through the ValidateNogo action) by putting it into a field (e.g., `nogo_fix`) in the OutputGroupInfo section of the input targets,
+        # we can see the fix for the input targets, but will miss the fixes for the dependent targets.
+        out_nogo_fix_tmp = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo.fix.tmp")
+        out_nogo_fix = go.declare_file(go, name = source.library.name, ext = pre_ext + ".nogo.fix")
     else:
         out_facts = None
         out_nogo_log = None
         out_nogo_validation = None
+        out_nogo_fix_tmp = None
+        out_nogo_fix = None
 
     direct = source.deps
 
@@ -113,6 +124,8 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
             out_facts = out_facts,
             out_nogo_log = out_nogo_log,
             out_nogo_validation = out_nogo_validation,
+            out_nogo_fix_tmp = out_nogo_fix_tmp,
+            out_nogo_fix = out_nogo_fix,
             nogo = nogo,
             out_cgo_export_h = out_cgo_export_h,
             gc_goopts = source.gc_goopts,
@@ -142,6 +155,8 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
             out_facts = out_facts,
             out_nogo_log = out_nogo_log,
             out_nogo_validation = out_nogo_validation,
+            out_nogo_fix_tmp = out_nogo_fix_tmp,
+            out_nogo_fix = out_nogo_fix,
             nogo = nogo,
             gc_goopts = source.gc_goopts,
             cgo = False,
@@ -187,6 +202,7 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
         facts_file = out_facts,
         runfiles = source.runfiles,
         _validation_output = out_nogo_validation,
+        _nogo_fix_output = out_nogo_fix,
         _cgo_deps = cgo_deps,
     )
     x_defs = dict(source.x_defs)

go/private/actions/compilepkg.bzl

@@ -70,6 +70,8 @@ def emit_compilepkg(
         out_facts = None,
         out_nogo_log = None,
         out_nogo_validation = None,
+        out_nogo_fix_tmp = None,
+        out_nogo_fix = None,
         nogo = None,
         out_cgo_export_h = None,
         gc_goopts = [],
@@ -87,6 +89,10 @@ def emit_compilepkg(
         fail("nogo must be specified if and only if out_nogo_log is specified")
     if bool(nogo) != bool(out_nogo_validation):
         fail("nogo must be specified if and only if out_nogo_validation is specified")
+    if bool(nogo) != bool(out_nogo_fix_tmp):
+        fail("nogo must be specified if and only if out_nogo_fix_tmp is specified")
+    if bool(nogo) != bool(out_nogo_fix):
+        fail("nogo must be specified if and only if out_nogo_fix is specified")
 
     if cover and go.coverdata:
         archives = archives + [go.coverdata]
@@ -221,6 +227,8 @@ def emit_compilepkg(
             out_facts = out_facts,
             out_log = out_nogo_log,
             out_validation = out_nogo_validation,
+            out_nogo_fix_tmp = out_nogo_fix_tmp,
+            out_nogo_fix = out_nogo_fix,
             nogo = nogo,
         )
 
@@ -238,6 +246,8 @@ def _run_nogo(
         out_facts,
         out_log,
         out_validation,
+        out_nogo_fix_tmp,
+        out_nogo_fix,
         nogo):
     """Runs nogo on Go source files, including those generated by cgo."""
     sdk = go.sdk
@@ -246,7 +256,7 @@ def _run_nogo(
                      [archive.data.facts_file for archive in archives if archive.data.facts_file] +
                      [archive.data.export_file for archive in archives])
     inputs_transitive = [sdk.tools, sdk.headers, go.stdlib.libs]
-    outputs = [out_facts, out_log]
+    outputs = [out_facts, out_log, out_nogo_fix_tmp]
 
     args = go.builder_args(go, "nogo", use_path_mapping = True)
     args.add_all(sources, before_each = "-src")
@@ -271,6 +281,7 @@ def _run_nogo(
     args.add_all(archives, before_each = "-facts", map_each = _facts)
     args.add("-out_facts", out_facts)
     args.add("-out_log", out_log)
+    args.add("-out_fix", out_nogo_fix_tmp)
     args.add("-nogo", nogo)
 
     # This action runs nogo and produces the facts files for downstream nogo actions.
@@ -299,9 +310,12 @@ def _run_nogo(
     validation_args.add("nogovalidation")
     validation_args.add(out_validation)
     validation_args.add(out_log)
+    validation_args.add(out_nogo_fix_tmp)
+    validation_args.add(out_nogo_fix)
+
     go.actions.run(
-        inputs = [out_log],
-        outputs = [out_validation],
+        inputs = [out_log, out_nogo_fix_tmp],
+        outputs = [out_validation, out_nogo_fix],
         mnemonic = "ValidateNogo",
         executable = go.toolchain._builder,
         arguments = [validation_args],

go/private/rules/binary.bzl

@@ -148,13 +148,20 @@ def _go_binary_impl(ctx):
         executable = executable,
     )
     validation_output = archive.data._validation_output
+    nogo_fix_output = archive.data._nogo_fix_output
+
+    nogo_validation_outputs = []
+    if validation_output:
+        nogo_validation_outputs.append(validation_output)
+    if nogo_fix_output:
+        nogo_validation_outputs.append(nogo_fix_output)
 
     providers = [
         archive,
         OutputGroupInfo(
             cgo_exports = archive.cgo_exports,
             compilation_outputs = [archive.data.file],
-            _validation = [validation_output] if validation_output else [],
+            _validation = nogo_validation_outputs,
         ),
     ]
 

go/private/rules/library.bzl

@@ -48,6 +48,13 @@ def _go_library_impl(ctx):
     source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented())
     archive = go.archive(go, source)
     validation_output = archive.data._validation_output
+    nogo_fix_output = archive.data._nogo_fix_output
+
+    nogo_validation_outputs = []
+    if validation_output:
+        nogo_validation_outputs.append(validation_output)
+    if nogo_fix_output:
+        nogo_validation_outputs.append(nogo_fix_output)
 
     return [
         library,
@@ -65,7 +72,7 @@ def _go_library_impl(ctx):
         OutputGroupInfo(
             cgo_exports = archive.cgo_exports,
             compilation_outputs = [archive.data.file],
-            _validation = [validation_output] if validation_output else [],
+            _validation = nogo_validation_outputs,
         ),
     ]
 

go/private/rules/test.bzl

@@ -75,6 +75,11 @@ def _go_test_impl(ctx):
     internal_archive = go.archive(go, internal_source)
     if internal_archive.data._validation_output:
         validation_outputs.append(internal_archive.data._validation_output)
+    if internal_archive.data._nogo_fix_output:
+        # We do not include those from external_archive that corresponds to a separate package
+        # since that package would be built separately, during which the nogo fixes are produced already.
+        validation_outputs.append(internal_archive.data._nogo_fix_output)
+
     go_srcs = [src for src in internal_source.srcs if src.extension == "go"]
 
     # Compile the library with the external black box tests

go/tools/builders/BUILD.bazel

@@ -31,6 +31,21 @@ go_test(
     ],
 )
 
+go_test(
+    name = "nogo_change_test",
+    size = "small",
+    srcs = [
+        "difflib.go",
+        "nogo_change.go",
+        "nogo_change_serialization.go",
+        "nogo_change_serialization_test.go",
+        "nogo_change_test.go",
+    ],
+    deps = [
+        "@org_golang_x_tools//go/analysis",
+    ],
+)
+
 go_test(
     name = "stdliblist_test",
     size = "small",
@@ -95,8 +110,11 @@ go_source(
     name = "nogo_srcs",
     srcs = [
         "constants.go",
+        "difflib.go",
         "env.go",
         "flags.go",
+        "nogo_change.go",
+        "nogo_change_serialization.go",
         "nogo_main.go",
         "nogo_typeparams_go117.go",
         "nogo_typeparams_go118.go",