Lint missing diagnostic formatting arguments

tools/customlint/diagnosticargs.go

@@ -0,0 +1,140 @@
+package customlint
+
+import (
+	"go/ast"
+	"go/constant"
+	"go/token"
+	"go/types"
+	"regexp"
+	"strconv"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+var diagnosticArgsAnalyzer = &analysis.Analyzer{
+	Name:      "diagnosticargs",
+	Doc:       "checks diagnostic messages have enough formatting arguments",
+	Requires:  []*analysis.Analyzer{inspect.Analyzer},
+	FactTypes: []analysis.Fact{new(diagnosticArgumentCount)},
+	Run:       runDiagnosticArgs,
+}
+
+type diagnosticArgumentCount struct {
+	Count int
+}
+
+func (*diagnosticArgumentCount) AFact() {}
+
+var diagnosticPlaceholder = regexp.MustCompile(`\{(\d+)\}`)
+
+func isDiagnosticMessage(t types.Type) bool {
+	pointer, ok := types.Unalias(t).(*types.Pointer)
+	if !ok {
+		return false
+	}
+	named, ok := types.Unalias(pointer.Elem()).(*types.Named)
+	return ok && named.Obj().Name() == "Message" && named.Obj().Pkg() != nil &&
+		strings.HasSuffix(named.Obj().Pkg().Path(), "/internal/diagnostics")
+}
+
+func runDiagnosticArgs(pass *analysis.Pass) (any, error) {
+	counts := make(map[types.Object]diagnosticArgumentCount)
+	if strings.HasSuffix(pass.Pkg.Path(), "/internal/diagnostics") {
+		for _, file := range pass.Files {
+			for _, decl := range file.Decls {
+				gen, ok := decl.(*ast.GenDecl)
+				if !ok || gen.Tok != token.VAR {
+					continue
+				}
+				for _, spec := range gen.Specs {
+					value := spec.(*ast.ValueSpec)
+					for i, expr := range value.Values {
+						unary, ok := expr.(*ast.UnaryExpr)
+						if !ok || unary.Op != token.AND || !isDiagnosticMessage(pass.TypesInfo.TypeOf(expr)) {
+							continue
+						}
+						literal, ok := unary.X.(*ast.CompositeLit)
+						if !ok {
+							continue
+						}
+						for _, element := range literal.Elts {
+							field, ok := element.(*ast.KeyValueExpr)
+							if !ok {
+								continue
+							}
+							key, ok := field.Key.(*ast.Ident)
+							if !ok || key.Name != "text" {
+								continue
+							}
+							text := pass.TypesInfo.Types[field.Value].Value
+							if text == nil || text.Kind() != constant.String {
+								continue
+							}
+							count := diagnosticArgumentCount{}
+							for _, match := range diagnosticPlaceholder.FindAllStringSubmatch(constant.StringVal(text), -1) {
+								index, err := strconv.Atoi(match[1])
+								if err == nil {
+									count.Count = max(count.Count, index+1)
+								}
+							}
+							object := pass.TypesInfo.Defs[value.Names[i]]
+							counts[object] = count
+							pass.ExportObjectFact(object, &count)
+						}
+					}
+				}
+			}
+		}
+	}
+
+	check := func(call *ast.CallExpr, message ast.Expr, actual int) {
+		var object types.Object
+		switch expr := ast.Unparen(message).(type) {
+		case *ast.Ident:
+			object = pass.TypesInfo.Uses[expr]
+		case *ast.SelectorExpr:
+			object = pass.TypesInfo.Uses[expr.Sel]
+		}
+		if object == nil {
+			return
+		}
+		count, ok := counts[object]
+		if !ok && !pass.ImportObjectFact(object, &count) {
+			return
+		}
+		if actual < count.Count {
+			pass.ReportRangef(call, "diagnostic %s requires at least %d formatting arguments, got %d", object.Name(), count.Count, actual)
+		}
+	}
+
+	in := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	in.Preorder([]ast.Node{(*ast.CallExpr)(nil)}, func(node ast.Node) {
+		call := node.(*ast.CallExpr)
+		if call.Ellipsis.IsValid() {
+			return
+		}
+		signature, ok := pass.TypesInfo.TypeOf(call.Fun).(*types.Signature)
+		if !ok || !signature.Variadic() {
+			return
+		}
+		fixed := signature.Params().Len() - 1
+		if len(call.Args) < fixed {
+			return
+		}
+		if fixed > 0 && isDiagnosticMessage(signature.Params().At(fixed-1).Type()) {
+			check(call, call.Args[fixed-1], len(call.Args)-fixed)
+			return
+		}
+		if selector, ok := call.Fun.(*ast.SelectorExpr); ok {
+			selection := pass.TypesInfo.Selections[selector]
+			if selection != nil && selection.Kind() == types.MethodVal &&
+				isDiagnosticMessage(selection.Recv()) && selector.Sel.Name == "Localize" {
+				check(call, selector.X, len(call.Args)-fixed)
+			}
+		}
+	})
+	return nil, nil
+}

tools/customlint/plugin.go

@@ -19,6 +19,7 @@ func (f *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) {
 		checkChildrenAnalyzer,
 		cleanupAnalyzer,
 		caseBodyAnalyzer,
+		diagnosticArgsAnalyzer,
 		forbidParentAccessAnalyzer,
 		protectedFieldAnalyzer,
 		shadowAnalyzer,

tools/customlint/testdata/diagnosticargs/diagnosticargs.go

@@ -0,0 +1,36 @@
+package diagnosticargs
+
+import d "testdata/internal/diagnostics"
+
+func diagnostic(message *d.Message, args ...any) {}
+
+func nodeDiagnostic(node int, message *d.Message, args ...any) {}
+
+func optionDiagnostic(message *d.Message, option string, args ...any) {
+	diagnostic(message, append([]any{option}, args...)...)
+}
+
+func calls(args []any, message *d.Message) {
+	diagnostic(d.Circularity)
+	diagnostic(d.Circularity, "a -> b -> a")
+	diagnostic(d.Plain)
+	diagnostic(d.Plain, "extra")
+	diagnostic(d.Multiple, "one")
+	diagnostic(d.Multiple, "one", "two")
+	diagnostic(d.Multiple, "one", "two", "three")
+	diagnostic(d.Sparse, "one", "two")
+	diagnostic(d.Sparse, "one", "two", "three")
+	nodeDiagnostic(0, d.Circularity)
+	nodeDiagnostic(0, d.Circularity, "cycle")
+	optionDiagnostic(d.Circularity, "cycle")
+	d.Circularity.Localize()
+	d.Circularity.Localize("cycle")
+	(*d.Message).Localize(d.Circularity)
+	(*d.Message).Localize(d.Circularity, "cycle")
+	diagnostic(d.Circularity, args...)
+	diagnostic(message)
+	local := d.Circularity
+	diagnostic(local)
+	unrelated := func(value any, args ...any) {}
+	unrelated(d.Circularity)
+}

tools/customlint/testdata/diagnosticargs/diagnosticargs.go.golden

@@ -0,0 +1,49 @@
+	package diagnosticargs
+
+	import d "testdata/internal/diagnostics"
+
+	func diagnostic(message *d.Message, args ...any) {}
+
+	func nodeDiagnostic(node int, message *d.Message, args ...any) {}
+
+	func optionDiagnostic(message *d.Message, option string, args ...any) {
+		diagnostic(message, append([]any{option}, args...)...)
+	}
+
+	func calls(args []any, message *d.Message) {
+		diagnostic(d.Circularity)
+		~~~~~~~~~~~~~~~~~~~~~~~~~
+!!! diagnosticargs: diagnostic Circularity requires at least 1 formatting arguments, got 0
+		diagnostic(d.Circularity, "a -> b -> a")
+		diagnostic(d.Plain)
+		diagnostic(d.Plain, "extra")
+		diagnostic(d.Multiple, "one")
+		~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+!!! diagnosticargs: diagnostic Multiple requires at least 2 formatting arguments, got 1
+		diagnostic(d.Multiple, "one", "two")
+		diagnostic(d.Multiple, "one", "two", "three")
+		diagnostic(d.Sparse, "one", "two")
+		~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+!!! diagnosticargs: diagnostic Sparse requires at least 3 formatting arguments, got 2
+		diagnostic(d.Sparse, "one", "two", "three")
+		nodeDiagnostic(0, d.Circularity)
+		~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+!!! diagnosticargs: diagnostic Circularity requires at least 1 formatting arguments, got 0
+		nodeDiagnostic(0, d.Circularity, "cycle")
+		optionDiagnostic(d.Circularity, "cycle")
+		d.Circularity.Localize()
+		~~~~~~~~~~~~~~~~~~~~~~~~
+!!! diagnosticargs: diagnostic Circularity requires at least 1 formatting arguments, got 0
+		d.Circularity.Localize("cycle")
+		(*d.Message).Localize(d.Circularity)
+		~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+!!! diagnosticargs: diagnostic Circularity requires at least 1 formatting arguments, got 0
+		(*d.Message).Localize(d.Circularity, "cycle")
+		diagnostic(d.Circularity, args...)
+		diagnostic(message)
+		local := d.Circularity
+		diagnostic(local)
+		unrelated := func(value any, args ...any) {}
+		unrelated(d.Circularity)
+	}
+

tools/customlint/testdata/internal/diagnostics/diagnostics.go

@@ -0,0 +1,14 @@
+package diagnostics
+
+type Message struct {
+	text string
+}
+
+var Circularity = &Message{text: "Circularity detected while resolving configuration: {0}"}
+var Plain = &Message{text: "No placeholders"}
+var Multiple = &Message{text: "{0} and {1} and {0}"}
+var Sparse = &Message{text: "{2}"}
+
+func (m *Message) Localize(args ...any) string {
+	return m.text
+}

tools/customlint/testdata/internal/diagnostics/diagnostics.go.golden

@@ -0,0 +1,15 @@
+	package diagnostics
+
+	type Message struct {
+		text string
+	}
+
+	var Circularity = &Message{text: "Circularity detected while resolving configuration: {0}"}
+	var Plain = &Message{text: "No placeholders"}
+	var Multiple = &Message{text: "{0} and {1} and {0}"}
+	var Sparse = &Message{text: "{2}"}
+
+	func (m *Message) Localize(args ...any) string {
+		return m.text
+	}
+

tsc/internal/checker/checker.go

@@ -9478,11 +9478,12 @@ func (c *Checker) checkTypeArguments(signature *Signature, typeArgumentNodes []*
 				errorNode = typeArgumentNodes[i]
 			}
 			var diags []*ast.Diagnostic
-			if !c.checkTypeAssignableToEx(typeArgument, c.getTypeWithThisArgument(c.instantiateType(constraint, mapper), typeArgument, false), errorNode, typeArgumentHeadMessage, &diags) {
+			instantiatedConstraint := c.getTypeWithThisArgument(c.instantiateType(constraint, mapper), typeArgument, false)
+			if !c.checkTypeAssignableToEx(typeArgument, instantiatedConstraint, errorNode, typeArgumentHeadMessage, &diags) {
 				if len(diags) != 0 {
 					diagnostic := diags[0]
 					if headMessage != nil {
-						diagnostic = ast.NewDiagnosticChain(diagnostic, diagnostics.Type_0_does_not_satisfy_the_constraint_1)
+						diagnostic = ast.NewDiagnosticChain(diagnostic, diagnostics.Type_0_does_not_satisfy_the_constraint_1, c.typeToString(typeArgument, nil), c.typeToString(instantiatedConstraint, nil))
 					}
 					c.addDiagnostic(diagnostic)
 				}

tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.errors.txt

@@ -0,0 +1,25 @@
+typeArgumentConstraintDiagnostic.ts(2,13): error TS2344: Type 'number' does not satisfy the constraint 'string'.
+typeArgumentConstraintDiagnostic.ts(5,11): error TS2344: Type 'number' does not satisfy the constraint 'string'.
+typeArgumentConstraintDiagnostic.ts(8,17): error TS2344: Type '{ value: number; }' does not satisfy the constraint '{ value: string; }'.
+  Types of property 'value' are incompatible.
+    Type 'number' is not assignable to type 'string'.
+
+
+==== typeArgumentConstraintDiagnostic.ts (3 errors) ====
+    declare function constrained<T extends string>(): void;
+    constrained<number>();
+                ~~~~~~
+!!! error TS2344: Type 'number' does not satisfy the constraint 'string'.
+
+    declare function dependent<T extends U, U>(): void;
+    dependent<number, string>();
+              ~~~~~~
+!!! error TS2344: Type 'number' does not satisfy the constraint 'string'.
+
+    declare function dependentObject<T extends U, U>(): void;
+    dependentObject<{ value: number }, { value: string }>();
+                    ~~~~~~~~~~~~~~~~~
+!!! error TS2344: Type '{ value: number; }' does not satisfy the constraint '{ value: string; }'.
+!!! error TS2344:   Types of property 'value' are incompatible.
+!!! error TS2344:     Type 'number' is not assignable to type 'string'.
+

tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.symbols

@@ -0,0 +1,30 @@
+//// [tests/cases/compiler/typeArgumentConstraintDiagnostic.ts] ////
+
+=== typeArgumentConstraintDiagnostic.ts ===
+declare function constrained<T extends string>(): void;
+>constrained : Symbol(constrained, Decl(typeArgumentConstraintDiagnostic.ts, 0, 0))
+>T : Symbol(T, Decl(typeArgumentConstraintDiagnostic.ts, 0, 29))
+
+constrained<number>();
+>constrained : Symbol(constrained, Decl(typeArgumentConstraintDiagnostic.ts, 0, 0))
+
+declare function dependent<T extends U, U>(): void;
+>dependent : Symbol(dependent, Decl(typeArgumentConstraintDiagnostic.ts, 1, 22))
+>T : Symbol(T, Decl(typeArgumentConstraintDiagnostic.ts, 3, 27))
+>U : Symbol(U, Decl(typeArgumentConstraintDiagnostic.ts, 3, 39))
+>U : Symbol(U, Decl(typeArgumentConstraintDiagnostic.ts, 3, 39))
+
+dependent<number, string>();
+>dependent : Symbol(dependent, Decl(typeArgumentConstraintDiagnostic.ts, 1, 22))
+
+declare function dependentObject<T extends U, U>(): void;
+>dependentObject : Symbol(dependentObject, Decl(typeArgumentConstraintDiagnostic.ts, 4, 28))
+>T : Symbol(T, Decl(typeArgumentConstraintDiagnostic.ts, 6, 33))
+>U : Symbol(U, Decl(typeArgumentConstraintDiagnostic.ts, 6, 45))
+>U : Symbol(U, Decl(typeArgumentConstraintDiagnostic.ts, 6, 45))
+
+dependentObject<{ value: number }, { value: string }>();
+>dependentObject : Symbol(dependentObject, Decl(typeArgumentConstraintDiagnostic.ts, 4, 28))
+>value : Symbol(value, Decl(typeArgumentConstraintDiagnostic.ts, 7, 17))
+>value : Symbol(value, Decl(typeArgumentConstraintDiagnostic.ts, 7, 36))
+

tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.types

@@ -0,0 +1,26 @@
+//// [tests/cases/compiler/typeArgumentConstraintDiagnostic.ts] ////
+
+=== typeArgumentConstraintDiagnostic.ts ===
+declare function constrained<T extends string>(): void;
+>constrained : <T extends string>() => void
+
+constrained<number>();
+>constrained<number>() : void
+>constrained : <T extends string>() => void
+
+declare function dependent<T extends U, U>(): void;
+>dependent : <T extends U, U>() => void
+
+dependent<number, string>();
+>dependent<number, string>() : void
+>dependent : <T extends U, U>() => void
+
+declare function dependentObject<T extends U, U>(): void;
+>dependentObject : <T extends U, U>() => void
+
+dependentObject<{ value: number }, { value: string }>();
+>dependentObject<{ value: number }, { value: string }>() : void
+>dependentObject : <T extends U, U>() => void
+>value : number
+>value : string
+

tsc/testdata/tests/cases/compiler/typeArgumentConstraintDiagnostic.ts

@@ -0,0 +1,11 @@
+// @strict: true
+// @noEmit: true
+
+declare function constrained<T extends string>(): void;
+constrained<number>();
+
+declare function dependent<T extends U, U>(): void;
+dependent<number, string>();
+
+declare function dependentObject<T extends U, U>(): void;
+dependentObject<{ value: number }, { value: string }>();