Skip to content

Commit

Permalink
Accept checks by name
Browse files Browse the repository at this point in the history
  • Loading branch information
jjti committed Jan 2, 2024
1 parent 0452507 commit e6faa1d
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 66 deletions.
102 changes: 59 additions & 43 deletions config.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package spancheck

import (
"errors"
"flag"
"fmt"
"log"
Expand All @@ -11,45 +10,59 @@ import (
)

// Check is a type of check that can be enabled or disabled.
type Check string
type Check int

const (
// EndCheck if enabled, checks that span.End() is called after span creation and before the function returns.
EndCheck Check = "end"
EndCheck Check = iota

// SetStatusCheck if enabled, checks that `span.SetStatus(codes.Error, msg)` is called when returning an error.
SetStatusCheck = "set-status"
SetStatusCheck

// RecordErrorCheck if enabled, checks that span.RecordError(err) is called when returning an error.
RecordErrorCheck = "record-error"
RecordErrorCheck
)

var (
// AllChecks is a list of all checks.
AllChecks = []string{
string(EndCheck),
string(SetStatusCheck),
string(RecordErrorCheck),
func (c Check) String() string {
switch c {
case EndCheck:
return "end"
case SetStatusCheck:
return "set-status"
case RecordErrorCheck:
return "record-error"
default:
return ""
}
}

errNoChecks = errors.New("no checks enabled")
errInvalidCheck = errors.New("invalid check")
var (
// Checks is a list of all checks by name.
Checks = map[string]Check{
EndCheck.String(): EndCheck,
SetStatusCheck.String(): SetStatusCheck,
RecordErrorCheck.String(): RecordErrorCheck,
}
)

// Config is a configuration for the spancheck analyzer.
type Config struct {
fs flag.FlagSet

// EnabledChecks is a list of checks that are enabled.
EnabledChecks []Check

// ignoreChecksSignatures is a regex that, if matched, disables the
// SetStatus and RecordError checks on error.
ignoreChecksSignatures *regexp.Regexp
// EnabledChecks is a list of checks to enable by name.
EnabledChecks []string

// IgnoreChecksSignaturesSlice is a slice of strings that are turned into
// the IgnoreSetStatusCheckSignatures regex.
IgnoreChecksSignaturesSlice []string

endCheckEnabled bool
setStatusEnabled bool
recordErrorEnabled bool

// ignoreChecksSignatures is a regex that, if matched, disables the
// SetStatus and RecordError checks on error.
ignoreChecksSignatures *regexp.Regexp
}

// NewConfigFromFlags returns a new Config with default values and flags for CLI usage.
Expand All @@ -59,16 +72,12 @@ func NewConfigFromFlags() *Config {
cfg.fs = flag.FlagSet{}

// Set the list of checks to enable.
checkDefault := []string{}
for _, check := range cfg.EnabledChecks {
checkDefault = append(checkDefault, string(check))
}
checkStrings := cfg.fs.String("checks", strings.Join(checkDefault, ","), fmt.Sprintf("comma-separated list of checks to enable (options: %v)", strings.Join(AllChecks, ", ")))
checks, err := parseChecks(*checkStrings)
if err != nil {
log.Default().Fatalf("failed to parse checks: %v", err)
checkOptions := []string{}
for check := range Checks {
checkOptions = append(checkOptions, check)
}
cfg.EnabledChecks = checks
checkStrings := cfg.fs.String("checks", "end", fmt.Sprintf("comma-separated list of checks to enable (options: %v)", strings.Join(checkOptions, ", ")))
cfg.EnabledChecks = strings.Split(*checkStrings, ",")

// Set the list of function signatures to ignore checks for.
ignoreCheckSignatures := flag.String("ignore-check-signatures", "", "comma-separated list of regex for function signatures that disable checks on errors")
Expand All @@ -80,41 +89,48 @@ func NewConfigFromFlags() *Config {
// NewDefaultConfig returns a new Config with default values.
func NewDefaultConfig() *Config {
return &Config{
EnabledChecks: []Check{EndCheck},
EnabledChecks: []string{EndCheck.String()},
}
}

// finalize parses checks and signatures from the public string slices of Config.
func (c *Config) finalize() {
c.parseSignatures()

checks := parseChecks(c.EnabledChecks)
c.endCheckEnabled = slices.Contains(checks, EndCheck)
c.setStatusEnabled = slices.Contains(checks, SetStatusCheck)
c.recordErrorEnabled = slices.Contains(checks, RecordErrorCheck)
}

// parseSignatures sets the Ignore*CheckSignatures regex from the string slices.
func (c *Config) parseSignatures() {
if c.ignoreChecksSignatures == nil && len(c.IgnoreChecksSignaturesSlice) > 0 {
c.ignoreChecksSignatures = createRegex(c.IgnoreChecksSignaturesSlice)
}
}

func parseChecks(checksFlag string) ([]Check, error) {
if checksFlag == "" {
return nil, errNoChecks
func parseChecks(checksSlice []string) []Check {
if len(checksSlice) == 0 {
return nil
}

checks := []Check{}
for _, check := range strings.Split(checksFlag, ",") {
check = strings.TrimSpace(check)
if check == "" {
for _, check := range checksSlice {
checkName := strings.TrimSpace(check)
if checkName == "" {
continue
}

if !slices.Contains(AllChecks, check) {
return nil, fmt.Errorf("%w: %s", errInvalidCheck, check)
check, ok := Checks[checkName]
if !ok {
continue
}

checks = append(checks, Check(check))
}

if len(checks) == 0 {
return nil, errNoChecks
checks = append(checks, check)
}

return checks, nil
return checks
}

func parseSignatures(sigFlag string) *regexp.Regexp {
Expand Down
13 changes: 4 additions & 9 deletions config_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package spancheck

import (
"strings"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -11,13 +12,12 @@ func Test_parseChecks(t *testing.T) {

for flag, tc := range map[string]struct {
checks []Check
err error
}{
"": {
err: errNoChecks,
checks: []Check{},
},
"unknown": {
err: errInvalidCheck,
checks: []Check{},
},
"end": {
checks: []Check{EndCheck},
Expand All @@ -34,12 +34,7 @@ func Test_parseChecks(t *testing.T) {
t.Parallel()
r := require.New(t)

checks, err := parseChecks(flag)
if tc.err != nil {
r.ErrorIs(err, tc.err)
return
}
r.NoError(err)
checks := parseChecks(strings.Split(flag, ","))
r.Equal(tc.checks, checks)
})
}
Expand Down
10 changes: 4 additions & 6 deletions spancheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"go/types"
"log"
"regexp"
"slices"
"strings"

"golang.org/x/tools/go/analysis"
Expand Down Expand Up @@ -42,8 +41,7 @@ func NewAnalyzerWithConfig(config *Config) *analysis.Analyzer {
}

func newAnalyzer(config *Config) *analysis.Analyzer {
// set up the config if slices were provided.
config.parseSignatures()
config.finalize()

return &analysis.Analyzer{
Name: "spancheck",
Expand Down Expand Up @@ -170,23 +168,23 @@ func runFunc(pass *analysis.Pass, node ast.Node, config *Config) {

// Check for missing calls.
for _, sv := range spanVars {
if slices.Contains(config.EnabledChecks, EndCheck) {
if config.endCheckEnabled {
// Check if there's no End to the span.
if ret := missingSpanCalls(pass, g, sv, "End", func(pass *analysis.Pass, ret *ast.ReturnStmt) *ast.ReturnStmt { return ret }, nil); ret != nil {
pass.ReportRangef(sv.stmt, "%s.End is not called on all paths, possible memory leak", sv.vr.Name())
pass.ReportRangef(ret, "return can be reached without calling %s.End", sv.vr.Name())
}
}

if slices.Contains(config.EnabledChecks, SetStatusCheck) {
if config.setStatusEnabled {
// Check if there's no SetStatus to the span setting an error.
if ret := missingSpanCalls(pass, g, sv, "SetStatus", returnsErr, config.ignoreChecksSignatures); ret != nil {
pass.ReportRangef(sv.stmt, "%s.SetStatus is not called on all paths", sv.vr.Name())
pass.ReportRangef(ret, "return can be reached without calling %s.SetStatus", sv.vr.Name())
}
}

if slices.Contains(config.EnabledChecks, RecordErrorCheck) {
if config.recordErrorEnabled {
// Check if there's no RecordError to the span setting an error.
if ret := missingSpanCalls(pass, g, sv, "RecordError", returnsErr, config.ignoreChecksSignatures); ret != nil {
pass.ReportRangef(sv.stmt, "%s.RecordError is not called on all paths", sv.vr.Name())
Expand Down
16 changes: 8 additions & 8 deletions spancheck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ func Test(t *testing.T) {
for dir, config := range map[string]*spancheck.Config{
"base": spancheck.NewConfigFromFlags(),
"disableerrorchecks": {
EnabledChecks: []spancheck.Check{
spancheck.EndCheck,
spancheck.RecordErrorCheck,
spancheck.SetStatusCheck,
EnabledChecks: []string{
spancheck.EndCheck.String(),
spancheck.RecordErrorCheck.String(),
spancheck.SetStatusCheck.String(),
},
IgnoreChecksSignaturesSlice: []string{"telemetry.Record", "recordErr"},
},
"enableall": {
EnabledChecks: []spancheck.Check{
spancheck.EndCheck,
spancheck.RecordErrorCheck,
spancheck.SetStatusCheck,
EnabledChecks: []string{
spancheck.EndCheck.String(),
spancheck.RecordErrorCheck.String(),
spancheck.SetStatusCheck.String(),
},
},
} {
Expand Down

0 comments on commit e6faa1d

Please sign in to comment.