From 3abef0ba608035fcc9e698aea22317746913a59b Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 16 Jan 2024 20:14:03 +0100 Subject: [PATCH] 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. --- go/tools/builders/compilepkg.go | 18 +++++++++++++++--- tests/core/nogo/generate/empty_test.go | 9 +++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/go/tools/builders/compilepkg.go b/go/tools/builders/compilepkg.go index bcbdb64224..2589e6035a 100644 --- a/go/tools/builders/compilepkg.go +++ b/go/tools/builders/compilepkg.go @@ -203,6 +203,7 @@ func compileArchive( } defer cleanup() + ignoreForNogo := make(map[string]bool) 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", }) - + ignoreForNogo[emptyGoFile.Name()] = true } packageName := srcs.goSrcs[0].pkg var goSrcs, cgoSrcs []string @@ -270,16 +271,17 @@ func compileArchive( // constraints. compilingWithCgo := haveCgo && cgoEnabled + keepForNogo := func(s string) bool { return !ignoreForNogo[s] } // 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 + goSrcsNogo := filterSlice(goSrcs, keepForNogo) cgoSrcsNogo := 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...) + goSrcsNogo = filterSlice(append([]string{}, goSrcs...), keepForNogo) cgoSrcsNogo = append([]string{}, cgoSrcs...) relCoverPath := make(map[string]string) @@ -621,3 +623,13 @@ func sanitizePathForIdentifier(path string) string { return '_' }, path) } + +func filterSlice(slice []string, keep func(string) bool) []string { + var filtered []string + for _, s := range slice { + if keep(s) { + filtered = append(filtered, s) + } + } + return filtered +} diff --git a/tests/core/nogo/generate/empty_test.go b/tests/core/nogo/generate/empty_test.go index a4f8de1266..c195f0383b 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 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), }) } }