diff --git a/cmd/goyacc/yacc.go b/cmd/goyacc/yacc.go index 5a8ede0a482..bc6395480e8 100644 --- a/cmd/goyacc/yacc.go +++ b/cmd/goyacc/yacc.go @@ -606,7 +606,7 @@ outer: } j = chfind(2, tokname) if j >= NTBASE { - lerrorf(ruleline, "nonterminal "+nontrst[j-NTBASE].name+" illegal after %%prec") + lerrorf(ruleline, "nonterminal %s illegal after %%prec", nontrst[j-NTBASE].name) } levprd[nprod] = toklev[j] t = gettok() @@ -1565,7 +1565,7 @@ more: } if pempty[i] != OK { fatfl = 0 - errorf("nonterminal " + nontrst[i].name + " never derives any token string") + errorf("nonterminal %s never derives any token string", nontrst[i].name) } } diff --git a/go/analysis/passes/printf/doc.go b/go/analysis/passes/printf/doc.go index 85da8346f75..eebf40208d1 100644 --- a/go/analysis/passes/printf/doc.go +++ b/go/analysis/passes/printf/doc.go @@ -45,6 +45,18 @@ // // log.Print("%d", 123) // log.Print call has possible formatting directive %d // +// Conversely, it also reports calls to Printf-like functions with a +// non-constant format string and no other arguments: +// +// fmt.Printf(message) // non-constant format string in call to fmt.Printf +// +// Such calls may have been intended for the function's Print-like +// counterpart: if the value of message happens to contain "%", +// misformatting will occur. In this case, the checker additionally +// suggests a fix to turn the call into: +// +// fmt.Printf("%s", message) +// // # Inferred printf wrappers // // Functions that delegate their arguments to fmt.Printf are diff --git a/go/analysis/passes/printf/main.go b/go/analysis/passes/printf/main.go new file mode 100644 index 00000000000..2a0fb7ad6c7 --- /dev/null +++ b/go/analysis/passes/printf/main.go @@ -0,0 +1,20 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build ignore + +// The printf command applies the printf checker to the specified +// packages of Go source code. +// +// Run with: +// +// $ go run ./go/analysis/passes/printf/main.go -- packages... +package main + +import ( + "golang.org/x/tools/go/analysis/passes/printf" + "golang.org/x/tools/go/analysis/singlechecker" +) + +func main() { singlechecker.Main(printf.Analyzer) } diff --git a/go/analysis/passes/printf/printf.go b/go/analysis/passes/printf/printf.go index 32350192583..b3453f8fc06 100644 --- a/go/analysis/passes/printf/printf.go +++ b/go/analysis/passes/printf/printf.go @@ -372,64 +372,29 @@ var isPrint = stringSet{ "(testing.TB).Skipf": true, } -// formatString returns the format string argument and its index within -// the given printf-like call expression. -// -// The last parameter before variadic arguments is assumed to be -// a format string. -// -// The first string literal or string constant is assumed to be a format string -// if the call's signature cannot be determined. -// -// If it cannot find any format string parameter, it returns ("", -1). -func formatString(pass *analysis.Pass, call *ast.CallExpr) (format string, idx int) { +// formatStringIndex returns the index of the format string (the last +// non-variadic parameter) within the given printf-like call +// expression, or -1 if unknown. +func formatStringIndex(pass *analysis.Pass, call *ast.CallExpr) int { typ := pass.TypesInfo.Types[call.Fun].Type - if typ != nil { - if sig, ok := typ.(*types.Signature); ok { - if !sig.Variadic() { - // Skip checking non-variadic functions. - return "", -1 - } - idx := sig.Params().Len() - 2 - if idx < 0 { - // Skip checking variadic functions without - // fixed arguments. - return "", -1 - } - s, ok := stringConstantArg(pass, call, idx) - if !ok { - // The last argument before variadic args isn't a string. - return "", -1 - } - return s, idx - } + if typ == nil { + return -1 // missing type } - - // Cannot determine call's signature. Fall back to scanning for the first - // string constant in the call. - for idx := range call.Args { - if s, ok := stringConstantArg(pass, call, idx); ok { - return s, idx - } - if pass.TypesInfo.Types[call.Args[idx]].Type == types.Typ[types.String] { - // Skip checking a call with a non-constant format - // string argument, since its contents are unavailable - // for validation. - return "", -1 - } + sig, ok := typ.(*types.Signature) + if !ok { + return -1 // ill-typed } - return "", -1 -} - -// stringConstantArg returns call's string constant argument at the index idx. -// -// ("", false) is returned if call's argument at the index idx isn't a string -// constant. -func stringConstantArg(pass *analysis.Pass, call *ast.CallExpr, idx int) (string, bool) { - if idx >= len(call.Args) { - return "", false + if !sig.Variadic() { + // Skip checking non-variadic functions. + return -1 } - return stringConstantExpr(pass, call.Args[idx]) + idx := sig.Params().Len() - 2 + if idx < 0 { + // Skip checking variadic functions without + // fixed arguments. + return -1 + } + return idx } // stringConstantExpr returns expression's string constant value. @@ -536,10 +501,34 @@ type formatState struct { // checkPrintf checks a call to a formatted print routine such as Printf. func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, fn *types.Func) { - format, idx := formatString(pass, call) - if idx < 0 { - if false { - pass.Reportf(call.Lparen, "can't check non-constant format in call to %s", fn.FullName()) + idx := formatStringIndex(pass, call) + if idx < 0 || idx >= len(call.Args) { + return + } + formatArg := call.Args[idx] + format, ok := stringConstantExpr(pass, formatArg) + if !ok { + // Format string argument is non-constant. + + // It is a common mistake to call fmt.Printf(msg) with a + // non-constant format string and no arguments: + // if msg contains "%", misformatting occurs. + // Report the problem and suggest a fix: fmt.Printf("%s", msg). + if idx == len(call.Args)-1 { + pass.Report(analysis.Diagnostic{ + Pos: formatArg.Pos(), + End: formatArg.End(), + Message: fmt.Sprintf("non-constant format string in call to %s", + fn.FullName()), + SuggestedFixes: []analysis.SuggestedFix{{ + Message: `Insert "%s" format string`, + TextEdits: []analysis.TextEdit{{ + Pos: formatArg.Pos(), + End: formatArg.Pos(), + NewText: []byte(`"%s", `), + }}, + }}, + }) } return } diff --git a/go/analysis/passes/printf/printf_test.go b/go/analysis/passes/printf/printf_test.go index 853d8619b25..3506fec1fc2 100644 --- a/go/analysis/passes/printf/printf_test.go +++ b/go/analysis/passes/printf/printf_test.go @@ -16,4 +16,5 @@ func Test(t *testing.T) { printf.Analyzer.Flags.Set("funcs", "Warn,Warnf") analysistest.Run(t, testdata, printf.Analyzer, "a", "b", "nofmt", "typeparams") + analysistest.RunWithSuggestedFixes(t, testdata, printf.Analyzer, "fix") } diff --git a/go/analysis/passes/printf/testdata/src/fix/fix.go b/go/analysis/passes/printf/testdata/src/fix/fix.go new file mode 100644 index 00000000000..f5c9f654165 --- /dev/null +++ b/go/analysis/passes/printf/testdata/src/fix/fix.go @@ -0,0 +1,20 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests of the printf checker's suggested fixes. + +package fix + +import ( + "fmt" + "log" + "os" +) + +func nonConstantFormat(s string) { // #60529 + fmt.Printf(s) // want `non-constant format string in call to fmt.Printf` + fmt.Printf(s, "arg") + fmt.Fprintf(os.Stderr, s) // want `non-constant format string in call to fmt.Fprintf` + log.Printf(s) // want `non-constant format string in call to log.Printf` +} diff --git a/go/analysis/passes/printf/testdata/src/fix/fix.go.golden b/go/analysis/passes/printf/testdata/src/fix/fix.go.golden new file mode 100644 index 00000000000..57e5bb7db91 --- /dev/null +++ b/go/analysis/passes/printf/testdata/src/fix/fix.go.golden @@ -0,0 +1,20 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// This file contains tests of the printf checker's suggested fixes. + +package fix + +import ( + "fmt" + "log" + "os" +) + +func nonConstantFormat(s string) { // #60529 + fmt.Printf("%s", s) // want `non-constant format string in call to fmt.Printf` + fmt.Printf(s, "arg") + fmt.Fprintf(os.Stderr, "%s", s) // want `non-constant format string in call to fmt.Fprintf` + log.Printf("%s", s) // want `non-constant format string in call to log.Printf` +} diff --git a/go/analysis/passes/tests/tests.go b/go/analysis/passes/tests/tests.go index f5e760ca265..5b4598235cf 100644 --- a/go/analysis/passes/tests/tests.go +++ b/go/analysis/passes/tests/tests.go @@ -6,7 +6,6 @@ package tests import ( _ "embed" - "fmt" "go/ast" "go/token" "go/types" @@ -184,13 +183,13 @@ func checkAddCalls(pass *analysis.Pass, fn *ast.FuncDecl, params *types.Tuple) { i := mismatched[0] expr := call.Args[i] t := pass.TypesInfo.Types[expr].Type - pass.ReportRangef(expr, fmt.Sprintf("mismatched type in call to (*testing.F).Add: %v, fuzz target expects %v", t, params.At(i+1).Type())) + pass.ReportRangef(expr, "mismatched type in call to (*testing.F).Add: %v, fuzz target expects %v", t, params.At(i+1).Type()) } else if len(mismatched) > 1 { var gotArgs, wantArgs []types.Type for i := 0; i < len(call.Args); i++ { gotArgs, wantArgs = append(gotArgs, pass.TypesInfo.Types[call.Args[i]].Type), append(wantArgs, params.At(i+1).Type()) } - pass.ReportRangef(call, fmt.Sprintf("mismatched types in call to (*testing.F).Add: %v, fuzz target expects %v", gotArgs, wantArgs)) + pass.ReportRangef(call, "mismatched types in call to (*testing.F).Add: %v, fuzz target expects %v", gotArgs, wantArgs) } } return true @@ -244,7 +243,7 @@ func validateFuzzArgs(pass *analysis.Pass, params *types.Tuple, expr ast.Expr) b } } } - pass.ReportRangef(exprRange, "fuzzing arguments can only have the following types: "+formatAcceptedFuzzType()) + pass.ReportRangef(exprRange, "fuzzing arguments can only have the following types: %s", formatAcceptedFuzzType()) ok = false } }