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 16, 2024
1 parent 31549c1 commit 3abef0b
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 9 deletions.
18 changes: 15 additions & 3 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()

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.
Expand All @@ -227,7 +228,7 @@ func compileArchive(
matched: true,
pkg: "empty",
})

ignoreForNogo[emptyGoFile.Name()] = true
}
packageName := srcs.goSrcs[0].pkg
var goSrcs, cgoSrcs []string
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
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 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 3abef0b

Please sign in to comment.