[tools] go/analysis/passes/printf: report non-constant format, no args
26 views
Skip to first unread message
Alan Donovan (Gerrit)
May 15, 2024, 10:43:33 AM5/15/24
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Alan Donovan has uploaded the change for review![]( "Open in Gerrit")
Commit message
go/analysis/passes/printf: report non-constant format, no args
Calls such as fmt.Printf(s), where s is non-constant and there
are no arguments to format, are invariably a mistake. This
CL causes the printf checker to report them, and to offer
a suggested fix of fmt.Printf("%s", s).
Also:
- tests
- fixes to existing violations in x/tools (3 bugs, 2 merely bad form).
- an ignore-tagged main file for the printf checker.
Fixes golang/go#60529
Change-Id: Ic7cc4f3e7487bc1549948fa03b5b0b27959948e5
Change diff
diff --git a/cmd/goyacc/yacc.go b/cmd/goyacc/yacc.go
index 5a8ede0..bc63954 100644
--- a/cmd/goyacc/yacc.go
+++ b/cmd/goyacc/yacc.go
@@ -606,7 +606,7 @@
}
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 @@
}
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/main.go b/go/analysis/passes/printf/main.go
new file mode 100644
index 0000000..2a0fb7a
--- /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 3235019..6ec0d72 100644
--- a/go/analysis/passes/printf/printf.go
+++ b/go/analysis/passes/printf/printf.go
@@ -382,54 +382,26 @@
// 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) {
+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 +508,34 @@
// 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.TypesInfo.Types[formatArg].Value == nil {
+ 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 853d861..3506fec 100644
--- a/go/analysis/passes/printf/printf_test.go
+++ b/go/analysis/passes/printf/printf_test.go
@@ -16,4 +16,5 @@
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 0000000..9f9057a
--- /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 0000000..db972b0
--- /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 f5e760c..5b45982 100644
--- a/go/analysis/passes/tests/tests.go
+++ b/go/analysis/passes/tests/tests.go
@@ -6,7 +6,6 @@
import (
_ "embed"
- "fmt"
"go/ast"
"go/token"
"go/types"
@@ -184,13 +183,13 @@
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 @@
}
}
}
- 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
}
}
Change information
Files:
- M cmd/goyacc/yacc.go
- A go/analysis/passes/printf/main.go
- M go/analysis/passes/printf/printf.go
- M go/analysis/passes/printf/printf_test.go
- A go/analysis/passes/printf/testdata/src/fix/fix.go
- A go/analysis/passes/printf/testdata/src/fix/fix.go.golden
- M go/analysis/passes/tests/tests.go
Change size: M
Delta: 7 files changed, 110 insertions(+), 54 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
May 15, 2024, 10:45:32 AM5/15/24
to goph...@pubsubhelper.golang.org, Tim King, Russ Cox, Go LUCI, golang-co...@googlegroups.com
Attention needed from Russ Cox and Tim King
Alan Donovan added 1 comment![]( "Open in Gerrit")
File cmd/goyacc/yacc.go
Line 609, Patchset 1 (Latest):
lerrorf(ruleline, "nonterminal %s illegal after %%prec", nontrst[j-NTBASE].name)Alan Donovan . unresolved
Does this count as a false positive? The previous code was sound so long as name doesn't contain "%", which it very likely does not in this case.
I wonder whether it needs a variant to catch ("..." + s + "...") that suggests using ("...%s...", s).
Related details
Attention is currently required from:
- Russ Cox
- Tim King
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
May 15, 2024, 12:31:28 PM5/15/24
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Russ Cox and Tim King
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Russ Cox
- Tim King
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
May 17, 2024, 12:59:29 PM5/17/24
to goph...@pubsubhelper.golang.org, Go LUCI, Tim King, Russ Cox, golang-co...@googlegroups.com
Attention needed from Russ Cox and Tim King
Alan Donovan added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Alan Donovan . resolved
Russ: Tim's on vacation; would you mind looking at this CL before the freeze? Thanks.
Related details
Attention is currently required from:
- Russ Cox
- Tim King
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
May 17, 2024, 1:02:53 PM5/17/24
to goph...@pubsubhelper.golang.org, Go LUCI, Tim King, Russ Cox, golang-co...@googlegroups.com
Attention needed from Russ Cox and Tim King
Alan Donovan added 1 comment![]( "Open in Gerrit")
File go/analysis/passes/printf/printf.go
Line 408, Patchset 2 (Parent):
// Cannot determine call's signature. Fall back to scanning for the firstAlan Donovan . unresolved
(This block was deleted as it is unreachable in well-typed code, which the printf analyzer requires.)
Alan Donovan (Gerrit)
May 22, 2024, 1:04:15 PM5/22/24
to goph...@pubsubhelper.golang.org, Robert Findley, Russ Cox, Tim King, Go LUCI, golang-co...@googlegroups.com
Attention needed from Robert Findley
Alan Donovan added 1 comment![]( "Open in Gerrit")
File cmd/goyacc/yacc.go
Line 609, Patchset 1:
lerrorf(ruleline, "nonterminal %s illegal after %%prec", nontrst[j-NTBASE].name)Alan Donovan . unresolved
Does this count as a false positive? The previous code was sound so long as name doesn't contain "%", which it very likely does not in this case.
I wonder whether it needs a variant to catch ("..." + s + "...") that suggests using ("...%s...", s).
Attention is currently required from:
- Robert Findley
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Robert Findley (Gerrit)
May 22, 2024, 8:02:44 PM5/22/24
to Alan Donovan, goph...@pubsubhelper.golang.org, Russ Cox, Tim King, Go LUCI, golang-co...@googlegroups.com
Attention needed from Alan Donovan
Robert Findley voted and added 3 comments![]( "Open in Gerrit")
Votes added by Robert Findley
| Code-Review | +2 |
3 comments
File cmd/goyacc/yacc.go
Line 609, Patchset 1:
lerrorf(ruleline, "nonterminal %s illegal after %%prec", nontrst[j-NTBASE].name)Alan Donovan . resolved
Alan DonovanDoes this count as a false positive? The previous code was sound so long as name doesn't contain "%", which it very likely does not in this case.
I wonder whether it needs a variant to catch ("..." + s + "...") that suggests using ("...%s...", s).
(Russ opined in the proposal committee that this is fine.)
Robert Findley
Acknowledged
File go/analysis/passes/printf/printf.go
Line 375, Patchset 2 (Latest):
// 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).Robert Findley . unresolved
This comment is stale, and needs to be updated for the new signature of this function.
Line 524, Patchset 2 (Latest):
if idx == len(call.Args)-1 && pass.TypesInfo.Types[formatArg].Value == nil {Robert Findley . unresolved
We already know that the argument is non constant, so shouldn't need to check Value == nil again.
Related details
Attention is currently required from:
- Alan Donovan
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
May 23, 2024, 10:51:55 AM5/23/24
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #3 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
Attention is currently required from:
- Alan Donovan
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
May 23, 2024, 10:52:06 AM5/23/24
to goph...@pubsubhelper.golang.org, Robert Findley, Russ Cox, Tim King, Go LUCI, golang-co...@googlegroups.com
Alan Donovan added 3 comments![]( "Open in Gerrit")
File go/analysis/passes/printf/printf.go
Line 375, Patchset 2:
// 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).Robert Findley . resolved
This comment is stale, and needs to be updated for the new signature of this function.
Alan Donovan
Oops, I missed that. Rewrote the whole to:
// formatStringIndex returns the index of the format string (the last
// non-variadic parameter) within the given printf-like call
// expression, or -1 if unknown.
Line 408, Patchset 2 (Parent):
// Cannot determine call's signature. Fall back to scanning for the firstAlan Donovan . resolved
(This block was deleted as it is unreachable in well-typed code, which the printf analyzer requires.)
Alan Donovan
Acknowledged
Line 524, Patchset 2:
if idx == len(call.Args)-1 && pass.TypesInfo.Types[formatArg].Value == nil {Robert Findley . resolved
We already know that the argument is non constant, so shouldn't need to check Value == nil again.
Alan Donovan
Nice. Done.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
May 23, 2024, 2:03:44 PM5/23/24
to goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, Russ Cox, Tim King, golang-co...@googlegroups.com
Alan Donovan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alan Donovan
| Hold | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 3 (Latest):
Alan Donovan . resolved
Hold for go1.24, otherwise revendoring x/tools in GOROOT will bring this into 1.23 during the freeze.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
May 23, 2024, 2:44:36 PM5/23/24
to goph...@pubsubhelper.golang.org, Go LUCI, Robert Findley, Russ Cox, Tim King, golang-co...@googlegroups.com
Alan Donovan added 1 comment![]( "Open in Gerrit")
Commit Message
Line 22, Patchset 3 (Latest):
DO NOT SUMBIT until CL 587855 (to clean up std) landsAlan Donovan . unresolved
Don't forget to add a go1.24 release note when we eventually vendor this into GOROOT.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
May 23, 2024, 9:16:15 PM5/23/24
to goph...@pubsubhelper.golang.org, Russ Cox, Go LUCI, Robert Findley, Tim King, golang-co...@googlegroups.com
Attention needed from Russ Cox
Alan Donovan added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Russ Cox . resolved
Alan DonovanPlease wait for Go 1.24 at this point.
For once, I'm a step ahead of you! ;-)
Related details
Attention is currently required from:
- Russ Cox
Submit Requirements:
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tim King (Gerrit)
Jul 19, 2024, 1:29:14 PM7/19/24
to Alan Donovan, goph...@pubsubhelper.golang.org, Russ Cox, Go LUCI, Robert Findley, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Russ Cox
Tim King added 3 comments![]( "Open in Gerrit")
File go/analysis/passes/printf/printf.go
Line 391, Patchset 3 (Latest):
idx := sig.Params().Len() - 2Tim King . unresolved
I think a comment should explain "-2" here. Took me a minute.
Line 508, Patchset 3 (Latest):
formatArg := call.Args[idx]Tim King . unresolved
Is it worth bothering adjusted idx for method expressions? My guess is no, but I thought I'd ask.
Line 524, Patchset 3 (Latest):
Message: "Insert \"%s\" format string",Tim King . unresolved
nit: Good place to use back ticks `
Related details
Attention is currently required from:
- Alan Donovan
- Russ Cox
Submit Requirements:
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Jul 22, 2024, 12:07:20 PM7/22/24
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Russ Cox
Alan Donovan uploaded new patchset![]( "Open in Gerrit")
Alan Donovan uploaded patch set #4 to this change.
Following approvals got outdated and were removed:
- TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
Related details
Attention is currently required from:
- Alan Donovan
- Russ Cox
Submit Requirements:
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Jul 22, 2024, 12:07:30 PM7/22/24
to goph...@pubsubhelper.golang.org, Russ Cox, Go LUCI, Robert Findley, Tim King, golang-co...@googlegroups.com
Attention needed from Russ Cox and Tim King
Alan Donovan voted and added 4 comments![]( "Open in Gerrit")
Votes added by Alan Donovan
| Hold | +1 |
4 comments
Commit Message
Don't forget to add a go1.24 release note when we eventually vendor this into GOROOT.
Alan Donovan
Will do.
File go/analysis/passes/printf/printf.go
I think a comment should explain "-2" here. Took me a minute.
Alan Donovan
The doc comment explains that it "returns the index of the format string (the last
non-variadic parameter)". Not sure we need to repeat that here.
Line 508, Patchset 3:
formatArg := call.Args[idx]Tim King . unresolved
Is it worth bothering adjusted idx for method expressions? My guess is no, but I thought I'd ask.
Alan Donovan
We're analyzing a CallExpr that statically calls a function symbol, so there's no method expression here. I'm not sure what you're suggesting.
nit: Good place to use back ticks `
Attention is currently required from:
- Russ Cox
- Tim King
Submit Requirements:
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Jul 22, 2024, 12:07:41 PM7/22/24
to goph...@pubsubhelper.golang.org, Russ Cox, Go LUCI, Robert Findley, Tim King, golang-co...@googlegroups.com
Attention needed from Russ Cox and Tim King
Alan Donovan added 1 comment![]( "Open in Gerrit")
File go/analysis/passes/printf/printf.go
Line 508, Patchset 3:
formatArg := call.Args[idx]Tim King . resolved
Alan DonovanIs it worth bothering adjusted idx for method expressions? My guess is no, but I thought I'd ask.
We're analyzing a CallExpr that statically calls a function symbol, so there's no method expression here. I'm not sure what you're suggesting.
Alan Donovan
Acknowledged
Related details
Attention is currently required from:
- Russ Cox
- Tim King
Submit Requirements:
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Jul 22, 2024, 12:07:47 PM7/22/24
to goph...@pubsubhelper.golang.org, Russ Cox, Go LUCI, Robert Findley, Tim King, golang-co...@googlegroups.com
Attention needed from Russ Cox and Tim King
Alan Donovan voted Auto-Submit+1![]( "Open in Gerrit")
| Auto-Submit | +1 |
Alan Donovan (Gerrit)
Jul 22, 2024, 12:24:10 PM7/22/24
to goph...@pubsubhelper.golang.org, Go LUCI, Russ Cox, Robert Findley, Tim King, golang-co...@googlegroups.com
Attention needed from Russ Cox and Tim King
Alan Donovan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alan Donovan
Related details
Attention is currently required from:
- Russ Cox
- Tim King
Submit Requirements:
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tim King (Gerrit)
Jul 22, 2024, 12:55:03 PM7/22/24
to Alan Donovan, goph...@pubsubhelper.golang.org, Go LUCI, Russ Cox, Robert Findley, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Russ Cox
Tim King voted Code-Review+2![]( "Open in Gerrit")
Attention is currently required from:
- Alan Donovan
- Russ Cox
Submit Requirements:
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Jul 22, 2024, 12:55:24 PM7/22/24
to goph...@pubsubhelper.golang.org, Tim King, Go LUCI, Russ Cox, Robert Findley, golang-co...@googlegroups.com
Attention needed from Russ Cox
Alan Donovan added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Attention is currently required from:
- Russ Cox
Submit Requirements:
Code-Review
No-Holds
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Aug 5, 2024, 1:36:27 PM8/5/24
to goph...@pubsubhelper.golang.org, Tim King, Go LUCI, Russ Cox, Robert Findley, golang-co...@googlegroups.com
Attention needed from Russ Cox
Alan Donovan removed a vote from this change![]( "Open in Gerrit")
Removed Hold+1 by Russ Cox <r...@golang.org>
Related details
Attention is currently required from:
- Russ Cox
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gopher Robot (Gerrit)
Aug 5, 2024, 1:36:32 PM8/5/24
to Alan Donovan, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Tim King, Go LUCI, Russ Cox, Robert Findley, golang-co...@googlegroups.com
Gopher Robot submitted the change![]( "Open in Gerrit")
Change information
Commit message:
go/analysis/passes/printf: report non-constant format, no args
Calls such as fmt.Printf(s), where s is non-constant and there
are no arguments to format, are invariably a mistake. This
CL causes the printf checker to report them, and to offer
a suggested fix of fmt.Printf("%s", s).
Also:
- tests
- docs
- fixes to existing violations in x/tools (3 bugs, 2 merely bad form).
- an ignore-tagged main file for the printf checker.
Fixes golang/go#60529
Change-Id: Ic7cc4f3e7487bc1549948fa03b5b0b27959948e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/585795
Reviewed-by: Robert Findley <rfin...@google.com>
Auto-Submit: Alan Donovan <adon...@google.com>
Reviewed-by: Tim King <tak...@google.com>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Files:
- M cmd/goyacc/yacc.go
- M go/analysis/passes/printf/doc.go
- A go/analysis/passes/printf/main.go
- M go/analysis/passes/printf/printf.go
- M go/analysis/passes/printf/printf_test.go
- A go/analysis/passes/printf/testdata/src/fix/fix.go
- A go/analysis/passes/printf/testdata/src/fix/fix.go.golden
- M go/analysis/passes/tests/tests.go
Change size: M
Delta: 8 files changed, 125 insertions(+), 64 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +2 by Tim King, +2 by Robert Findley
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Aug 5, 2024, 1:40:18 PM8/5/24
to Gopher Robot, goph...@pubsubhelper.golang.org, Tim King, Go LUCI, Russ Cox, Robert Findley, golang-co...@googlegroups.com
Alan Donovan added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Alan Donovan . resolved
Russ, this is awaiting your unhold.
Alan Donovan
I just realized that I can remove your Hold, and have done so.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages