From 43b469a3a904bd945ba2bead5520f07c652451ab Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Wed, 17 Nov 2021 13:29:57 -0500 Subject: [PATCH] go/analysis/passes/printf: update logic now that type parameters are interfaces Now that the underlying of type parameters is their constraint interface, a couple places in the printf analyzer need to be updated: - We need to explicitly exclude type parameters when looking at interfaces in isFormatter. - We need to consider at the element type, not its underlying, when looking at pointers to type parameters Change-Id: I8c6843e001a98d45ff0f30df305e3536335f567e Reviewed-on: https://go-review.googlesource.com/c/tools/+/364678 Trust: Robert Findley Run-TryBot: Robert Findley gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- go/analysis/passes/printf/printf.go | 8 +++++++- go/analysis/passes/printf/types.go | 6 ++++-- internal/typeparams/common.go | 7 +++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/go/analysis/passes/printf/printf.go b/go/analysis/passes/printf/printf.go index 0206073578..dee37d78ae 100644 --- a/go/analysis/passes/printf/printf.go +++ b/go/analysis/passes/printf/printf.go @@ -25,6 +25,7 @@ import ( "golang.org/x/tools/go/analysis/passes/internal/analysisutil" "golang.org/x/tools/go/ast/inspector" "golang.org/x/tools/go/types/typeutil" + "golang.org/x/tools/internal/typeparams" ) func init() { @@ -520,7 +521,12 @@ func printfNameAndKind(pass *analysis.Pass, call *ast.CallExpr) (fn *types.Func, func isFormatter(typ types.Type) bool { // If the type is an interface, the value it holds might satisfy fmt.Formatter. if _, ok := typ.Underlying().(*types.Interface); ok { - return true + // Don't assume type parameters could be formatters. With the greater + // expressiveness of constraint interface syntax we expect more type safety + // when using type parameters. + if !typeparams.IsTypeParam(typ) { + return true + } } obj, _, _ := types.LookupFieldOrMethod(typ, false, nil, "Format") fn, ok := obj.(*types.Func) diff --git a/go/analysis/passes/printf/types.go b/go/analysis/passes/printf/types.go index 81bf36e1ee..270e917c80 100644 --- a/go/analysis/passes/printf/types.go +++ b/go/analysis/passes/printf/types.go @@ -178,10 +178,12 @@ func (m *argMatcher) match(typ types.Type, topLevel bool) bool { return true } + if typeparams.IsTypeParam(typ.Elem()) { + return true // We don't know whether the logic below applies. Give up. + } + under := typ.Elem().Underlying() switch under.(type) { - case *typeparams.TypeParam: - return true // We don't know whether the logic below applies. Give up. case *types.Struct: // see below case *types.Array: // see below case *types.Slice: // see below diff --git a/internal/typeparams/common.go b/internal/typeparams/common.go index 9fc6b4beb8..961d036fdb 100644 --- a/internal/typeparams/common.go +++ b/internal/typeparams/common.go @@ -13,6 +13,7 @@ package typeparams import ( "go/ast" "go/token" + "go/types" ) // A IndexExprData holds data from both ast.IndexExpr and the new @@ -23,3 +24,9 @@ type IndexExprData struct { Indices []ast.Expr // index expressions Rbrack token.Pos // position of "]" } + +// IsTypeParam reports whether t is a type parameter. +func IsTypeParam(t types.Type) bool { + _, ok := t.(*TypeParam) + return ok +}