[internal-branch.go1.24-vendor] go/analysis/passes/printf: suppress errors for non-const format strings
The new check added in golang/go#60529 reports errors for non-constant
format strings with no arguments. These are almost always bugs, but are
often mild or inconsequential, and can be numerous in existing code
bases.
To reduce friction from this change, gate the new check on the implied
language version.
For golang/go#71485
Change-Id: I4926da2809dd14ba70ae530cd1657119f5377ad5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/645595
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Findley <rfindley@google.com>
(cherry picked from commit 9f450b061cce9ade250237ffe62343132e90d69d)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/645697
diff --git a/go/analysis/passes/printf/printf.go b/go/analysis/passes/printf/printf.go
index 171ad20..011ea8b 100644
--- a/go/analysis/passes/printf/printf.go
+++ b/go/analysis/passes/printf/printf.go
@@ -25,6 +25,7 @@
"golang.org/x/tools/go/ast/inspector"
"golang.org/x/tools/go/types/typeutil"
"golang.org/x/tools/internal/typeparams"
+ "golang.org/x/tools/internal/versions"
)
func init() {
@@ -108,12 +109,12 @@
}
}
-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
}
@@ -182,7 +183,7 @@
}
// 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
@@ -409,20 +410,29 @@
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)
- case KindPrint:
- checkPrint(pass, call, fn)
+ 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)
+ case KindPrint:
+ checkPrint(pass, n, fn)
+ }
}
})
}
@@ -503,7 +513,7 @@
}
// 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) {
+func checkPrintf(pass *analysis.Pass, fileVersion string, kind Kind, call *ast.CallExpr, fn *types.Func) {
idx := formatStringIndex(pass, call)
if idx < 0 || idx >= len(call.Args) {
return
@@ -517,7 +527,17 @@
// 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 198cf6e..1ce9c28 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 @@
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 0000000..8798291
--- /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 0000000..34d944c
--- /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 f5c9f65..0000000
--- 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 57e5bb7..0000000
--- 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 0000000..4077912
--- /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)
+}
