Do not report nogo diagnostics for cgo generated files (#4081)

**What type of PR is this?**

Bug fix

**What does this PR do? Why is it needed?**

**Which issues(s) does this PR fix?**

Fixes #4079 

**Other notes for review**

Author: fmeum

go/private/actions/compilepkg.bzl

@@ -252,7 +252,7 @@ def _run_nogo(
     args.add_all(sources, before_each = "-src")
     if cgo_go_srcs:
         inputs_direct.append(cgo_go_srcs)
-        args.add_all([cgo_go_srcs], before_each = "-src")
+        args.add_all([cgo_go_srcs], before_each = "-ignore_src")
     if cover_mode:
         args.add("-cover_mode", cover_mode)
     if recompile_internal_deps:

go/tools/builders/nogo.go

@@ -20,13 +20,14 @@ func nogo(args []string) error {
 
 	fs := flag.NewFlagSet("GoNogo", flag.ExitOnError)
 	goenv := envFlags(fs)
-	var unfilteredSrcs, recompileInternalDeps multiFlag
+	var unfilteredSrcs, ignoreSrcs, recompileInternalDeps multiFlag
 	var deps, facts archiveMultiFlag
 	var importPath, packagePath, nogoPath, packageListPath string
 	var testFilter string
 	var outFactsPath, outLogPath string
 	var coverMode string
-	fs.Var(&unfilteredSrcs, "src", ".go, .c, .cc, .m, .mm, .s, or .S file to be filtered and compiled")
+	fs.Var(&unfilteredSrcs, "src", ".go, .c, .cc, .m, .mm, .s, or .S file to be filtered and checked")
+	fs.Var(&ignoreSrcs, "ignore_src", ".go, .c, .cc, .m, .mm, .s, or .S file to be filtered and checked, but with its diagnostics ignored")
 	fs.Var(&deps, "arc", "Import path, package path, and file name of a direct dependency, separated by '='")
 	fs.Var(&facts, "facts", "Import path, package path, and file name of a direct dependency's nogo facts file, separated by '='")
 	fs.StringVar(&importPath, "importpath", "", "The import path of the package being compiled. Not passed to the compiler, but may be displayed in debug data.")
@@ -49,7 +50,7 @@ func nogo(args []string) error {
 	}
 
 	// Filter sources.
-	srcs, err := filterAndSplitFiles(unfilteredSrcs)
+	srcs, err := filterAndSplitFiles(append(unfilteredSrcs, ignoreSrcs...))
 	if err != nil {
 		return err
 	}
@@ -81,10 +82,10 @@ func nogo(args []string) error {
 		return err
 	}
 
-	return runNogo(workDir, nogoPath, goSrcs, facts, importPath, importcfgPath, outFactsPath, outLogPath)
+	return runNogo(workDir, nogoPath, goSrcs, ignoreSrcs, facts, importPath, importcfgPath, outFactsPath, outLogPath)
 }
 
-func runNogo(workDir string, nogoPath string, srcs []string, facts []archive, packagePath, importcfgPath, outFactsPath string, outLogPath string) error {
+func runNogo(workDir string, nogoPath string, srcs, ignores []string, facts []archive, packagePath, importcfgPath, outFactsPath string, outLogPath string) error {
 	if len(srcs) == 0 {
 		// emit_compilepkg expects a nogo facts file, even if it's empty.
 		// We also need to write the validation output log.
@@ -105,6 +106,9 @@ func runNogo(workDir string, nogoPath string, srcs []string, facts []archive, pa
 		args = append(args, "-fact", fmt.Sprintf("%s=%s", fact.importPath, fact.file))
 	}
 	args = append(args, "-x", outFactsPath)
+	for _, ignore := range ignores {
+		args = append(args, "-ignore", ignore)
+	}
 	args = append(args, srcs...)
 
 	paramsFile := filepath.Join(workDir, "nogo.param")

go/tools/builders/nogo_main.go

@@ -77,6 +77,8 @@ func run(args []string) (error, int) {
 	importcfg := flags.String("importcfg", "", "The import configuration file")
 	packagePath := flags.String("p", "", "The package path (importmap) of the package being compiled")
 	xPath := flags.String("x", "", "The archive file where serialized facts should be written")
+	var ignores multiFlag
+	flags.Var(&ignores, "ignore", "Names of files to ignore")
 	flags.Parse(args)
 	srcs := flags.Args()
 
@@ -85,7 +87,7 @@ func run(args []string) (error, int) {
 		return fmt.Errorf("error parsing importcfg: %v", err), nogoError
 	}
 
-	diagnostics, facts, err := checkPackage(analyzers, *packagePath, packageFile, importMap, factMap, srcs)
+	diagnostics, facts, err := checkPackage(analyzers, *packagePath, packageFile, importMap, factMap, srcs, ignores)
 	if err != nil {
 		return fmt.Errorf("error running analyzers: %v", err), nogoError
 	}
@@ -156,7 +158,7 @@ func readImportCfg(file string) (packageFile map[string]string, importMap map[st
 // It returns an empty string if no source code diagnostics need to be printed.
 //
 // This implementation was adapted from that of golang.org/x/tools/go/checker/internal/checker.
-func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFile, importMap map[string]string, factMap map[string]string, filenames []string) (string, []byte, error) {
+func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFile, importMap map[string]string, factMap map[string]string, filenames, ignoreFiles []string) (string, []byte, error) {
 	// Register fact types and establish dependencies between analyzers.
 	actions := make(map[*analysis.Analyzer]*action)
 	var visit func(a *analysis.Analyzer) *action
@@ -211,8 +213,22 @@ func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFil
 		act.pkg = pkg
 	}
 
+	ignoreFilesSet := map[string]struct{}{}
+	for _, ignore := range ignoreFiles {
+		ignoreFilesSet[ignore] = struct{}{}
+	}
 	// Process nolint directives similar to golangci-lint.
+	// Also skip over fully ignored files.
 	for _, f := range pkg.syntax {
+		if _, ok := ignoreFilesSet[pkg.fset.Position(f.Pos()).Filename]; ok {
+			for _, act := range actions {
+				act.nolint = append(act.nolint, &Range{
+					from: pkg.fset.Position(f.FileStart),
+					to:   pkg.fset.Position(f.End()).Line,
+				})
+			}
+			continue
+		}
 		// CommentMap will correctly associate comments to the largest node group
 		// applicable. This handles inline comments that might trail a large
 		// assignment and will apply the comment to the entire assignment.

tests/core/nogo/custom/custom_test.go

@@ -39,6 +39,7 @@ nogo(
         ":foofuncname",
         ":importfmt",
         ":visibility",
+        ":cgogen",
     ],
     # config = "",
     visibility = ["//visibility:public"],
@@ -71,6 +72,14 @@ go_library(
     visibility = ["//visibility:public"],
 )
 
+go_library(
+    name = "cgogen",
+    srcs = ["cgogen.go"],
+    importpath = "cgogenanalyzer",
+    deps = ["@org_golang_x_tools//go/analysis"],
+    visibility = ["//visibility:public"],
+)
+
 go_library(
     name = "has_errors",
     srcs = ["has_errors.go"],
@@ -85,6 +94,13 @@ go_library(
     deps = [":dep"],
 )
 
+go_library(
+    name = "uses_cgo_clean",
+    srcs = ["examplepkg/uses_cgo_clean.go"],
+    importpath = "uses_cgo_clean",
+    cgo = True,
+)
+
 go_library(
     name = "uses_cgo_with_errors",
     srcs = [
@@ -109,7 +125,7 @@ go_library(
 )
 
 -- foofuncname.go --
-// importfmt checks for functions named "Foo".
+// foofuncname checks for functions named "Foo".
 // It has the same package name as another check to test the checks with
 // the same package name do not conflict.
 package importfmt
@@ -279,6 +295,40 @@ func run(pass *analysis.Pass) (interface{}, error) {
 
 	return nil, nil
 }
+-- cgogen.go --
+// cgogen reports diagnostics on files generated by cgo.
+package cgogen
+
+import (
+	"go/ast"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+const doc = "report synthetic diagnostics on files generated by cgo"
+
+var Analyzer = &analysis.Analyzer{
+	Name: "cgogen",
+	Run:  run,
+	Doc:  doc,
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	for _, f := range pass.Files {
+		ast.Inspect(f, func(n ast.Node) bool {
+			switch n := n.(type) {
+			case *ast.Comment:
+				if strings.HasPrefix(n.Text, "//go:linkname") || strings.HasPrefix(n.Text, "//go:cgo_import") {
+					pass.Reportf(n.Pos(), "nogo must not run on code generated by cgo")
+				}
+				return true
+			}
+			return true
+		})
+	}
+	return nil, nil
+}
 
 -- config.json --
 {
@@ -462,6 +512,10 @@ func Test(t *testing.T) {
 			target:      "//:no_errors",
 			wantSuccess: true,
 			excludes:    []string{"no_errors.go"},
+		}, {
+			desc:        "uses_cgo_clean",
+			target:      "//:uses_cgo_clean",
+			wantSuccess: true,
 		},
 	} {
 		t.Run(test.desc, func(t *testing.T) {
@@ -477,9 +531,9 @@ func Test(t *testing.T) {
 			stderr := &bytes.Buffer{}
 			cmd.Stderr = stderr
 			if err := cmd.Run(); err == nil && !test.wantSuccess {
-				t.Fatal("unexpected success")
+				t.Fatalf("unexpected success\n%s", stderr)
 			} else if err != nil && test.wantSuccess {
-				t.Fatalf("unexpected error: %v", err)
+				t.Fatalf("unexpected error: %v\n%s", err, stderr)
 			}
 
 			for _, pattern := range test.includes {