Skip to content

Commit

Permalink
go/analysis/passes/printf: return Result for querying func Kind
Browse files Browse the repository at this point in the history 
printf.Result allows clients to learn if a function is a Print/Printf
function or a wrapper of one.

This aids in developing Ananlyzer's applying checks on Print/Printf
functions.

Implements @alandonovan suggestion from
golang/go#29616 (comment)

Change-Id: I57833d5c65c417f2e4a761d9187fc133937f39fb
  • Loading branch information
@aaronbee
aaronbee committed Aug 31, 2019
1 parent 573d992 commit 2961237
Showing 1 changed file with 77 additions and 36 deletions.
113 changes: 77 additions & 36 deletions go/analysis/passes/printf/printf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"go/constant"
"go/token"
"go/types"
"reflect"
"regexp"
"sort"
"strconv"
Expand All @@ -31,11 +32,12 @@ func init() {
}

var Analyzer = &analysis.Analyzer{
Name: "printf",
Doc: doc,
Requires: []*analysis.Analyzer{inspect.Analyzer},
Run: run,
FactTypes: []analysis.Fact{new(isWrapper)},
Name: "printf",
Doc: doc,
Requires: []*analysis.Analyzer{inspect.Analyzer},
Run: run,
ResultType: reflect.TypeOf((*Result)(nil)),
FactTypes: []analysis.Fact{new(isWrapper)},
}

const doc = `check consistency of Printf format strings and arguments
Expand Down Expand Up @@ -66,28 +68,75 @@ argument list. Otherwise it is assumed to be Print-like, taking a list
of arguments with no format string.
`

// Kind is a kind of fmt function behavior.
type Kind int

const (
KindNone Kind = iota // not a fmt wrapper function
KindPrint // function behaves like fmt.Print
KindPrintf // function behaves like fmt.Printf
KindErrorf // function behaves like fmt.Errorf
)

func (kind Kind) String() string {
switch kind {
case KindPrint:
return "print"
case KindPrintf:
return "printf"
}
return ""
}

// Result is the printf analyzer's result type. Clients may query the result
// to learn whether a function behaves like fmt.Print or fmt.Printf.
type Result struct {
funcs map[*types.Func]Kind
}

// Kind reports whether fn behaves like fmt.Print or fmt.Printf.
func (r *Result) Kind(fn *types.Func) Kind {
_, ok := isPrint[fn.FullName()]
if !ok {
// Next look up just "printf", for use with -printf.funcs.
_, ok = isPrint[strings.ToLower(fn.Name())]
}
if ok {
if strings.HasSuffix(fn.Name(), "f") {
return KindPrintf
} else {
return KindPrint
}
}

return r.funcs[fn]
}

// isWrapper is a fact indicating that a function is a print or printf wrapper.
type isWrapper struct{ Kind funcKind }
type isWrapper struct{ Kind Kind }

func (f *isWrapper) AFact() {}

func (f *isWrapper) String() string {
switch f.Kind {
case kindPrintf:
case KindPrintf:
return "printfWrapper"
case kindPrint:
case KindPrint:
return "printWrapper"
case kindErrorf:
case KindErrorf:
return "errorfWrapper"
default:
return "unknownWrapper"
}
}

func run(pass *analysis.Pass) (interface{}, error) {
findPrintfLike(pass)
res := &Result{
funcs: make(map[*types.Func]Kind),
}
findPrintfLike(pass, res)
checkCall(pass)
return nil, nil
return res, nil
}

