chore: add linter enforcing static WithSourceErrorf format strings

tools/analyzers/cmd/analyzers/main.go

@@ -17,6 +17,7 @@ import (
 	"github.com/authzed/spicedb/tools/analyzers/paniccheck"
 	"github.com/authzed/spicedb/tools/analyzers/protomarshalcheck"
 	"github.com/authzed/spicedb/tools/analyzers/singleflightcheck"
+	"github.com/authzed/spicedb/tools/analyzers/staticformatcheck"
 	"github.com/authzed/spicedb/tools/analyzers/telemetryconvcheck"
 	"github.com/authzed/spicedb/tools/analyzers/zerologmarshalcheck"
 )
@@ -34,6 +35,7 @@ func main() {
 		protomarshalcheck.Analyzer(),
 		zerologmarshalcheck.Analyzer(),
 		singleflightcheck.Analyzer(),
+		staticformatcheck.Analyzer(),
 		telemetryconvcheck.Analyzer(),
 	}
 

tools/analyzers/staticformatcheck/staticformatcheck.go

@@ -0,0 +1,84 @@
+package staticformatcheck
+
+import (
+	"go/ast"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+// checkedFuncNames holds the names of the format-style functions whose format
+// string argument must be a compile-time constant. The format string is
+// expected to be the last argument before the variadic format arguments.
+//
+// Matching is intentionally by name only, not scoped to a type or package, so
+// names here must be distinctive enough not to collide with unrelated
+// same-named methods. WithSourceErrorf is unique to the schema compiler's
+// dslNode; a generic name like Errorf would over-match fmt.Errorf,
+// testing.T.Errorf, and others and cannot be added without type scoping.
+var checkedFuncNames = map[string]struct{}{
+	"WithSourceErrorf": {},
+}
+
+func Analyzer() *analysis.Analyzer {
+	return &analysis.Analyzer{
+		Name:     "staticformatcheck",
+		Doc:      "reports calls to WithSourceErrorf whose format string argument is not a compile-time constant",
+		Run:      run,
+		Requires: []*analysis.Analyzer{inspect.Analyzer},
+	}
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+	nodeFilter := []ast.Node{(*ast.CallExpr)(nil)}
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
+		call := n.(*ast.CallExpr)
+
+		selectorExpr, ok := call.Fun.(*ast.SelectorExpr)
+		if !ok {
+			return
+		}
+
+		if _, ok := checkedFuncNames[selectorExpr.Sel.Name]; !ok {
+			return
+		}
+
+		// Ensure the callee is a function and derive the format string index
+		// from the call site's signature: the format string is the parameter
+		// immediately before the variadic format arguments. For method
+		// expressions the receiver appears as the first parameter, which shifts
+		// the format string index accordingly.
+		if _, ok := pass.TypesInfo.Uses[selectorExpr.Sel].(*types.Func); !ok {
+			return
+		}
+
+		signature, ok := pass.TypesInfo.TypeOf(call.Fun).(*types.Signature)
+		if !ok || !signature.Variadic() {
+			return
+		}
+
+		formatArgIndex := signature.Params().Len() - 2
+		if formatArgIndex < 0 || len(call.Args) <= formatArgIndex {
+			return
+		}
+
+		formatParamType, ok := signature.Params().At(formatArgIndex).Type().(*types.Basic)
+		if !ok || formatParamType.Kind() != types.String {
+			return
+		}
+
+		formatArg := call.Args[formatArgIndex]
+		if tv, ok := pass.TypesInfo.Types[formatArg]; ok && tv.Value != nil {
+			// The format string is a compile-time constant.
+			return
+		}
+
+		pass.Reportf(formatArg.Pos(), "format string argument to `%s` must be a static string", selectorExpr.Sel.Name)
+	})
+
+	return nil, nil
+}

tools/analyzers/staticformatcheck/staticformatcheck_test.go

@@ -0,0 +1,15 @@
+package staticformatcheck
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestAnalyzer(t *testing.T) {
+	analyzer := Analyzer()
+
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, analyzer, "disallowedformat")
+	analysistest.Run(t, testdata, analyzer, "validformat")
+}

tools/analyzers/staticformatcheck/testdata/src/disallowedformat/disallowedformat.go

@@ -0,0 +1,28 @@
+package disallowedformat
+
+// NOTE: this def is just to get us to a `WithSourceErrorf` call without
+// needing to import things, because importing in these tests is difficult.
+type someNode struct{}
+
+func (n *someNode) WithSourceErrorf(sourceCode string, message string, args ...any) error {
+	return nil
+}
+
+func dynamicFormat(n *someNode, name string) error {
+	message := "found dynamic message: " + name
+	return n.WithSourceErrorf(name, message) // want "format string argument to `WithSourceErrorf` must be a static string"
+}
+
+func dynamicConcatFormat(n *someNode, name string) error {
+	return n.WithSourceErrorf(name, "found dynamic message: "+name) // want "format string argument to `WithSourceErrorf` must be a static string"
+}
+
+func dynamicNestedFormat(n *someNode, name string) []error {
+	message := "found dynamic message: " + name
+	return append([]error{}, n.WithSourceErrorf(name, message)) // want "format string argument to `WithSourceErrorf` must be a static string"
+}
+
+func dynamicMethodExprFormat(n *someNode, name string) error {
+	message := "found dynamic message: " + name
+	return (*someNode).WithSourceErrorf(n, name, message) // want "format string argument to `WithSourceErrorf` must be a static string"
+}

tools/analyzers/staticformatcheck/testdata/src/validformat/validformat.go

@@ -0,0 +1,39 @@
+package validformat
+
+// NOTE: this def is just to get us to a `WithSourceErrorf` call without
+// needing to import things, because importing in these tests is difficult.
+type someNode struct{}
+
+func (n *someNode) WithSourceErrorf(sourceCode string, message string, args ...any) error {
+	return nil
+}
+
+// unrelated has a non-format signature and must not be checked, even though
+// its method shares the checked name.
+type unrelated struct{}
+
+func (u *unrelated) WithSourceErrorf(message string) error {
+	return nil
+}
+
+const errTemplate = "found duplicate name: %s"
+
+func literalFormat(n *someNode, name string) error {
+	return n.WithSourceErrorf(name, "found duplicate name: %s", name)
+}
+
+func constantFormat(n *someNode, name string) error {
+	return n.WithSourceErrorf(name, errTemplate, name)
+}
+
+func literalConcatFormat(n *someNode, name string) error {
+	return n.WithSourceErrorf(name, "found duplicate "+"name: %s", name)
+}
+
+func methodExprFormat(n *someNode, name string) error {
+	return (*someNode).WithSourceErrorf(n, name, "found duplicate name: %s", name)
+}
+
+func unrelatedSignature(u *unrelated, name string) error {
+	return u.WithSourceErrorf("found dynamic message: " + name)
+}