diff --git a/tools/customlint/diagnosticargs.go b/tools/customlint/diagnosticargs.go new file mode 100644 index 0000000000000..cc2e94df9e315 --- /dev/null +++ b/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 +} diff --git a/tools/customlint/plugin.go b/tools/customlint/plugin.go index 476b68e7a8901..410512a76e6b1 100644 --- a/tools/customlint/plugin.go +++ b/tools/customlint/plugin.go @@ -19,6 +19,7 @@ func (f *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { checkChildrenAnalyzer, cleanupAnalyzer, caseBodyAnalyzer, + diagnosticArgsAnalyzer, forbidParentAccessAnalyzer, protectedFieldAnalyzer, shadowAnalyzer, diff --git a/tools/customlint/testdata/diagnosticargs/diagnosticargs.go b/tools/customlint/testdata/diagnosticargs/diagnosticargs.go new file mode 100644 index 0000000000000..ad426d373f76c --- /dev/null +++ b/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) +} diff --git a/tools/customlint/testdata/diagnosticargs/diagnosticargs.go.golden b/tools/customlint/testdata/diagnosticargs/diagnosticargs.go.golden new file mode 100644 index 0000000000000..b53cfe57f4ffe --- /dev/null +++ b/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) + } + diff --git a/tools/customlint/testdata/internal/diagnostics/diagnostics.go b/tools/customlint/testdata/internal/diagnostics/diagnostics.go new file mode 100644 index 0000000000000..7200ed0ad1ab4 --- /dev/null +++ b/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 +} diff --git a/tools/customlint/testdata/internal/diagnostics/diagnostics.go.golden b/tools/customlint/testdata/internal/diagnostics/diagnostics.go.golden new file mode 100644 index 0000000000000..2ca7474d20851 --- /dev/null +++ b/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 + } + diff --git a/tsc/internal/checker/checker.go b/tsc/internal/checker/checker.go index 0f6ad0deacdc3..5cdbf40428c76 100644 --- a/tsc/internal/checker/checker.go +++ b/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) } diff --git a/tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.errors.txt b/tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.errors.txt new file mode 100644 index 0000000000000..7b9165f06bdfb --- /dev/null +++ b/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(): void; + constrained(); + ~~~~~~ +!!! error TS2344: Type 'number' does not satisfy the constraint 'string'. + + declare function dependent(): void; + dependent(); + ~~~~~~ +!!! error TS2344: Type 'number' does not satisfy the constraint 'string'. + + declare function dependentObject(): 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'. + \ No newline at end of file diff --git a/tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.symbols b/tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.symbols new file mode 100644 index 0000000000000..da43243b4dd55 --- /dev/null +++ b/tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.symbols @@ -0,0 +1,30 @@ +//// [tests/cases/compiler/typeArgumentConstraintDiagnostic.ts] //// + +=== typeArgumentConstraintDiagnostic.ts === +declare function constrained(): void; +>constrained : Symbol(constrained, Decl(typeArgumentConstraintDiagnostic.ts, 0, 0)) +>T : Symbol(T, Decl(typeArgumentConstraintDiagnostic.ts, 0, 29)) + +constrained(); +>constrained : Symbol(constrained, Decl(typeArgumentConstraintDiagnostic.ts, 0, 0)) + +declare function dependent(): 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(); +>dependent : Symbol(dependent, Decl(typeArgumentConstraintDiagnostic.ts, 1, 22)) + +declare function dependentObject(): 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)) + diff --git a/tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.types b/tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.types new file mode 100644 index 0000000000000..ecb87cb5fd7b4 --- /dev/null +++ b/tsc/testdata/baselines/reference/compiler/typeArgumentConstraintDiagnostic.types @@ -0,0 +1,26 @@ +//// [tests/cases/compiler/typeArgumentConstraintDiagnostic.ts] //// + +=== typeArgumentConstraintDiagnostic.ts === +declare function constrained(): void; +>constrained : () => void + +constrained(); +>constrained() : void +>constrained : () => void + +declare function dependent(): void; +>dependent : () => void + +dependent(); +>dependent() : void +>dependent : () => void + +declare function dependentObject(): void; +>dependentObject : () => void + +dependentObject<{ value: number }, { value: string }>(); +>dependentObject<{ value: number }, { value: string }>() : void +>dependentObject : () => void +>value : number +>value : string + diff --git a/tsc/testdata/tests/cases/compiler/typeArgumentConstraintDiagnostic.ts b/tsc/testdata/tests/cases/compiler/typeArgumentConstraintDiagnostic.ts new file mode 100644 index 0000000000000..fcb2cb3044ce6 --- /dev/null +++ b/tsc/testdata/tests/cases/compiler/typeArgumentConstraintDiagnostic.ts @@ -0,0 +1,11 @@ +// @strict: true +// @noEmit: true + +declare function constrained(): void; +constrained(); + +declare function dependent(): void; +dependent(); + +declare function dependentObject(): void; +dependentObject<{ value: number }, { value: string }>();