qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-18 19:44:46 -07:00
Code Releases Activity

go/analysis/passes/printf: warn against using %v verb in Error() methods

Browse Source 
go vet should warn against using %v verb in Error() methods just like it
does for String().

Fixes golang/go#33884

Change-Id: I14e30aae316dc84adc62e2b2a0144cc157bb2698
GitHub-Last-Rev: 12240ac78d9c8afb12b014eff8f81ea0bf42471f
GitHub-Pull-Request: golang/tools#214
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223239
Reviewed-by: Michael Matloob <matloob@golang.org>
This commit is contained in:
Sergey Glushchenko 2020-03-23 19:11:59 +00:00 committed by Michael Matloob
parent ef1313dc6d
commit a576cf5246
2 changed files with 85 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
go/analysis/passes/printf/printf.go
View File

@ -895,43 +895,51 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, state *formatState) (o
pass.ReportRangef(call, "%s format %s has arg %s of wrong type %s", state.name, state.format, analysisutil.Format(pass.Fset, arg), typeString)
return false
}
if v.typ&argString != 0 && v.verb != 'T' && !bytes.Contains(state.flags, []byte{'#'}) && recursiveStringer(pass, arg) {
pass.ReportRangef(call, "%s format %s with arg %s causes recursive String method call", state.name, state.format, analysisutil.Format(pass.Fset, arg))
if v.typ&argString != 0 && v.verb != 'T' && !bytes.Contains(state.flags, []byte{'#'}) {
if methodName, ok := recursiveStringer(pass, arg); ok {
pass.ReportRangef(call, "%s format %s with arg %s causes recursive %s method call", state.name, state.format, analysisutil.Format(pass.Fset, arg), methodName)
return false
}
}
return true
}
// recursiveStringer reports whether the argument e is a potential
// recursive call to stringer, such as t and &t in these examples:
// recursive call to stringer or is an error, such as t and &t in these examples:
//
// func (t *T) String() string { printf("%s", t) }
// func (t T) String() string { printf("%s", t) }
// func (t T) Error() string { printf("%s", t) }
// func (t T) String() string { printf("%s", &t) }
//
func recursiveStringer(pass *analysis.Pass, e ast.Expr) bool {
func recursiveStringer(pass *analysis.Pass, e ast.Expr) (string, bool) {
typ := pass.TypesInfo.Types[e].Type
// It's unlikely to be a recursive stringer if it has a Format method.
if isFormatter(typ) {
return false
return "", false
}
// Does e allow e.String()?
obj, _, _ := types.LookupFieldOrMethod(typ, false, pass.Pkg, "String")
stringMethod, ok := obj.(*types.Func)
if !ok {
return false
// Does e allow e.String() or e.Error()?
strObj, _, _ := types.LookupFieldOrMethod(typ, false, pass.Pkg, "String")
strMethod, strOk := strObj.(*types.Func)
errObj, _, _ := types.LookupFieldOrMethod(typ, false, pass.Pkg, "Error")
errMethod, errOk := errObj.(*types.Func)
if !strOk && !errOk {
return "", false
}
// Is the expression e within the body of that String method?
if stringMethod.Pkg() != pass.Pkg || !stringMethod.Scope().Contains(e.Pos()) {
return false
// Is the expression e within the body of that String or Error method?
var method *types.Func
if strOk && strMethod.Pkg() == pass.Pkg && strMethod.Scope().Contains(e.Pos()) {
method = strMethod
} else if errOk && errMethod.Pkg() == pass.Pkg && errMethod.Scope().Contains(e.Pos()) {
method = errMethod
} else {
return "", false
}
sig := stringMethod.Type().(*types.Signature)
sig := method.Type().(*types.Signature)
if !isStringer(sig) {
return false
return "", false
}
// Is it the receiver r, or &r?
@ -939,9 +947,11 @@ func recursiveStringer(pass *analysis.Pass, e ast.Expr) bool {
e = u.X // strip off & from &r
}
if id, ok := e.(*ast.Ident); ok {
return pass.TypesInfo.Uses[id] == sig.Recv()
if pass.TypesInfo.Uses[id] == sig.Recv() {
return method.Name(), true
}
return false
}
return "", false
}
// isStringer reports whether the method signature matches the String() definition in fmt.Stringer.
@ -1065,8 +1075,8 @@ func checkPrint(pass *analysis.Pass, call *ast.CallExpr, fn *types.Func) {
if isFunctionValue(pass, arg) {
pass.ReportRangef(call, "%s arg %s is a func value, not called", fn.Name(), analysisutil.Format(pass.Fset, arg))
}
if recursiveStringer(pass, arg) {
pass.ReportRangef(call, "%s arg %s causes recursive call to String method", fn.Name(), analysisutil.Format(pass.Fset, arg))
if methodName, ok := recursiveStringer(pass, arg); ok {
pass.ReportRangef(call, "%s arg %s causes recursive call to %s method", fn.Name(), analysisutil.Format(pass.Fset, arg), methodName)
}
}
}

53
go/analysis/passes/printf/testdata/src/a/a.go vendored
View File

@ -526,6 +526,59 @@ func (p *recursivePtrStringer) String() string {
return fmt.Sprintln(p) // want "Sprintln arg p causes recursive call to String method"
}
type recursiveError int
func (s recursiveError) Error() string {
_ = fmt.Sprintf("%d", s)
_ = fmt.Sprintf("%#v", s)
_ = fmt.Sprintf("%v", s) // want "Sprintf format %v with arg s causes recursive Error method call"
_ = fmt.Sprintf("%v", &s) // want "Sprintf format %v with arg &s causes recursive Error method call"
_ = fmt.Sprintf("%T", s) // ok; does not recursively call Error
return fmt.Sprintln(s) // want "Sprintln arg s causes recursive call to Error method"
}
type recursivePtrError int
func (p *recursivePtrError) Error() string {
_ = fmt.Sprintf("%v", *p)
_ = fmt.Sprint(&p) // ok; prints address
return fmt.Sprintln(p) // want "Sprintln arg p causes recursive call to Error method"
}
type recursiveStringerAndError int
func (s recursiveStringerAndError) String() string {
_ = fmt.Sprintf("%d", s)
_ = fmt.Sprintf("%#v", s)
_ = fmt.Sprintf("%v", s) // want "Sprintf format %v with arg s causes recursive String method call"
_ = fmt.Sprintf("%v", &s) // want "Sprintf format %v with arg &s causes recursive String method call"
_ = fmt.Sprintf("%T", s) // ok; does not recursively call String
return fmt.Sprintln(s) // want "Sprintln arg s causes recursive call to String method"
}
func (s recursiveStringerAndError) Error() string {
_ = fmt.Sprintf("%d", s)
_ = fmt.Sprintf("%#v", s)
_ = fmt.Sprintf("%v", s) // want "Sprintf format %v with arg s causes recursive Error method call"
_ = fmt.Sprintf("%v", &s) // want "Sprintf format %v with arg &s causes recursive Error method call"
_ = fmt.Sprintf("%T", s) // ok; does not recursively call Error
return fmt.Sprintln(s) // want "Sprintln arg s causes recursive call to Error method"
}
type recursivePtrStringerAndError int
func (p *recursivePtrStringerAndError) String() string {
_ = fmt.Sprintf("%v", *p)
_ = fmt.Sprint(&p) // ok; prints address
return fmt.Sprintln(p) // want "Sprintln arg p causes recursive call to String method"
}
func (p *recursivePtrStringerAndError) Error() string {
_ = fmt.Sprintf("%v", *p)
_ = fmt.Sprint(&p) // ok; prints address
return fmt.Sprintln(p) // want "Sprintln arg p causes recursive call to Error method"
}
// implements a String() method but with non-matching return types
type nonStringerWrongReturn int