Rework checks in response to pr feedback

- checks is a slice now, not a set of boolean enable/disables

README.md

@@ -16,6 +16,10 @@ spancheck ./...
 
 ## Example
 
+```bash
+spancheck -checks 'end,set-status,record-error' ./...
+```
+
 ```go
 func _() error {
     // span.End is not called on all paths, possible memory leak
@@ -36,24 +40,16 @@ func _() error {
 
 ## Configuration
 
-Only the `span.End()` check is enabled by default. The others can be enabled with `-enable-all`, `-enable-record-error-check`, or `-enable-set-status-check`.
+Only the `span.End()` check is enabled by default. The others can be enabled with `-checks 'end,set-status,record-error'`.
 
 ```txt
 $ spancheck -h
 ...
 Flags:
-  -disable-end-check
-        disable the check for calling span.End() after span creation
-  -enable-all
-        enable all checks, overriding individual check flags
-  -enable-record-error-check
-        enable check for a span.RecordError(err) call when returning an error
-  -enable-set-status-check
-        enable check for a span.SetStatus(codes.Error, msg) call when returning an error
-  -ignore-record-error-check-signatures string
-        comma-separated list of function signature regex that disable the span.RecordError(err) check on errors
-  -ignore-set-status-check-signatures string
-        comma-separated list of function signature regex that disable the span.SetStatus(codes.Error, msg) check on errors
+  -checks string
+        comma-separated list of checks to enable (options: end, set-status, record-error) (default "end")
+  -ignore-check-signatures string
+        comma-separated list of regex for function signatures that disable checks on errors
 ```
 
 ### Ignore check signatures
@@ -87,10 +83,10 @@ func recordErr(span trace.Span, err error) error {
 }
 ```
 
-The warnings are can be ignored by setting `-ignore-set-status-check-signatures` flag to `recordErr`:
+The warnings are can be ignored by setting `-ignore-check-signatures` flag to `recordErr`:
 
 ```bash
-spancheck -enable-set-status-check -ignore-set-status-check-signatures 'recordErr' ./...
+spancheck -checks 'end,set-status,record-error' -ignore-check-signatures 'recordErr' ./...
 ```
 
 ## Problem Statement