type printfWrapper struct {
Expand Down Expand Up @@ -154,7 +203,7 @@ func maybePrintfWrapper(info *types.Info, decl ast.Decl) *printfWrapper {
}

// findPrintfLike scans the entire package to find printf-like functions.
func findPrintfLike(pass *analysis.Pass) (interface{}, error) {
func findPrintfLike(pass *analysis.Pass, res *Result) (interface{}, error) {
// Gather potential wrappers and call graph between them.
byObj := make(map[*types.Func]*printfWrapper)
var wrappers []*printfWrapper
Expand Down Expand Up @@ -209,7 +258,7 @@ func findPrintfLike(pass *analysis.Pass) (interface{}, error) {

fn, kind := printfNameAndKind(pass, call)
if kind != 0 {
checkPrintfFwd(pass, w, call, kind)
checkPrintfFwd(pass, w, call, kind, res)
return true
}

Expand All @@ -232,20 +281,11 @@ func match(info *types.Info, arg ast.Expr, param *types.Var) bool {
return ok && info.ObjectOf(id) == param
}

type funcKind int

const (
kindUnknown funcKind = iota
kindPrintf = iota
kindPrint
kindErrorf
)

// checkPrintfFwd checks that a printf-forwarding wrapper is forwarding correctly.
// It diagnoses writing fmt.Printf(format, args) instead of fmt.Printf(format, args...).
func checkPrintfFwd(pass *analysis.Pass, w *printfWrapper, call *ast.CallExpr, kind funcKind) {
matched := kind == kindPrint ||
kind != kindUnknown && len(call.Args) >= 2 && match(pass.TypesInfo, call.Args[len(call.Args)-2], w.format)
func checkPrintfFwd(pass *analysis.Pass, w *printfWrapper, call *ast.CallExpr, kind Kind, res *Result) {
matched := kind == KindPrint ||
kind != KindNone && len(call.Args) >= 2 && match(pass.TypesInfo, call.Args[len(call.Args)-2], w.format)
if !matched {
return
}
Expand All @@ -266,7 +306,7 @@ func checkPrintfFwd(pass *analysis.Pass, w *printfWrapper, call *ast.CallExpr, k
return
}
desc := "printf"
if kind == kindPrint {
if kind == KindPrint {
desc = "print"
}
pass.Reportf(call.Pos(), "missing ... in args forwarded to %s-like function", desc)
Expand All @@ -277,8 +317,9 @@ func checkPrintfFwd(pass *analysis.Pass, w *printfWrapper, call *ast.CallExpr, k
if !pass.ImportObjectFact(fn, &fact) {
fact.Kind = kind
pass.ExportObjectFact(fn, &fact)
res.funcs[fn] = kind
for _, caller := range w.callers {
checkPrintfFwd(pass, caller.w, caller.call, kind)
checkPrintfFwd(pass, caller.w, caller.call, kind, res)
}
}
}
Expand Down Expand Up @@ -427,15 +468,15 @@ func checkCall(pass *analysis.Pass) {
call := n.(*ast.CallExpr)
fn, kind := printfNameAndKind(pass, call)
switch kind {
case kindPrintf, kindErrorf:
case KindPrintf, KindErrorf:
checkPrintf(pass, kind, call, fn)
case kindPrint:
case KindPrint:
checkPrint(pass, call, fn)
}
})
}

func printfNameAndKind(pass *analysis.Pass, call *ast.CallExpr) (fn *types.Func, kind funcKind) {
func printfNameAndKind(pass *analysis.Pass, call *ast.CallExpr) (fn *types.Func, kind Kind) {
fn, _ = typeutil.Callee(pass.TypesInfo, call).(*types.Func)
if fn == nil {
return nil, 0
Expand All @@ -448,11 +489,11 @@ func printfNameAndKind(pass *analysis.Pass, call *ast.CallExpr) (fn *types.Func,
}
if ok {
if fn.Name() == "Errorf" {
kind = kindErrorf
kind = KindErrorf
} else if strings.HasSuffix(fn.Name(), "f") {
kind = kindPrintf
kind = KindPrintf
} else {
kind = kindPrint
kind = KindPrint
}
return fn, kind
}
Expand All @@ -462,7 +503,7 @@ func printfNameAndKind(pass *analysis.Pass, call *ast.CallExpr) (fn *types.Func,
return fn, fact.Kind
}

return fn, kindUnknown
return fn, KindNone
}

// isFormatter reports whether t satisfies fmt.Formatter.
Expand Down Expand Up @@ -504,7 +545,7 @@ type formatState struct {
}

// checkPrintf checks a call to a formatted print routine such as Printf.
func checkPrintf(pass *analysis.Pass, kind funcKind, call *ast.CallExpr, fn *types.Func) {
func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, fn *types.Func) {
format, idx := formatString(pass, call)
if idx < 0 {
if false {
Expand Down Expand Up @@ -542,7 +583,7 @@ func checkPrintf(pass *analysis.Pass, kind funcKind, call *ast.CallExpr, fn *typ
anyIndex = true
}
if state.verb == 'w' {
if kind != kindErrorf {
if kind != KindErrorf {
pass.Reportf(call.Pos(), "%s call has error-wrapping directive %%w", state.name)
return
}
Expand Down

0 comments on commit 2961237

Please sign in to comment.