build: add new mustcallcheck analyzer and fix instances (#2886)

tstirrat15 · web-flow · commit 7f9bd2706935 · 2026-02-06T19:56:22.000Z
diff --git a/internal/datasets/subjectsetbytype.go b/internal/datasets/subjectsetbytype.go
@@ -58,7 +58,10 @@ func (s *SubjectByTypeSet) ForEachType(handler func(rr *core.RelationReference,
 func (s *SubjectByTypeSet) Map(mapper func(rr *core.RelationReference) (*core.RelationReference, error)) (*SubjectByTypeSet, error) {
 	mapped := NewSubjectByTypeSet()
 	for key, subjectset := range s.byType {
-		ns, rel := tuple.MustSplitRelRef(key)
+		ns, rel, err := tuple.SplitRelRef(key)
+		if err != nil {
+			return nil, err
+		}
 		updatedType, err := mapper(&core.RelationReference{
 			Namespace: ns,
 			Relation:  rel,
diff --git a/internal/datastore/crdb/schema/indexutil.go b/internal/datastore/crdb/schema/indexutil.go
@@ -27,10 +27,11 @@ func mustInit() {
 	}
 }
 
+var indexColumnRegex = regexp.MustCompile(`\(([^)]+)\)`)
+
 func parseIndexColumns(columnsSQL string) ([]string, error) {
 	// Match columns within parentheses, handling both PRIMARY KEY and table_name formats
-	re := regexp.MustCompile(`\(([^)]+)\)`)
-	matches := re.FindStringSubmatch(columnsSQL)
+	matches := indexColumnRegex.FindStringSubmatch(columnsSQL)
 	if len(matches) < 2 {
 		return nil, fmt.Errorf("no columns found in parentheses in SQL: %s", columnsSQL)
 	}
diff --git a/internal/datastore/memdb/readwrite.go b/internal/datastore/memdb/readwrite.go
@@ -103,7 +103,15 @@ func (rwt *memdbReadWriteTx) write(tx *memdb.Txn, mutations ...tuple.Relationshi
 				if err != nil {
 					return err
 				}
-				if tuple.MustString(rt) == tuple.MustString(mutation.Relationship) {
+				existingRelationshipString, err := tuple.String(rt)
+				if err != nil {
+					return err
+				}
+				mutationRelationshipString, err := tuple.String(mutation.Relationship)
+				if err != nil {
+					return err
+				}
+				if existingRelationshipString == mutationRelationshipString {
 					continue
 				}
 			}
diff --git a/magefiles/lint.go b/magefiles/lint.go
@@ -93,6 +93,8 @@ func (Lint) Analyzers() error {
 		"-exprstatementcheck.disallowed-expr-statement-types=*github.com/rs/zerolog.Event:MarshalZerologObject:missing Send or Msg on zerolog log Event",
 		"-paniccheck",
 		"-paniccheck.skip-files=_test,zz_,migrations/index.go",
+		"-mustcallcheck",
+		"-mustcallcheck.skip-files=_test,zz_",
 		"-zerologmarshalcheck",
 		"-zerologmarshalcheck.skip-files=_test,zz_",
 		"-protomarshalcheck",
diff --git a/pkg/cmd/serve.go b/pkg/cmd/serve.go
@@ -135,13 +135,22 @@ func RegisterServeFlags(cmd *cobra.Command, config *server.Config) error {
 		return fmt.Errorf("failed to mark flag as hidden: %w", err)
 	}
 	namespaceCacheFlags.DurationVar(&config.SchemaWatchHeartbeat, "datastore-schema-watch-heartbeat", 1*time.Second, "heartbeat time on the schema watch in the datastore (if supported). 0 means to default to the datastore's minimum.")
-	server.MustRegisterCacheFlags(namespaceCacheFlags, "ns-cache", "schema", &config.NamespaceCacheConfig, namespaceCacheDefaults)
+	err := server.RegisterCacheFlags(namespaceCacheFlags, "ns-cache", "schema", &config.NamespaceCacheConfig, namespaceCacheDefaults)
+	if err != nil {
+		return fmt.Errorf("could not register namespace cache flags: %w", err)
+	}
 
 	dispatchFlags := nfs.FlagSet(BoldBlue("Dispatch"))
 	// Flags for configuring the dispatch server
 	util.RegisterGRPCServerFlags(dispatchFlags, &config.DispatchServer, "dispatch-cluster", "dispatch", ":50053", false)
-	server.MustRegisterCacheFlags(dispatchFlags, "dispatch-cache", "dispatch calls this server makes to other servers", &config.DispatchCacheConfig, dispatchCacheDefaults)
-	server.MustRegisterCacheFlags(dispatchFlags, "dispatch-cluster-cache", "dispatch calls this server receives from other servers", &config.ClusterDispatchCacheConfig, dispatchClusterCacheDefaults)
+	err = server.RegisterCacheFlags(dispatchFlags, "dispatch-cache", "dispatch calls this server makes to other servers", &config.DispatchCacheConfig, dispatchCacheDefaults)
+	if err != nil {
+		return fmt.Errorf("could not register dispatch cache flags: %w", err)
+	}
+	err = server.RegisterCacheFlags(dispatchFlags, "dispatch-cluster-cache", "dispatch calls this server receives from other servers", &config.ClusterDispatchCacheConfig, dispatchClusterCacheDefaults)
+	if err != nil {
+		return fmt.Errorf("could not register dispatch cluster cache flags: %w", err)
+	}
 
 	// Flags for configuring dispatch requests
 	dispatchFlags.Uint16Var(&config.DispatchChunkSize, "dispatch-chunk-size", 100, "maximum number of object IDs in a dispatched request")
@@ -188,7 +197,10 @@ func RegisterServeFlags(cmd *cobra.Command, config *server.Config) error {
 		return fmt.Errorf("failed to mark flag as deprecated: %w", err)
 	}
 
-	server.MustRegisterCacheFlags(experimentalFlags, "lookup-resources-chunk-cache", "LookupResources3 chunks", &config.LR3ResourceChunkCacheConfig, lr3ChunkCacheDefaults)
+	err = server.RegisterCacheFlags(experimentalFlags, "lookup-resources-chunk-cache", "LookupResources3 chunks", &config.LR3ResourceChunkCacheConfig, lr3ChunkCacheDefaults)
+	if err != nil {
+		return fmt.Errorf("could not register lookup resources chunk cache flags: %w", err)
+	}
 
 	tracingFlags := nfs.FlagSet(BoldBlue("Tracing"))
 	// Flags for tracing
diff --git a/pkg/cmd/server/cacheconfig.go b/pkg/cmd/server/cacheconfig.go
@@ -137,9 +137,8 @@ func parsePercent(str string, freeMem uint64) (uint64, error) {
 	return freeMem / 100 * parsedPercent, nil
 }
 
-// MustRegisterCacheFlags registers flags used to configure SpiceDB's various
-// caches.
-func MustRegisterCacheFlags(flags *pflag.FlagSet, flagPrefix, flagDescription string, config, defaults *CacheConfig) {
+// RegisterCacheFlags registers flags used to configure SpiceDB's various caches.
+func RegisterCacheFlags(flags *pflag.FlagSet, flagPrefix, flagDescription string, config, defaults *CacheConfig) error {
 	config.Name = defaults.Name
 	flagPrefix = cmp.Or(flagPrefix, "cache")
 	flags.StringVar(&config.MaxCost, flagPrefix+"-max-cost", defaults.MaxCost, "upper bound (in bytes or as a percent of available memory) of the cache for "+flagDescription)
@@ -150,6 +149,7 @@ func MustRegisterCacheFlags(flags *pflag.FlagSet, flagPrefix, flagDescription st
 	// Hidden flags.
 	flags.StringVar(&config.CacheKindForTesting, flagPrefix+"-kind-for-testing", defaults.CacheKindForTesting, "choose a different kind of cache, for testing")
 	if err := flags.MarkHidden(flagPrefix + "-kind-for-testing"); err != nil {
-		panic(err)
+		return err
 	}
+	return nil
 }
diff --git a/pkg/composableschemadsl/compiler/translator.go b/pkg/composableschemadsl/compiler/translator.go
@@ -610,7 +610,7 @@ func translateExpressionOperationDirect(tctx *translationContext, expressionOpNo
 				return nil, err
 			}
 
-			return namespace.MustFunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelation), nil
+			return namespace.FunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelation)
 		}
 
 		return namespace.TupleToUserset(tuplesetRelation, usersetRelation), nil
diff --git a/pkg/development/devcontext.go b/pkg/development/devcontext.go
@@ -148,12 +148,17 @@ func newDevContextWithDatastore(ctx context.Context, requestContext *devinterfac
 		RelationshipChunkCache: nil, // Disable caching for devcontext
 	}
 
+	dispatcher, err := graph.NewLocalOnlyDispatcher(params)
+	if err != nil {
+		return nil, nil, err
+	}
+
 	return &DevContext{
 		Ctx:            ctx,
 		Datastore:      ds,
 		CompiledSchema: compiled,
 		Revision:       currentRevision,
-		Dispatcher:     graph.MustNewLocalOnlyDispatcher(params),
+		Dispatcher:     dispatcher,
 	}, nil, nil
 }
 
diff --git a/pkg/namespace/builder.go b/pkg/namespace/builder.go
@@ -307,7 +307,7 @@ func TupleToUserset(tuplesetRelation, usersetRelation string) *core.SetOperation
 // MustFunctionedTupleToUserset creates a child which first loads all tuples with the specific relation,
 // and then applies the function to all children on the usersets found by following a relation on those loaded
 // tuples.
-func MustFunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelation string) *core.SetOperation_Child {
+func FunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelation string) (*core.SetOperation_Child, error) {
 	function := core.FunctionedTupleToUserset_FUNCTION_ANY
 
 	switch functionName {
@@ -318,7 +318,7 @@ func MustFunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelatio
 		function = core.FunctionedTupleToUserset_FUNCTION_ALL
 
 	default:
-		panic(spiceerrors.MustBugf("unknown function name: %s", functionName))
+		return nil, spiceerrors.MustBugf("unknown function name: %s", functionName)
 	}
 
 	return &core.SetOperation_Child{
@@ -334,7 +334,17 @@ func MustFunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelatio
 				},
 			},
 		},
+	}, nil
+}
+
+// MustFunctionedTupleToUserset is a wrapper around FunctionedTupleToUserset that panics if an error
+// is encountered.
+func MustFunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelation string) *core.SetOperation_Child {
+	operation, err := FunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelation)
+	if err != nil {
+		panic(err)
 	}
+	return operation
 }
 
 // Rewrite wraps a rewrite as a set operation child of another rewrite.
diff --git a/pkg/schema/reachabilitygraph.go b/pkg/schema/reachabilitygraph.go
@@ -44,7 +44,10 @@ func (rg *DefinitionReachability) RelationsEncounteredForResource(
 
 	relationRefs := make([]*core.RelationReference, 0, len(relationNames))
 	for _, relationName := range relationNames {
-		namespace, relation := tuple.MustSplitRelRef(relationName)
+		namespace, relation, err := tuple.SplitRelRef(relationName)
+		if err != nil {
+			return nil, err
+		}
 		relationRefs = append(relationRefs, &core.RelationReference{
 			Namespace: namespace,
 			Relation:  relation,
@@ -113,7 +116,10 @@ func (rg *DefinitionReachability) RelationsEncounteredForSubject(
 
 	relationRefs := make([]*core.RelationReference, 0, allRelationNames.Len())
 	for _, relationName := range allRelationNames.AsSlice() {
-		namespace, relation := tuple.MustSplitRelRef(relationName)
+		namespace, relation, err := tuple.SplitRelRef(relationName)
+		if err != nil {
+			return nil, err
+		}
 		relationRefs = append(relationRefs, &core.RelationReference{
 			Namespace: namespace,
 			Relation:  relation,
diff --git a/pkg/schemadsl/compiler/translator.go b/pkg/schemadsl/compiler/translator.go
@@ -600,7 +600,7 @@ func translateExpressionOperationDirect(tctx *translationContext, expressionOpNo
 				return nil, err
 			}
 
-			return namespace.MustFunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelation), nil
+			return namespace.FunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelation)
 		}
 
 		return namespace.TupleToUserset(tuplesetRelation, usersetRelation), nil
diff --git a/pkg/tuple/strings.go b/pkg/tuple/strings.go
@@ -1,6 +1,7 @@
 package tuple
 
 import (
+	"errors"
 	"sort"
 	"strings"
 	"time"
@@ -21,12 +22,20 @@ func JoinRelRef(namespace, relation string) string { return namespace + "#" + re
 // MustSplitRelRef splits a string produced by `JoinRelRef()` and panics if
 // it fails.
 func MustSplitRelRef(relRef string) (namespace, relation string) {
+	namespace, relation, err := SplitRelRef(relRef)
+	if err != nil {
+		panic(err)
+	}
+	return namespace, relation
+}
+
+func SplitRelRef(relRef string) (string, string, error) {
 	var ok bool
-	namespace, relation, ok = strings.Cut(relRef, "#")
+	namespace, relation, ok := strings.Cut(relRef, "#")
 	if !ok {
-		panic("improperly formatted relation reference")
+		return "", "", errors.New("improperly formatted relation reference")
 	}
-	return namespace, relation
+	return namespace, relation, nil
 }
 
 // StringRR converts a RR object to a string.
diff --git a/tools/analyzers/cmd/analyzers/main.go b/tools/analyzers/cmd/analyzers/main.go
@@ -7,6 +7,7 @@ import (
 	"github.com/authzed/spicedb/tools/analyzers/exprstatementcheck"
 	"github.com/authzed/spicedb/tools/analyzers/iferrafterrowclosecheck"
 	"github.com/authzed/spicedb/tools/analyzers/lendowncastcheck"
+	"github.com/authzed/spicedb/tools/analyzers/mustcallcheck"
 	"github.com/authzed/spicedb/tools/analyzers/mutexcheck"
 	"github.com/authzed/spicedb/tools/analyzers/nilvaluecheck"
 	"github.com/authzed/spicedb/tools/analyzers/paniccheck"
@@ -23,6 +24,7 @@ func main() {
 		closeafterusagecheck.Analyzer(),
 		iferrafterrowclosecheck.Analyzer(),
 		paniccheck.Analyzer(),
+		mustcallcheck.Analyzer(),
 		lendowncastcheck.Analyzer(),
 		protomarshalcheck.Analyzer(),
 		zerologmarshalcheck.Analyzer(),
diff --git a/tools/analyzers/mustcallcheck/mustcallcheck.go b/tools/analyzers/mustcallcheck/mustcallcheck.go
diff --git a/tools/analyzers/mustcallcheck/mustcallcheck_test.go b/tools/analyzers/mustcallcheck/mustcallcheck_test.go
diff --git a/tools/analyzers/mustcallcheck/testdata/src/badmustcalls/badmustcalls.go b/tools/analyzers/mustcallcheck/testdata/src/badmustcalls/badmustcalls.go
diff --git a/tools/analyzers/mustcallcheck/testdata/src/cobrautiltest/cobrautiltest.go b/tools/analyzers/mustcallcheck/testdata/src/cobrautiltest/cobrautiltest.go
diff --git a/tools/analyzers/mustcallcheck/testdata/src/goodmustcalls/goodmustcalls.go b/tools/analyzers/mustcallcheck/testdata/src/goodmustcalls/goodmustcalls.go

Original file line number	Diff line number	Diff line change
`@@ -27,10 +27,11 @@ func mustInit() {`
`27`	`27`	`}`
`28`	`28`	`}`
`29`	`29`
	`30`	+var indexColumnRegex = regexp.MustCompile(`\(([^)]+)\)`)
	`31`	`+`
`30`	`32`	`func parseIndexColumns(columnsSQL string) ([]string, error) {`
`31`	`33`	`// Match columns within parentheses, handling both PRIMARY KEY and table_name formats`
`32`		- re := regexp.MustCompile(`\(([^)]+)\)`)
`33`		`- matches := re.FindStringSubmatch(columnsSQL)`
	`34`	`+ matches := indexColumnRegex.FindStringSubmatch(columnsSQL)`
`34`	`35`	`if len(matches) < 2 {`
`35`	`36`	`return nil, fmt.Errorf("no columns found in parentheses in SQL: %s", columnsSQL)`
`36`	`37`	`}`
Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,15 @@ func (rwt memdbReadWriteTx) write(tx memdb.Txn, mutations ...tuple.Relationshi`
`103`	`103`	`if err != nil {`
`104`	`104`	`return err`
`105`	`105`	`}`
`106`		`- if tuple.MustString(rt) == tuple.MustString(mutation.Relationship) {`
	`106`	`+ existingRelationshipString, err := tuple.String(rt)`
	`107`	`+ if err != nil {`
	`108`	`+ return err`
	`109`	`+ }`
	`110`	`+ mutationRelationshipString, err := tuple.String(mutation.Relationship)`
	`111`	`+ if err != nil {`
	`112`	`+ return err`
	`113`	`+ }`
	`114`	`+ if existingRelationshipString == mutationRelationshipString {`
`107`	`115`	`continue`
`108`	`116`	`}`
`109`	`117`	`}`
Original file line number	Diff line number	Diff line change
`@@ -137,9 +137,8 @@ func parsePercent(str string, freeMem uint64) (uint64, error) {`
`137`	`137`	`return freeMem / 100 * parsedPercent, nil`
`138`	`138`	`}`
`139`	`139`
`140`		`-// MustRegisterCacheFlags registers flags used to configure SpiceDB's various`
`141`		`-// caches.`
`142`		`-func MustRegisterCacheFlags(flags pflag.FlagSet, flagPrefix, flagDescription string, config, defaults CacheConfig) {`
	`140`	`+// RegisterCacheFlags registers flags used to configure SpiceDB's various caches.`
	`141`	`+func RegisterCacheFlags(flags pflag.FlagSet, flagPrefix, flagDescription string, config, defaults CacheConfig) error {`
`143`	`142`	`config.Name = defaults.Name`
`144`	`143`	`flagPrefix = cmp.Or(flagPrefix, "cache")`
`145`	`144`	`flags.StringVar(&config.MaxCost, flagPrefix+"-max-cost", defaults.MaxCost, "upper bound (in bytes or as a percent of available memory) of the cache for "+flagDescription)`
`@@ -150,6 +149,7 @@ func MustRegisterCacheFlags(flags *pflag.FlagSet, flagPrefix, flagDescription st`
`150`	`149`	`// Hidden flags.`
`151`	`150`	`flags.StringVar(&config.CacheKindForTesting, flagPrefix+"-kind-for-testing", defaults.CacheKindForTesting, "choose a different kind of cache, for testing")`
`152`	`151`	`if err := flags.MarkHidden(flagPrefix + "-kind-for-testing"); err != nil {`
`153`		`- panic(err)`
	`152`	`+ return err`
`154`	`153`	`}`
	`154`	`+ return nil`
`155`	`155`	`}`
Original file line number	Diff line number	Diff line change
`@@ -610,7 +610,7 @@ func translateExpressionOperationDirect(tctx *translationContext, expressionOpNo`
`610`	`610`	`return nil, err`
`611`	`611`	`}`
`612`	`612`
`613`		`- return namespace.MustFunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelation), nil`
	`613`	`+ return namespace.FunctionedTupleToUserset(tuplesetRelation, functionName, usersetRelation)`
`614`	`614`	`}`
`615`	`615`
`616`	`616`	`return namespace.TupleToUserset(tuplesetRelation, usersetRelation), nil`