@@ -133,9 +129,9 @@ This linter helps developers with steps 1-3.
 
 This linter supports three checks, each documented below. Only the check for `span.End()` is enabled by default. See [Configuration](#configuration) for instructions on enabling the others.
 
-### `span.End()` Check
+### `span.End()`
 
-Enabled by default. Disable with `-disable-end-check`.
+Enabled by default.
 
 Not calling `End` can cause memory leaks and prevents spans from being closed.
 
@@ -151,9 +147,9 @@ func task(ctx context.Context) error {
 }
 ```
 
-### `span.SetStatus(codes.Error, "msg")` Check
+### `span.SetStatus(codes.Error, "msg")`
 
-Disabled by default. Enable with `-enable-set-status-check`.
+Disabled by default. Enable with `-checks 'set-status'`.
 
 Developers should call `SetStatus` on spans. The status attribute is an important, first-class attribute:
 
@@ -177,9 +173,9 @@ func _() error {
 
 OpenTelemetry docs: [Set span status](https://opentelemetry.io/docs/instrumentation/go/manual/#set-span-status).
 
-### `span.RecordError(err)` Check
+### `span.RecordError(err)`
 
-Disabled by default. Enable with `-enable-record-error-check`.
+Disabled by default. Enable with `-checks 'record-error'`.
 
 Calling `RecordError` creates a new exception-type [event (structured log message)](https://opentelemetry.io/docs/concepts/signals/traces/#span-events) on the span. This is recommended to capture the error's stack trace.
 

config.go

@@ -1,93 +1,120 @@
 package spancheck
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"log"
 	"regexp"
+	"slices"
 	"strings"
 )
 
+// Check is a type of check that can be enabled or disabled.
+type Check string
+
+const (
+	// EndCheck if enabled, checks that span.End() is called after span creation and before the function returns.
+	EndCheck Check = "end"
+
+	// SetStatusCheck if enabled, checks that `span.SetStatus(codes.Error, msg)` is called when returning an error.
+	SetStatusCheck = "set-status"
+
+	// RecordErrorCheck if enabled, checks that span.RecordError(err) is called when returning an error.
+	RecordErrorCheck = "record-error"
+)
+
 var (
-	ignoreSetStatusCheckSignatures   = "ignore-set-status-check-signatures"
-	ignoreRecordErrorCheckSignatures = "ignore-record-error-check-signatures"
+	// AllChecks is a list of all checks.
+	AllChecks = []string{
+		string(EndCheck),
+		string(SetStatusCheck),
+		string(RecordErrorCheck),
+	}
+
+	errNoChecks     = errors.New("no checks enabled")
+	errInvalidCheck = errors.New("invalid check")
 )
 
 // Config is a configuration for the spancheck analyzer.
 type Config struct {
 	fs flag.FlagSet
 
-	// EnableAll enables all checks and takes precedence over other fields like
-	// DisableEndCheck. Ignore*CheckSignatures still apply.
-	EnableAll bool
+	// EnabledChecks is a list of checks that are enabled.
+	EnabledChecks []Check
 
-	// DisableEndCheck enables the check for calling span.End().
-	DisableEndCheck bool
+	// ignoreChecksSignatures is a regex that, if matched, disables the
+	// SetStatus and RecordError checks on error.
+	ignoreChecksSignatures *regexp.Regexp
 
-	// EnableSetStatusCheck enables the check for calling span.SetStatus.
-	EnableSetStatusCheck bool
-
-	// ignoreSetStatusCheckSignatures is a regex that, if matched, disables the
-	// SetStatus check for a particular error.
-	ignoreSetStatusCheckSignatures *regexp.Regexp
-
-	// IgnoreSetStatusCheckSignaturesSlice is a slice of strings that are turned into
+	// IgnoreChecksSignaturesSlice is a slice of strings that are turned into
 	// the IgnoreSetStatusCheckSignatures regex.
-	IgnoreSetStatusCheckSignaturesSlice []string
-
-	// EnableRecordErrorCheck enables the check for calling span.RecordError.
-	// By default, this check is disabled.
-	EnableRecordErrorCheck bool
-
-	// ignoreRecordErrorCheckSignatures is a regex that, if matched, disables the
-	// RecordError check for a particular error.
-	ignoreRecordErrorCheckSignatures *regexp.Regexp
-
-	// IgnoreRecordErrorCheckSignaturesSlice is a slice of strings that are turned into
-	// the IgnoreRecordErrorCheckSignatures regex.
-	IgnoreRecordErrorCheckSignaturesSlice []string
+	IgnoreChecksSignaturesSlice []string
 }
 
 // NewConfigFromFlags returns a new Config with default values and flags for CLI usage.
 func NewConfigFromFlags() *Config {
-	cfg := newDefaultConfig()
+	cfg := NewDefaultConfig()
 
 	cfg.fs = flag.FlagSet{}
-	cfg.fs.BoolVar(&cfg.DisableEndCheck, "disable-end-check", cfg.DisableEndCheck, "disable the check for calling span.End() after span creation")
-	cfg.fs.BoolVar(&cfg.EnableAll, "enable-all", cfg.EnableAll, "enable all checks, overriding individual check flags")
-	cfg.fs.BoolVar(&cfg.EnableSetStatusCheck, "enable-set-status-check", cfg.EnableSetStatusCheck, "enable check for a span.SetStatus(codes.Error, msg) call when returning an error")
-	ignoreSetStatusCheckSignatures := flag.String(ignoreSetStatusCheckSignatures, "", "comma-separated list of regex for function signature that disable the span.SetStatus(codes.Error, msg) check on errors")
-	flag.BoolVar(&cfg.EnableRecordErrorCheck, "enable-record-error-check", cfg.EnableRecordErrorCheck, "enable check for a span.RecordError(err) call when returning an error")
-	ignoreRecordErrorCheckSignatures := flag.String(ignoreRecordErrorCheckSignatures, "", "comma-separated list of regex for function signature that disable the span.RecordError(err) check on errors")
 
-	cfg.ignoreSetStatusCheckSignatures = parseSignatures(*ignoreSetStatusCheckSignatures)
-	cfg.ignoreRecordErrorCheckSignatures = parseSignatures(*ignoreRecordErrorCheckSignatures)
+	// 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)
+	}
+	cfg.EnabledChecks = checks
+
+	// 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")
+	cfg.ignoreChecksSignatures = parseSignatures(*ignoreCheckSignatures)
 
 	return cfg
 }
 
-func newDefaultConfig() *Config {
+// NewDefaultConfig returns a new Config with default values.
+func NewDefaultConfig() *Config {
 	return &Config{
-		DisableEndCheck:                       false,
-		EnableAll:                             false,
-		EnableRecordErrorCheck:                false,
-		EnableSetStatusCheck:                  false,
-		ignoreRecordErrorCheckSignatures:      nil,
-		IgnoreRecordErrorCheckSignaturesSlice: nil,
-		ignoreSetStatusCheckSignatures:        nil,
-		IgnoreSetStatusCheckSignaturesSlice:   nil,
+		EnabledChecks: []Check{EndCheck},
 	}
 }
 
 // parseSignatures sets the Ignore*CheckSignatures regex from the string slices.
 func (c *Config) parseSignatures() {
-	if c.ignoreRecordErrorCheckSignatures == nil && len(c.IgnoreRecordErrorCheckSignaturesSlice) > 0 {
-		c.ignoreRecordErrorCheckSignatures = createRegex(c.IgnoreRecordErrorCheckSignaturesSlice)
+	if c.ignoreChecksSignatures == nil && len(c.IgnoreChecksSignaturesSlice) > 0 {
+		c.ignoreChecksSignatures = createRegex(c.IgnoreChecksSignaturesSlice)
 	}
+}
 
-	if c.ignoreSetStatusCheckSignatures == nil && len(c.IgnoreSetStatusCheckSignaturesSlice) > 0 {
-		c.ignoreSetStatusCheckSignatures = createRegex(c.IgnoreSetStatusCheckSignaturesSlice)
+func parseChecks(checksFlag string) ([]Check, error) {
+	if checksFlag == "" {
+		return nil, errNoChecks
 	}
+
+	checks := []Check{}
+	for _, check := range strings.Split(checksFlag, ",") {
+		check = strings.TrimSpace(check)
+		if check == "" {
+			continue
+		}
+
+		if !slices.Contains(AllChecks, check) {
+			return nil, fmt.Errorf("%w: %s", errInvalidCheck, check)
+		}
+
+		checks = append(checks, Check(check))
+	}
+
+	if len(checks) == 0 {
+		return nil, errNoChecks
+	}
+
+	return checks, nil
 }
 
 func parseSignatures(sigFlag string) *regexp.Regexp {

go.work

@@ -5,5 +5,4 @@ use (
 	./testdata/base
 	./testdata/disableerrorchecks
 	./testdata/enableall
-	./testdata/enablechecks
 )

spancheck.go

@@ -8,6 +8,7 @@ import (
 	"go/types"
 	"log"
 	"regexp"
+	"slices"
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
@@ -169,25 +170,25 @@ func runFunc(pass *analysis.Pass, node ast.Node, config *Config) {
 
 	// Check for missing calls.
 	for _, sv := range spanVars {
-		if !config.DisableEndCheck || config.EnableAll {
+		if slices.Contains(config.EnabledChecks, EndCheck) {
 			// 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 config.EnableSetStatusCheck || config.EnableAll {
+		if slices.Contains(config.EnabledChecks, SetStatusCheck) {
 			// Check if there's no SetStatus to the span setting an error.
-			if ret := missingSpanCalls(pass, g, sv, "SetStatus", returnsErr, config.ignoreSetStatusCheckSignatures); ret != nil {
+			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 config.EnableRecordErrorCheck || config.EnableAll {
+		if slices.Contains(config.EnabledChecks, RecordErrorCheck) {
 			// Check if there's no RecordError to the span setting an error.
-			if ret := missingSpanCalls(pass, g, sv, "RecordError", returnsErr, config.ignoreRecordErrorCheckSignatures); ret != nil {
+			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())
 				pass.ReportRangef(ret, "return can be reached without calling %s.RecordError", sv.vr.Name())
 			}

spancheck_test.go

@@ -14,17 +14,19 @@ func Test(t *testing.T) {
 	for dir, config := range map[string]*spancheck.Config{
 		"base": spancheck.NewConfigFromFlags(),
 		"disableerrorchecks": {
-			EnableSetStatusCheck:                  true,
-			IgnoreSetStatusCheckSignaturesSlice:   []string{"telemetry.Record", "recordErr"},
-			EnableRecordErrorCheck:                true,
-			IgnoreRecordErrorCheckSignaturesSlice: []string{"telemetry.Record", "recordErr"},
+			EnabledChecks: []spancheck.Check{
+				spancheck.EndCheck,
+				spancheck.RecordErrorCheck,
+				spancheck.SetStatusCheck,
+			},
+			IgnoreChecksSignaturesSlice: []string{"telemetry.Record", "recordErr"},
 		},
 		"enableall": {
-			EnableAll: true,
-		},
-		"enablechecks": {
-			EnableSetStatusCheck:   true,
-			EnableRecordErrorCheck: true,
+			EnabledChecks: []spancheck.Check{
+				spancheck.EndCheck,
+				spancheck.RecordErrorCheck,
+				spancheck.SetStatusCheck,
+			},
 		},
 	} {
 		dir := dir