Skip to content

Commit cfdd15e

Browse files
committed
use -1 defualt value
1 parent 739df3d commit cfdd15e

File tree

5 files changed

+96
-84
lines changed

5 files changed

+96
-84
lines changed

go/analysis/passes/printf/printf.go

Lines changed: 37 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
package printf
66

77
import (
8-
"bytes"
98
_ "embed"
109
"fmt"
1110
"go/ast"
@@ -371,6 +370,18 @@ var isPrint = stringSet{
371370
"(testing.TB).Skipf": true,
372371
}
373372

373+
// stringConstantExpr returns expression's string constant value.
374+
//
375+
// ("", false) is returned if expression isn't a string
376+
// constant.
377+
func stringConstantExpr(pass *analysis.Pass, expr ast.Expr) (string, bool) {
378+
lit := pass.TypesInfo.Types[expr].Value
379+
if lit != nil && lit.Kind() == constant.String {
380+
return constant.StringVal(lit), true
381+
}
382+
return "", false
383+
}
384+
374385
// checkCall triggers the print-specific checks if the call invokes a print function.
375386
func checkCall(pass *analysis.Pass) {
376387
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
@@ -446,10 +457,10 @@ func isFormatter(typ types.Type) bool {
446457
types.Identical(sig.Params().At(1).Type(), types.Typ[types.Rune])
447458
}
448459

449-
// FormatStringIndex returns the index of the format string (the last
460+
// formatStringIndex returns the index of the format string (the last
450461
// non-variadic parameter) within the given printf-like call
451462
// expression, or -1 if unknown.
452-
func FormatStringIndex(info *types.Info, call *ast.CallExpr) int {
463+
func formatStringIndex(info *types.Info, call *ast.CallExpr) int {
453464
typ := info.Types[call.Fun].Type
454465
if typ == nil {
455466
return -1 // missing type
@@ -473,12 +484,12 @@ func FormatStringIndex(info *types.Info, call *ast.CallExpr) int {
473484

474485
// checkPrintf checks a call to a formatted print routine such as Printf.
475486
func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, name string) {
476-
idx := FormatStringIndex(pass.TypesInfo, call)
487+
idx := formatStringIndex(pass.TypesInfo, call)
477488
if idx < 0 || idx >= len(call.Args) {
478489
return
479490
}
480491
formatArg := call.Args[idx]
481-
format, ok := stringConstantExpr(pass.TypesInfo, formatArg)
492+
format, ok := stringConstantExpr(pass, formatArg)
482493
if !ok {
483494
// Format string argument is non-constant.
484495

@@ -518,20 +529,25 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, name string
518529
// "fmt.Sprintf call needs 1 arg but has 2 args".
519530
operations, err := fmtstr.Parse(format, idx)
520531
if err != nil {
532+
// All error messages are in predicate form ("call has a problem")
533+
// so that they may be affixed into a subject ("log.Printf ").
534+
// See https://go-review.googlesource.com/c/tools/+/632598/comment/9d980373_e6460abf/
521535
pass.ReportRangef(call.Args[idx], "%s %s", name, err)
522536
return
523537
}
524538

525-
maxArgIndex := firstArg
539+
// index of the highest used index.
540+
maxArgIndex := firstArg - 1
526541
anyIndex := false
527542
// Check formats against args.
528543
for _, operation := range operations {
529-
if operation.Prec != nil && operation.Prec.Index != -1 ||
530-
operation.Width != nil && operation.Width.Index != -1 ||
544+
if operation.Prec.Index != -1 ||
545+
operation.Width.Index != -1 ||
531546
operation.Verb.Index != -1 {
532547
anyIndex = true
533548
}
534-
if !okPrintfArg(pass, call, &maxArgIndex, firstArg, name, operation) { // One error per format is enough.
549+
if !okPrintfArg(pass, call, &maxArgIndex, firstArg, name, operation) {
550+
// One error per format is enough.
535551
return
536552
}
537553
if operation.Verb.Verb == 'w' {
@@ -543,16 +559,16 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, name string
543559
}
544560
}
545561
// Dotdotdot is hard.
546-
if call.Ellipsis.IsValid() && maxArgIndex >= len(call.Args)-1 {
562+
if call.Ellipsis.IsValid() && maxArgIndex >= len(call.Args)-2 {
547563
return
548564
}
549565
// If any formats are indexed, extra arguments are ignored.
550566
if anyIndex {
551567
return
552568
}
553569
// There should be no leftover arguments.
554-
if maxArgIndex != len(call.Args) {
555-
expect := maxArgIndex - firstArg
570+
if maxArgIndex+1 < len(call.Args) {
571+
expect := maxArgIndex + 1 - firstArg
556572
numArgs := len(call.Args) - firstArg
557573
pass.ReportRangef(call, "%s call needs %v but has %v", name, count(expect, "arg"), count(numArgs, "arg"))
558574
}
@@ -619,8 +635,8 @@ var printVerbs = []printVerb{
619635
}
620636

621637
// okPrintfArg compares the operation to the arguments actually present,
622-
// reporting any discrepancies it can discern. If the final argument is ellipsissed,
623-
// there's little it can do for that.
638+
// reporting any discrepancies it can discern, maxArgIndex was the index of the highest used index.
639+
// If the final argument is ellipsissed, there's little it can do for that.
624640
func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgIndex *int, firstArg int, name string, operation *fmtstr.Operation) (ok bool) {
625641
verb := operation.Verb.Verb
626642
var v printVerb
@@ -662,16 +678,15 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgIndex *int, firs
662678

663679
var argIndexes []int
664680
// First check for *.
665-
if operation.Width != nil && operation.Width.Dynamic != -1 {
681+
if operation.Width.Dynamic != -1 {
666682
argIndexes = append(argIndexes, operation.Width.Dynamic)
667683
}
668-
if operation.Prec != nil && operation.Prec.Dynamic > 0 {
684+
if operation.Prec.Dynamic != -1 {
669685
argIndexes = append(argIndexes, operation.Prec.Dynamic)
670686
}
671687
// If len(argIndexes)>0, we have something like %.*s and all
672688
// indexes in argIndexes must be an integer.
673-
for i := 0; i < len(argIndexes); i++ {
674-
argIndex := argIndexes[i]
689+
for _, argIndex := range argIndexes {
675690
if !argCanBeChecked(pass, call, argIndex, firstArg, operation, name) {
676691
return
677692
}
@@ -691,9 +706,7 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgIndex *int, firs
691706
argIndexes = append(argIndexes, operation.Verb.ArgIndex)
692707
}
693708
for _, index := range argIndexes {
694-
if index >= *maxArgIndex {
695-
*maxArgIndex = index + 1
696-
}
709+
*maxArgIndex = max(*maxArgIndex, index)
697710
}
698711

699712
// Special case for '%', go will print "fmt.Printf("%10.2%%dhello", 4)"
@@ -725,7 +738,7 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgIndex *int, firs
725738
pass.ReportRangef(call, "%s format %s has arg %s of wrong type %s%s", name, operation.Text, analysisutil.Format(pass.Fset, arg), typeString, details)
726739
return false
727740
}
728-
if v.typ&argString != 0 && v.verb != 'T' && !bytes.Contains(operation.Flags, []byte{'#'}) {
741+
if v.typ&argString != 0 && v.verb != 'T' && !strings.Contains(operation.Flags, "#") {
729742
if methodName, ok := recursiveStringer(pass, arg); ok {
730743
pass.ReportRangef(call, "%s format %s with arg %s causes recursive %s method call", name, operation.Text, analysisutil.Format(pass.Fset, arg), methodName)
731744
return false
@@ -886,7 +899,7 @@ func checkPrint(pass *analysis.Pass, call *ast.CallExpr, name string) {
886899
}
887900

888901
arg := args[0]
889-
if s, ok := stringConstantExpr(pass.TypesInfo, arg); ok {
902+
if s, ok := stringConstantExpr(pass, arg); ok {
890903
// Ignore trailing % character
891904
// The % in "abc 0.0%" couldn't be a formatting directive.
892905
s = strings.TrimSuffix(s, "%")
@@ -900,7 +913,7 @@ func checkPrint(pass *analysis.Pass, call *ast.CallExpr, name string) {
900913
if strings.HasSuffix(name, "ln") {
901914
// The last item, if a string, should not have a newline.
902915
arg = args[len(args)-1]
903-
if s, ok := stringConstantExpr(pass.TypesInfo, arg); ok {
916+
if s, ok := stringConstantExpr(pass, arg); ok {
904917
if strings.HasSuffix(s, "\n") {
905918
pass.ReportRangef(call, "%s arg list ends with redundant newline", name)
906919
}
@@ -916,17 +929,6 @@ func checkPrint(pass *analysis.Pass, call *ast.CallExpr, name string) {
916929
}
917930
}
918931

919-
// StringConstantExpr returns expression's string constant value.
920-
//
921-
// ("", false) is returned if expression isn't a string constant.
922-
func stringConstantExpr(info *types.Info, expr ast.Expr) (string, bool) {
923-
lit := info.Types[expr].Value
924-
if lit != nil && lit.Kind() == constant.String {
925-
return constant.StringVal(lit), true
926-
}
927-
return "", false
928-
}
929-
930932
// count(n, what) returns "1 what" or "N whats"
931933
// (assuming the plural of what is whats).
932934
func count(n int, what string) string {

gopls/internal/golang/codeaction.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -335,9 +335,9 @@ func quickFix(ctx context.Context, req *codeActionsRequest) error {
335335
req.addApplyFixAction(msg, fixMissingCalledFunction, req.loc)
336336
}
337337

338-
// "undeclared name: X" or "undefined: X" compiler error.
339-
// Offer a "Create variable/function X" code action.
340-
// See [createUndeclared] for command implementation.
338+
// "undeclared name: x" or "undefined: x" compiler error.
339+
// Offer a "Create variable/function x" code action.
340+
// See [fixUndeclared] for command implementation.
341341
case strings.HasPrefix(msg, "undeclared name: "),
342342
strings.HasPrefix(msg, "undefined: "):
343343
path, _ := astutil.PathEnclosingInterval(req.pgf.File, start, end)

gopls/internal/golang/fix.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func ApplyFix(ctx context.Context, fix string, snapshot *cache.Snapshot, fh file
112112
fixInvertIfCondition: singleFile(invertIfCondition),
113113
fixSplitLines: singleFile(splitLines),
114114
fixJoinLines: singleFile(joinLines),
115-
fixCreateUndeclared: singleFile(createUndeclared),
115+
fixCreateUndeclared: singleFile(CreateUndeclared),
116116
fixMissingInterfaceMethods: stubMissingInterfaceMethodsFixer,
117117
fixMissingCalledFunction: stubMissingCalledFunctionFixer,
118118
}

gopls/internal/golang/undeclared.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ func undeclaredFixTitle(path []ast.Node, errMsg string) string {
6868
return fmt.Sprintf("Create %s %s", noun, name)
6969
}
7070

71-
// createUndeclared generates a suggested declaration for an undeclared variable or function.
72-
func createUndeclared(fset *token.FileSet, start, end token.Pos, content []byte, file *ast.File, pkg *types.Package, info *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
71+
// CreateUndeclared generates a suggested declaration for an undeclared variable or function.
72+
func CreateUndeclared(fset *token.FileSet, start, end token.Pos, content []byte, file *ast.File, pkg *types.Package, info *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
7373
pos := start // don't use the end
7474
path, _ := astutil.PathEnclosingInterval(file, pos, pos)
7575
if len(path) < 2 {

internal/fmtstr/parse.go

Lines changed: 53 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -16,44 +16,32 @@ import (
1616
// Operation holds the parsed representation of a printf operation such as "%3.*[4]d".
1717
// It is constructed by [Parse].
1818
type Operation struct {
19-
Text string // Full text of the operation, e.g. "%[2]*.3d"
20-
Verb Verb // Verb specifier, guaranteed to exist, e.g., 'd' in '%[1]d'
21-
Range Range // The range of Text within the overall format string
22-
Flags []byte // Formatting flags, e.g. ['-', '0']
23-
Width *Size // Width specifier, e.g., '3' in '%3d'
24-
Prec *Size // Precision specifier, e.g., '.4' in '%.4f'
19+
Text string // full text of the operation, e.g. "%[2]*.3d"
20+
Verb Verb // verb specifier, guaranteed to exist, e.g., 'd' in '%[1]d'
21+
Range Range // the range of Text within the overall format string
22+
Flags string // formatting flags, e.g. "-0"
23+
Width Size // width specifier, e.g., '3' in '%3d'
24+
Prec Size // precision specifier, e.g., '.4' in '%.4f'
2525
}
2626

27-
// Internal parsing state to operation.
28-
type state struct {
29-
operation *Operation
30-
firstArg int // Index of the first argument after the format string
31-
argNum int // Which argument we're expecting to format now.
32-
hasIndex bool // Whether the argument is indexed.
33-
index int // The encountered index
34-
indexPos int // The encountered index's offset
35-
indexPending bool // Whether we have an indexed argument that has not resolved.
36-
nbytes int // number of bytes of the format string consumed.
37-
}
38-
39-
// Size describes a width or precision in a format operation.
40-
// It may represent a literal number, an asterisk, or an indexed asterisk.
27+
// Size describes an optional width or precision in a format operation.
28+
// It may represent no value, a literal number, an asterisk, or an indexed asterisk.
4129
type Size struct {
4230
// At most one of these two fields is non-negative.
43-
Fixed int // e.g. 4 from "%4d", otherwise -1.
44-
Dynamic int // index of argument providing dynamic size (e.g. %*d or %[3]*d), otherwise -1.
31+
Fixed int // e.g. 4 from "%4d", otherwise -1
32+
Dynamic int // index of argument providing dynamic size (e.g. %*d or %[3]*d), otherwise -1
4533

46-
Index int // If the width or precision uses an indexed argument (e.g. 2 in %[2]*d), this is the index, otherwise -1.
47-
Range Range // Position of the size specifier within the operation
34+
Index int // If the width or precision uses an indexed argument (e.g. 2 in %[2]*d), this is the index, otherwise -1
35+
Range Range // position of the size specifier within the operation
4836
}
4937

5038
// Verb represents the verb character of a format operation (e.g., 'd', 's', 'f').
5139
// It also includes positional information and any explicit argument indexing.
5240
type Verb struct {
5341
Verb rune
54-
Range Range // The positional range of the verb in the format string
55-
Index int // The index of an indexed argument, (e.g. 2 in %[2]d), otherwise -1.
56-
ArgIndex int // The argument index (0-based) associated with this verb, relative to CallExpr.
42+
Range Range // positional range of the verb in the format string
43+
Index int // index of an indexed argument, (e.g. 2 in %[2]d), otherwise -1
44+
ArgIndex int // argument index (0-based) associated with this verb, relative to CallExpr
5745
}
5846

5947
// byte offsets of format string
@@ -105,6 +93,18 @@ func Parse(format string, idx int) ([]*Operation, error) {
10593
return operations, nil
10694
}
10795

96+
// Internal parsing state to operation.
97+
type state struct {
98+
operation *Operation
99+
firstArg int // index of the first argument after the format string
100+
argNum int // which argument we're expecting to format now
101+
hasIndex bool // whether the argument is indexed
102+
index int // the encountered index
103+
indexPos int // the encountered index's offset
104+
indexPending bool // whether we have an indexed argument that has not resolved
105+
nbytes int // number of bytes of the format string consumed
106+
}
107+
108108
// parseOperation parses one format operation starting at the given substring `format`,
109109
// which should begin with '%'. It returns a fully populated state or an error
110110
// if the operation is malformed. The firstArg and argNum parameters help determine how
@@ -114,8 +114,17 @@ func Parse(format string, idx int) ([]*Operation, error) {
114114
func parseOperation(format string, firstArg, argNum int) (*state, error) {
115115
state := &state{
116116
operation: &Operation{
117-
Text: format,
118-
Flags: make([]byte, 0),
117+
Text: format,
118+
Width: Size{
119+
Fixed: -1,
120+
Dynamic: -1,
121+
Index: -1,
122+
},
123+
Prec: Size{
124+
Fixed: -1,
125+
Dynamic: -1,
126+
Index: -1,
127+
},
119128
},
120129
firstArg: firstArg,
121130
argNum: argNum,
@@ -184,27 +193,28 @@ func (s *Operation) addOffset(parsedLen int) {
184193

185194
s.Range.Start = parsedLen
186195
s.Range.End = s.Verb.Range.End
187-
if s.Prec != nil {
196+
197+
// one of Fixed or Dynamic is non-negative means existence.
198+
if s.Prec.Fixed == -1 && s.Prec.Dynamic == -1 {
188199
s.Prec.Range.Start += parsedLen
189200
s.Prec.Range.End += parsedLen
190201
}
191-
if s.Width != nil {
202+
if s.Width.Fixed == -1 && s.Width.Dynamic == -1 {
192203
s.Width.Range.Start += parsedLen
193204
s.Width.Range.End += parsedLen
194205
}
195206
}
196207

197208
// parseFlags accepts any printf flags.
198209
func (s *state) parseFlags() {
199-
for s.nbytes < len(s.operation.Text) {
200-
switch c := s.operation.Text[s.nbytes]; c {
201-
case '#', '0', '+', '-', ' ':
202-
s.operation.Flags = append(s.operation.Flags, c)
203-
s.nbytes++
204-
default:
205-
return
206-
}
207-
}
210+
s.operation.Flags = prefixOf(s.operation.Text[s.nbytes:], "#0+- ")
211+
s.nbytes += len(s.operation.Flags)
212+
}
213+
214+
// prefixOf returns the prefix of s composed only of runes from the specified set.
215+
func prefixOf(s, set string) string {
216+
rest := strings.TrimLeft(s, set)
217+
return s[:len(s)-len(rest)]
208218
}
209219

210220
// parseIndex parses an argument index of the form "[n]" that can appear
@@ -278,7 +288,7 @@ func (s *state) parseSize(kind sizeType) {
278288
if s.indexPending {
279289
// Absorb it.
280290
s.indexPending = false
281-
size := &Size{
291+
size := Size{
282292
Fixed: -1,
283293
Dynamic: s.argNum,
284294
Index: s.index,
@@ -299,7 +309,7 @@ func (s *state) parseSize(kind sizeType) {
299309
}
300310
} else {
301311
// Non-indexed asterisk: "%*d".
302-
size := &Size{
312+
size := Size{
303313
Dynamic: s.argNum,
304314
Index: -1,
305315
Fixed: -1,
@@ -323,7 +333,7 @@ func (s *state) parseSize(kind sizeType) {
323333
} else { // Literal number, e.g. "%10d"
324334
start := s.nbytes
325335
if num, ok := s.scanNum(); ok {
326-
size := &Size{
336+
size := Size{
327337
Fixed: num,
328338
Index: -1,
329339
Dynamic: -1,

0 commit comments

Comments
 (0)