From 1a931cbe82ded089be5fff106c7d83a61cbee952 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 18 Jan 2024 21:54:12 +0100 Subject: [PATCH] Fix regression in empty `.go` file filtering for nogo (#3832) * Fix regression in empty `.go` file filtering for nogo 6236dd80670cdfe1477a484e98e3f82dbcc28fe9 regressed the nogo exclusion logic for the empty `.go` file generated for targets that otherwise don't contain any `.go` files. The test had been silently disabled a while ago when the name pattern for the empty file was changed. This is fixed by reintroducing the exclusion logic and making the test more strict. * Update go/tools/builders/compilepkg.go Co-authored-by: Tyler French <66684063+tyler-french@users.noreply.github.com> --------- Co-authored-by: Tyler French <66684063+tyler-french@users.noreply.github.com> --- go/tools/builders/compilepkg.go | 21 ++++++++++++++------- tests/core/nogo/generate/empty_test.go | 9 +++------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/go/tools/builders/compilepkg.go b/go/tools/builders/compilepkg.go index bcbdb64224..81800fed69 100644 --- a/go/tools/builders/compilepkg.go +++ b/go/tools/builders/compilepkg.go @@ -203,6 +203,7 @@ func compileArchive( } defer cleanup() + emptyGoFilePath := "" if len(srcs.goSrcs) == 0 { // We need to run the compiler to create a valid archive, even if there's nothing in it. // Otherwise, GoPack will complain if we try to add assembly or cgo objects. @@ -227,7 +228,7 @@ func compileArchive( matched: true, pkg: "empty", }) - + emptyGoFilePath = emptyGoFile.Name() } packageName := srcs.goSrcs[0].pkg var goSrcs, cgoSrcs []string @@ -270,18 +271,24 @@ func compileArchive( // constraints. compilingWithCgo := haveCgo && cgoEnabled + filterForNogo := func(slice []string) []string { + filtered := make([]string, 0, len(slice)) + for _, s := range slice { + // Do not subject the generated empty .go file to nogo checks. + if s != emptyGoFilePath { + filtered = append(filtered, s) + } + } + return filtered + } // When coverage is set, source files will be modified during instrumentation. We should only run static analysis // over original source files and not the modified ones. // goSrcsNogo and cgoSrcsNogo are copies of the original source files for nogo to run static analysis. - goSrcsNogo := goSrcs - cgoSrcsNogo := cgoSrcs + goSrcsNogo := filterForNogo(goSrcs) + cgoSrcsNogo := append([]string{}, cgoSrcs...) // Instrument source files for coverage. if coverMode != "" { - // deep copy original source files for nogo static analysis, avoid being modified by coverage. - goSrcsNogo = append([]string{}, goSrcs...) - cgoSrcsNogo = append([]string{}, cgoSrcs...) - relCoverPath := make(map[string]string) for _, s := range coverSrcs { relCoverPath[abs(s)] = s diff --git a/tests/core/nogo/generate/empty_test.go b/tests/core/nogo/generate/empty_test.go index a4f8de1266..395555b9aa 100644 --- a/tests/core/nogo/generate/empty_test.go +++ b/tests/core/nogo/generate/empty_test.go @@ -63,8 +63,6 @@ package noempty import ( "fmt" - "path/filepath" - "strings" "golang.org/x/tools/go/analysis" ) @@ -77,12 +75,11 @@ var Analyzer = &analysis.Analyzer{ func run(pass *analysis.Pass) (interface{}, error) { for _, f := range pass.Files { - pos := pass.Fset.PositionFor(f.Pos(), false) - - if strings.HasSuffix(pos.Filename, filepath.Join(".", "_empty.go")) { + // Fail on any package that isn't the "simple_test"'s package ('simple') or 'main'. + if f.Name.Name != "simple" && f.Name.Name != "main" { pass.Report(analysis.Diagnostic{ Pos: 0, - Message: fmt.Sprintf("Detected generated source code from rules_go: %s", pos.Filename), + Message: fmt.Sprintf("Detected generated source code from rules_go: package %s", f.Name.Name), }) } }