Fix regression in empty .go file filtering for nogo (#3832)

* Fix regression in empty `.go` file filtering for nogo 6236dd8 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 <[email protected]> --------- Co-authored-by: Tyler French <[email protected]>
bazelbuild · Jan 18, 2024 · 1a931cb · 1a931cb
1 parent 03bbd51
commit 1a931cb
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 13 deletions.
diff --git 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
@@ -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),
 			})
 		}
 	}