diff --git a/go/analysis/passes/printf/printf.go b/go/analysis/passes/printf/printf.go index b95e2fd6f1a..81600a283aa 100644 --- a/go/analysis/passes/printf/printf.go +++ b/go/analysis/passes/printf/printf.go @@ -24,6 +24,7 @@ import ( "golang.org/x/tools/internal/analysisinternal" "golang.org/x/tools/internal/fmtstr" "golang.org/x/tools/internal/typeparams" + "golang.org/x/tools/internal/versions" ) func init() { @@ -107,12 +108,12 @@ func (f *isWrapper) String() string { } } -func run(pass *analysis.Pass) (interface{}, error) { +func run(pass *analysis.Pass) (any, error) { res := &Result{ funcs: make(map[*types.Func]Kind), } findPrintfLike(pass, res) - checkCall(pass) + checkCalls(pass) return res, nil } @@ -181,7 +182,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, res *Result) (interface{}, error) { +func findPrintfLike(pass *analysis.Pass, res *Result) (any, error) { // Gather potential wrappers and call graph between them. byObj := make(map[*types.Func]*printfWrapper) var wrappers []*printfWrapper @@ -408,20 +409,29 @@ func stringConstantExpr(pass *analysis.Pass, expr ast.Expr) (string, bool) { return "", false } -// checkCall triggers the print-specific checks if the call invokes a print function. -func checkCall(pass *analysis.Pass) { +// checkCalls triggers the print-specific checks for calls that invoke a print +// function. +func checkCalls(pass *analysis.Pass) { inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) nodeFilter := []ast.Node{ + (*ast.File)(nil), (*ast.CallExpr)(nil), } + + var fileVersion string // for selectively suppressing checks; "" if unknown. inspect.Preorder(nodeFilter, func(n ast.Node) { - call := n.(*ast.CallExpr) - fn, kind := printfNameAndKind(pass, call) - switch kind { - case KindPrintf, KindErrorf: - checkPrintf(pass, kind, call, fn.FullName()) - case KindPrint: - checkPrint(pass, call, fn.FullName()) + switch n := n.(type) { + case *ast.File: + fileVersion = versions.Lang(versions.FileVersion(pass.TypesInfo, n)) + + case *ast.CallExpr: + fn, kind := printfNameAndKind(pass, n) + switch kind { + case KindPrintf, KindErrorf: + checkPrintf(pass, fileVersion, kind, n, fn.FullName()) + case KindPrint: + checkPrint(pass, n, fn.FullName()) + } } }) } @@ -484,7 +494,7 @@ func isFormatter(typ types.Type) bool { } // checkPrintf checks a call to a formatted print routine such as Printf. -func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, name string) { +func checkPrintf(pass *analysis.Pass, fileVersion string, kind Kind, call *ast.CallExpr, name string) { idx := formatStringIndex(pass, call) if idx < 0 || idx >= len(call.Args) { return @@ -498,7 +508,17 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, name string // non-constant format string and no arguments: // if msg contains "%", misformatting occurs. // Report the problem and suggest a fix: fmt.Printf("%s", msg). - if !suppressNonconstants && idx == len(call.Args)-1 { + // + // However, as described in golang/go#71485, this analysis can produce a + // significant number of diagnostics in existing code, and the bugs it + // finds are sometimes unlikely or inconsequential, and may not be worth + // fixing for some users. Gating on language version allows us to avoid + // breaking existing tests and CI scripts. + if !suppressNonconstants && + idx == len(call.Args)-1 && + fileVersion != "" && // fail open + versions.AtLeast(fileVersion, "go1.24") { + pass.Report(analysis.Diagnostic{ Pos: formatArg.Pos(), End: formatArg.End(), diff --git a/go/analysis/passes/printf/printf_test.go b/go/analysis/passes/printf/printf_test.go index 198cf6ec549..1ce9c28c103 100644 --- a/go/analysis/passes/printf/printf_test.go +++ b/go/analysis/passes/printf/printf_test.go @@ -5,10 +5,13 @@ package printf_test import ( + "path/filepath" "testing" "golang.org/x/tools/go/analysis/analysistest" "golang.org/x/tools/go/analysis/passes/printf" + "golang.org/x/tools/internal/testenv" + "golang.org/x/tools/internal/testfiles" ) func Test(t *testing.T) { @@ -16,6 +19,19 @@ func Test(t *testing.T) { printf.Analyzer.Flags.Set("funcs", "Warn,Warnf") analysistest.Run(t, testdata, printf.Analyzer, - "a", "b", "nofmt", "typeparams", "issue68744", "issue70572") - analysistest.RunWithSuggestedFixes(t, testdata, printf.Analyzer, "fix") + "a", "b", "nofmt", "nonconst", "typeparams", "issue68744", "issue70572") +} + +func TestNonConstantFmtString_Go123(t *testing.T) { + testenv.NeedsGo1Point(t, 23) + + dir := testfiles.ExtractTxtarFileToTmp(t, filepath.Join(analysistest.TestData(), "nonconst_go123.txtar")) + analysistest.RunWithSuggestedFixes(t, dir, printf.Analyzer, "example.com/nonconst") +} + +func TestNonConstantFmtString_Go124(t *testing.T) { + testenv.NeedsGo1Point(t, 24) + + dir := testfiles.ExtractTxtarFileToTmp(t, filepath.Join(analysistest.TestData(), "nonconst_go124.txtar")) + analysistest.RunWithSuggestedFixes(t, dir, printf.Analyzer, "example.com/nonconst") } diff --git a/go/analysis/passes/printf/testdata/nonconst_go123.txtar b/go/analysis/passes/printf/testdata/nonconst_go123.txtar new file mode 100644 index 00000000000..87982917d9e --- /dev/null +++ b/go/analysis/passes/printf/testdata/nonconst_go123.txtar @@ -0,0 +1,61 @@ +This test checks for the correct suppression (or activation) of the +non-constant format string check (golang/go#60529), in a go1.23 module. + +See golang/go#71485 for details. + +-- go.mod -- +module example.com/nonconst + +go 1.23 + +-- nonconst.go -- +package nonconst + +import ( + "fmt" + "log" + "os" +) + +func _(s string) { + fmt.Printf(s) + fmt.Printf(s, "arg") + fmt.Fprintf(os.Stderr, s) + log.Printf(s) +} + +-- nonconst_go124.go -- +//go:build go1.24 +package nonconst + +import ( + "fmt" + "log" + "os" +) + +// With Go 1.24, the analyzer should be activated, as this is a go1.24 file. +func _(s string) { + 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` +} + +-- nonconst_go124.go.golden -- +//go:build go1.24 +package nonconst + +import ( + "fmt" + "log" + "os" +) + +// With Go 1.24, the analyzer should be activated, as this is a go1.24 file. +func _(s string) { + 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/printf/testdata/nonconst_go124.txtar b/go/analysis/passes/printf/testdata/nonconst_go124.txtar new file mode 100644 index 00000000000..34d944ce970 --- /dev/null +++ b/go/analysis/passes/printf/testdata/nonconst_go124.txtar @@ -0,0 +1,59 @@ +This test checks for the correct suppression (or activation) of the +non-constant format string check (golang/go#60529), in a go1.24 module. + +See golang/go#71485 for details. + +-- go.mod -- +module example.com/nonconst + +go 1.24 + +-- nonconst.go -- +package nonconst + +import ( + "fmt" + "log" + "os" +) + +func _(s string) { + 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` +} + +-- nonconst.go.golden -- +package nonconst + +import ( + "fmt" + "log" + "os" +) + +func _(s string) { + 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` +} + +-- nonconst_go123.go -- +//go:build go1.23 +package nonconst + +import ( + "fmt" + "log" + "os" +) + +// The analyzer should be silent, as this is a go1.23 file. +func _(s string) { + fmt.Printf(s) + fmt.Printf(s, "arg") + fmt.Fprintf(os.Stderr, s) + log.Printf(s) +} diff --git a/go/analysis/passes/printf/testdata/src/fix/fix.go b/go/analysis/passes/printf/testdata/src/fix/fix.go deleted file mode 100644 index f5c9f654165..00000000000 --- a/go/analysis/passes/printf/testdata/src/fix/fix.go +++ /dev/null @@ -1,20 +0,0 @@ -// 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 deleted file mode 100644 index 57e5bb7db91..00000000000 --- a/go/analysis/passes/printf/testdata/src/fix/fix.go.golden +++ /dev/null @@ -1,20 +0,0 @@ -// 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/printf/testdata/src/nonconst/nonconst.go b/go/analysis/passes/printf/testdata/src/nonconst/nonconst.go new file mode 100644 index 00000000000..40779123a52 --- /dev/null +++ b/go/analysis/passes/printf/testdata/src/nonconst/nonconst.go @@ -0,0 +1,23 @@ +// 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 handling of non-constant +// format strings (golang/go#60529). + +package nonconst + +import ( + "fmt" + "log" + "os" +) + +// As the language version is empty here, and the new check is gated on go1.24, +// diagnostics are suppressed here. +func nonConstantFormat(s string) { + fmt.Printf(s) + fmt.Printf(s, "arg") + fmt.Fprintf(os.Stderr, s) + log.Printf(s) +}