Skip to content

Commit

Permalink
Fix regression in empty .go file filtering for nogo
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fmeum committed Jan 18, 2024
1 parent 03bbd51 commit 2ea6f31
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 13 deletions.
21 changes: 14 additions & 7 deletions go/tools/builders/compilepkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -227,7 +228,7 @@ func compileArchive(
matched: true,
pkg: "empty",
})

emptyGoFilePath = emptyGoFile.Name()
}
packageName := srcs.goSrcs[0].pkg
var goSrcs, cgoSrcs []string
Expand Down Expand Up @@ -270,18 +271,24 @@ func compileArchive(
// constraints.
compilingWithCgo := haveCgo && cgoEnabled

filterForNogo := func(slice []string) []string {
var filtered []string
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
Expand Down
9 changes: 3 additions & 6 deletions tests/core/nogo/generate/empty_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ package noempty
import (
"fmt"
"path/filepath"
"strings"
"golang.org/x/tools/go/analysis"
)
Expand All @@ -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),
})
}
}
Expand Down

0 comments on commit 2ea6f31

Please sign in to comment.