diff --git a/go/tools/builders/compilepkg.go b/go/tools/builders/compilepkg.go index 09e3ef6e08..47eca65e9a 100644 --- a/go/tools/builders/compilepkg.go +++ b/go/tools/builders/compilepkg.go @@ -184,6 +184,15 @@ func compileArchive( } defer cleanup() + // As part of compilation process, rules_go does generate and/or rewrite code + // based on the original source files. We should only run static analysis + // over original source files and not the generated source as end users have + // little control over the generated source. + // + // nogoSrcsOrigin maps generated/rewritten source files back to original source. + // If the original source path is an empty string, exclude generated source from nogo run. + nogoSrcsOrigin := make(map[string]string) + if len(srcs.goSrcs) == 0 { // We need to run the compiler to create a valid archive, even if there's // nothing in it. GoPack will complain if we try to add assembly or cgo @@ -194,13 +203,13 @@ func compileArchive( // otherwise platforms without sandbox support may race to create/remove // the file during parallel compilation. emptyDir := filepath.Join(filepath.Dir(outPath), sanitizePathForIdentifier(importPath)) - if err := os.Mkdir(emptyDir, 0700); err != nil { + if err := os.Mkdir(emptyDir, 0o700); err != nil { return fmt.Errorf("could not create directory for _empty.go: %v", err) } defer os.RemoveAll(emptyDir) emptyPath := filepath.Join(emptyDir, "_empty.go") - if err := os.WriteFile(emptyPath, []byte("package empty\n"), 0666); err != nil { + if err := os.WriteFile(emptyPath, []byte("package empty\n"), 0o666); err != nil { return err } @@ -210,6 +219,8 @@ func compileArchive( matched: true, pkg: "empty", }) + + nogoSrcsOrigin[emptyPath] = "" } packageName := srcs.goSrcs[0].pkg var goSrcs, cgoSrcs []string @@ -290,9 +301,11 @@ func compileArchive( if i < len(goSrcs) { goSrcs[i] = coverSrc - } else { - cgoSrcs[i-len(goSrcs)] = coverSrc + nogoSrcsOrigin[coverSrc] = origSrc + continue } + + cgoSrcs[i-len(goSrcs)] = coverSrc } } @@ -312,7 +325,7 @@ func compileArchive( gcFlags = append(gcFlags, createTrimPath(gcFlags, srcDir)) } else { if cgoExportHPath != "" { - if err := ioutil.WriteFile(cgoExportHPath, nil, 0666); err != nil { + if err := ioutil.WriteFile(cgoExportHPath, nil, 0o666); err != nil { return err } } @@ -390,11 +403,31 @@ func compileArchive( // Run nogo concurrently. var nogoChan chan error outFactsPath := filepath.Join(workDir, nogoFact) - if nogoPath != "" { + nogoSrcs := make([]string, 0, len(goSrcs)) + for _, goSrc := range goSrcs { + // If source is found in the origin map, that means it's likely to be generated source file + // so feed the original source file to static analyzers instead of the generated one. + // + // If origin being empty, that means the generated source file has no origin (genrated by rules_go) + // thus ignore that entry entirely. + if originSrc, ok := nogoSrcsOrigin[goSrc]; ok { + if originSrc != "" { + nogoSrcs = append(nogoSrcs, originSrc) + } + continue + } + + // TODO(sluongng): most likely what remains here are CGO-generated source files as the result of calling cgo2() + // Need to determine whether we want to feed these CGO-generated files into static analyzers. + // + // Add unknown origin source files into the mix. + nogoSrcs = append(nogoSrcs, goSrc) + } + if nogoPath != "" && len(nogoSrcs) > 0 { ctx, cancel := context.WithCancel(context.Background()) nogoChan = make(chan error) go func() { - nogoChan <- runNogo(ctx, workDir, nogoPath, goSrcs, deps, packagePath, importcfgPath, outFactsPath) + nogoChan <- runNogo(ctx, workDir, nogoPath, nogoSrcs, deps, packagePath, importcfgPath, outFactsPath) }() defer func() { if nogoChan != nil { diff --git a/tests/core/nogo/coverage/README.rst b/tests/core/nogo/coverage/README.rst index cd1fdbf3f2..0f52b7945e 100644 --- a/tests/core/nogo/coverage/README.rst +++ b/tests/core/nogo/coverage/README.rst @@ -19,10 +19,5 @@ Verifies `#2146`_. gen_code_test ------------- -Checks how `nogo`_ would interact with source code that was generated as part of -rules_go's coverage implementation. Currently `nogo`_ would be run over these -generated source code, which is not desirable as end-user have very little control -over how these code were generated. - -In a future version, we shall flip this behavior so that `nogo`_ would not run -over source files that users have no control over. +Checks how `nogo`_ shoudl not interact with source code that was generated as part of +rules_go's coverage implementation. diff --git a/tests/core/nogo/coverage/gen_code_test.go b/tests/core/nogo/coverage/gen_code_test.go index 71614915bf..6f2c55b655 100644 --- a/tests/core/nogo/coverage/gen_code_test.go +++ b/tests/core/nogo/coverage/gen_code_test.go @@ -15,9 +15,6 @@ package gen_code_test import ( - "errors" - "os/exec" - "strings" "testing" "github.com/bazelbuild/rules_go/go/tools/bazel_testing" @@ -126,17 +123,8 @@ func run(pass *analysis.Pass) (interface{}, error) { } func TestNogoCoverGenCode(t *testing.T) { - out, err := bazel_testing.BazelOutput("coverage", "//:simple_test") - if err == nil { - t.Fatal("test should fail") - } - - var eErr *exec.ExitError - if errors.As(err, &eErr) && strings.Contains(string(eErr.Stderr), "was generated by rules_go (nocover)") { - // Expected failure - return + if out, err := bazel_testing.BazelOutput("coverage", "//:simple_test"); err != nil { + println(string(out)) + t.Fatal(err) } - - println(string(out)) - t.Fatal(err) } diff --git a/tests/core/nogo/generate/README.rst b/tests/core/nogo/generate/README.rst index 0d0cac0360..d9604fab5e 100644 --- a/tests/core/nogo/generate/README.rst +++ b/tests/core/nogo/generate/README.rst @@ -7,6 +7,6 @@ Tests to ensure `nogo`_ interaction with generated code. empty_test ------------- -Checks that `nogo`_ is running over the `_empty.go` file that was +Checks that `nogo`_ is not running over the `_empty.go` file that was generated as part of GoCompilePkg. diff --git a/tests/core/nogo/generate/empty_test.go b/tests/core/nogo/generate/empty_test.go index bdfa913836..a4f8de1266 100644 --- a/tests/core/nogo/generate/empty_test.go +++ b/tests/core/nogo/generate/empty_test.go @@ -15,9 +15,6 @@ package empty_test import ( - "errors" - "os/exec" - "strings" "testing" "github.com/bazelbuild/rules_go/go/tools/bazel_testing" @@ -98,19 +95,8 @@ func run(pass *analysis.Pass) (interface{}, error) { } func TestNogoGenEmptyCode(t *testing.T) { - out, err := bazel_testing.BazelOutput("build", "-k", "//:simple_test") - if err == nil { - t.Fatal("test should fail") - } - - var eErr *exec.ExitError - if errors.As(err, &eErr) && - strings.Contains(string(eErr.Stderr), "Detected generated source code from rules_go") && - strings.Contains(string(eErr.Stderr), "(noempty)") { - // Expected failure - return + if out, err := bazel_testing.BazelOutput("build", "-k", "//:simple_test"); err != nil { + println(string(out)) + t.Fatal(err) } - - println(string(out)) - t.Fatal(err) }