bazel: add sgexit lint to nogo (#59092)

Closes https://github.com/sourcegraph/sourcegraph/issues/54847

## Test plan

Sample deliberate error:

```
$ bazel build //dev/sg
INFO: Analyzed target //dev/sg:sg (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
ERROR: /home/noah/Sourcegraph/sourcegraph/dev/sg/BUILD.bazel:128:10: GoCompilePkg dev/sg/sg.a failed: (Exit 1): builder failed: error executing command (from target //dev/sg:sg) bazel-out/k8-opt-exec-2B5CBBC6/bin/external/go_sdk/builder_reset/builder compilepkg -sdk external/go_sdk -installsuffix linux_amd64 -src dev/sg/analytics.go -src dev/sg/checks.go -src ... (remaining 245 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
compilepkg: nogo: errors found by nogo during build-time code analysis:
dev/sg/sg_run.go:28:4: use of `signal.Notify` forbidden because "breaks dev/sg/internal/interrupt" (forbidigo)
Aspect @rules_rust//rust/private:clippy.bzl%rust_clippy_aspect of //dev/sg:sg up-to-date (nothing to build)
INFO: Elapsed time: 52.557s, Critical Path: 26.79s
INFO: 1435 processes: 2 internal, 1433 linux-sandbox.
FAILED: Build did NOT complete successfully
```
This commit is contained in:
Noah S-C 2023-12-19 14:46:33 +00:00 committed by GitHub
parent 792afc7ca3
commit 85b09cc66d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 51 additions and 32 deletions

View File

@ -9,6 +9,7 @@ go_library(
"//dev/linters/nolint",
"@com_github_ashanbrown_forbidigo//forbidigo:go_default_library", #keep
"@com_github_ashanbrown_forbidigo//pkg/analyzer:go_default_library", #keep
"@com_github_gobwas_glob//:glob",
"@org_golang_x_tools//go/analysis",
],
)

View File

@ -3,9 +3,11 @@ package forbidigo
import (
"fmt"
"go/ast"
"strings"
"github.com/ashanbrown/forbidigo/forbidigo"
"github.com/ashanbrown/forbidigo/pkg/analyzer"
"github.com/gobwas/glob"
"golang.org/x/tools/go/analysis"
"github.com/sourcegraph/sourcegraph/dev/linters/nolint"
@ -14,11 +16,23 @@ import (
// Analyzer is the analyzer nogo should use
var Analyzer = nolint.Wrap(analyzer.NewAnalyzer())
// defaultPatterns the patterns forbigigo should ban if they match.
// patterns the patterns forbigigo should ban if they match.
// JSON based off the pattern struct:
// https://sourcegraph.com/github.com/ashanbrown/forbidigo@57bf5a72a4f5c3c28dce5a8aebb37a9be36bece7/-/blob/forbidigo/patterns.go?L13-29
var defaultPatterns = []string{
fmt.Sprintf(`{"p":"^fmt\\.Errorf$", "msg": "%s should be used instead"}`, "`errors.Newf`"),
var patterns = []struct {
Pattern string
ExcludeFile func(path string) bool
}{
{
Pattern: fmt.Sprintf(`{"p": "^fmt\\.Errorf$", "msg": "%s should be used instead"}`, "`errors.Newf`"),
},
{
Pattern: `{"p": "^(os\\.Exit|signal\\.Notify|logger\\.Fatal|log\\.Fatal)$", "msg": "it breaks dev/sg/internal/interrupt"}`,
ExcludeFile: func(path string) bool {
match := glob.MustCompile("**dev/sg/**.go", '/')
return !match.Match(path) || (strings.Contains(path, "dev/sg/interrupt") || strings.HasSuffix(path, "_test.go") || strings.HasSuffix(path, "dev/sg/linters/go_checks.go"))
},
},
}
var config = struct {
@ -39,34 +53,38 @@ func init() {
// runAnalysis is copied from forbigigo and slightly modified
func runAnalysis(pass *analysis.Pass) (interface{}, error) {
linter, err := forbidigo.NewLinter(defaultPatterns,
forbidigo.OptionIgnorePermitDirectives(config.IgnorePermitDirective),
forbidigo.OptionExcludeGodocExamples(config.ExcludeGodocExamples),
forbidigo.OptionAnalyzeTypes(config.AnalyzeTypes),
)
if err != nil {
return nil, err
}
nodes := make([]ast.Node, 0, len(pass.Files))
for _, f := range pass.Files {
nodes = append(nodes, f)
}
runConfig := forbidigo.RunConfig{Fset: pass.Fset}
if config.AnalyzeTypes {
runConfig.TypesInfo = pass.TypesInfo
}
issues, err := linter.RunWithConfig(runConfig, nodes...)
if err != nil {
return nil, err
}
for _, i := range issues {
diag := analysis.Diagnostic{
Pos: i.Pos(),
Message: i.Details(),
Category: "restriction",
for _, group := range patterns {
linter, err := forbidigo.NewLinter([]string{group.Pattern},
forbidigo.OptionIgnorePermitDirectives(config.IgnorePermitDirective),
forbidigo.OptionExcludeGodocExamples(config.ExcludeGodocExamples),
forbidigo.OptionAnalyzeTypes(config.AnalyzeTypes),
)
if err != nil {
return nil, err
}
nodes := make([]ast.Node, 0, len(pass.Files))
for _, f := range pass.Files {
if group.ExcludeFile != nil && group.ExcludeFile(pass.Fset.Position(f.Package).Filename) {
continue
}
nodes = append(nodes, f)
}
runConfig := forbidigo.RunConfig{Fset: pass.Fset, TypesInfo: pass.TypesInfo}
issues, err := linter.RunWithConfig(runConfig, nodes...)
if err != nil {
return nil, err
}
for _, i := range issues {
diag := analysis.Diagnostic{
Pos: i.Pos(),
Message: i.Details(),
Category: "restriction",
}
pass.Report(diag)
}
pass.Report(diag)
}
return nil, nil
}

View File

@ -7,6 +7,7 @@ require (
github.com/OpenPeeDeeP/depguard/v2 v2.0.1
github.com/ashanbrown/forbidigo v1.5.1
github.com/go-critic/go-critic v0.6.7
github.com/gobwas/glob v0.2.3
github.com/gordonklaus/ineffassign v0.0.0-20230107090616-13ace0543b28
github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db
github.com/kyoh86/exportloopref v0.1.11
@ -24,7 +25,6 @@ require (
github.com/cockroachdb/redact v1.1.5 // indirect
github.com/getsentry/sentry-go v0.25.0 // indirect
github.com/go-toolsmith/astfmt v1.1.0 // indirect
github.com/gobwas/glob v0.2.3 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/gostaticanalysis/analysisutil v0.7.1 // indirect

View File

@ -43,7 +43,7 @@ var Targets = []Target{
lintLoggingLibraries(),
onlyLocal(lintTracingLibraries()),
goModGuards(),
lintSGExit(),
onlyLocal(lintSGExit()),
},
},
{