Skip to content

Commit

Permalink
Only print type-checking error once in nogo (#4083)
Browse files Browse the repository at this point in the history
**What type of PR is this?**

Bug fix

**What does this PR do? Why is it needed?**

Previously, it was printed once by analyzer.

**Which issues(s) does this PR fix?**

Fixes #4080

**Other notes for review**
  • Loading branch information
fmeum authored and tyler-french committed Sep 5, 2024
1 parent 046d5bc commit 5e2e09d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 5 deletions.
13 changes: 10 additions & 3 deletions go/tools/builders/nogo_main.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,7 @@ func (act *action) execOnce() {
act.pass = pass

var err error
if act.pkg.illTyped && !pass.Analyzer.RunDespiteErrors {
err = fmt.Errorf("analysis skipped due to type-checking error: %v", act.pkg.typeCheckError)
} else {
if !act.pkg.illTyped || pass.Analyzer.RunDespiteErrors {
act.result, err = pass.Analyzer.Run(pass)
if err == nil {
if got, want := reflect.TypeOf(act.result), pass.Analyzer.ResultType; got != want {
Expand Down Expand Up @@ -471,7 +469,13 @@ func checkAnalysisResults(actions []*action, pkg *goPackage) string {
if cwd == "" || err != nil {
errs = append(errs, fmt.Errorf("nogo failed to get CWD: %w", err))
}
numSkipped := 0
for _, act := range actions {
if act.pkg.illTyped && !act.a.RunDespiteErrors {
// Don't report type-checking errors once per analyzer.
numSkipped++
continue
}
if act.err != nil {
// Analyzer failed.
errs = append(errs, fmt.Errorf("analyzer %q failed: %v", act.a.Name, act.err))
Expand Down Expand Up @@ -543,6 +547,9 @@ func checkAnalysisResults(actions []*action, pkg *goPackage) string {
}
}
}
if numSkipped > 0 {
errs = append(errs, fmt.Errorf("%d analyzers skipped due to type-checking error: %v", numSkipped, pkg.typeCheckError))
}
if len(diagnostics) == 0 && len(errs) == 0 {
return ""
}
Expand Down
31 changes: 29 additions & 2 deletions tests/core/nogo/custom/custom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestMain(m *testing.M) {
Nogo: "@//:nogo",
Main: `
-- BUILD.bazel --
load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo")
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "nogo")
nogo(
name = "nogo",
Expand Down Expand Up @@ -124,6 +124,12 @@ go_library(
importpath = "dep",
)
go_binary(
name = "type_check_fail",
srcs = ["type_check_fail.go"],
pure = "on",
)
-- foofuncname.go --
// foofuncname checks for functions named "Foo".
// It has the same package name as another check to test the checks with
Expand Down Expand Up @@ -434,6 +440,16 @@ func Foo() bool { // This should fail foofuncname
return Bar()
}
-- type_check_fail.go --
package type_check_fail
import (
"strings"
)
func Foo() bool {
return strings.Split("a,b,c", ",") // This fails type-checking
}
`,
})
}
Expand All @@ -443,6 +459,7 @@ func Test(t *testing.T) {
desc, config, target string
wantSuccess bool
includes, excludes []string
bazelArgs []string
}{
{
desc: "default_config",
Expand Down Expand Up @@ -507,6 +524,16 @@ func Test(t *testing.T) {
// note the cross platform regex :)
`examplepkg[\\/]pure_src_with_err_calling_native.go:.*function must not be named Foo \(foofuncname\)`,
},
}, {
desc: "type_check_fail",
config: "config.json",
target: "//:type_check_fail",
wantSuccess: false,
includes: []string{
"4 analyzers skipped due to type-checking error: type_check_fail.go:8:10:",
},
// Ensure that nogo runs even though compilation fails
bazelArgs: []string{"--keep_going"},
}, {
desc: "no_errors",
target: "//:no_errors",
Expand All @@ -527,7 +554,7 @@ func Test(t *testing.T) {
defer replaceInFile("BUILD.bazel", customConfig, origConfig)
}

cmd := bazel_testing.BazelCmd("build", test.target)
cmd := bazel_testing.BazelCmd(append([]string{"build", test.target}, test.bazelArgs...)...)
stderr := &bytes.Buffer{}
cmd.Stderr = stderr
if err := cmd.Run(); err == nil && !test.wantSuccess {
Expand Down

0 comments on commit 5e2e09d

Please sign in to comment.