From 3b8c1183ce00acb2f669d829909a2e37f604b72e Mon Sep 17 00:00:00 2001 From: xzb <2598514867@qq.com> Date: Tue, 17 Dec 2024 05:22:47 +0800 Subject: [PATCH] resolve comment --- go/analysis/passes/printf/printf.go | 76 ++++++------ gopls/internal/golang/highlight.go | 39 +++--- internal/fmtstr/parse.go | 184 ++++++++++++++-------------- 3 files changed, 151 insertions(+), 148 deletions(-) diff --git a/go/analysis/passes/printf/printf.go b/go/analysis/passes/printf/printf.go index abc9275fd10..f2bffb64551 100644 --- a/go/analysis/passes/printf/printf.go +++ b/go/analysis/passes/printf/printf.go @@ -492,16 +492,16 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, name string return } - maxArgNum := firstArg + maxArgIndex := firstArg anyIndex := false // Check formats against args. for _, directive := range directives { if (directive.Prec != nil && directive.Prec.Index != -1) || (directive.Width != nil && directive.Width.Index != -1) || - (directive.Verb != nil && directive.Verb.Index != -1) { + (directive.Verb.Index != -1) { anyIndex = true } - if !okPrintfArg(pass, call, &maxArgNum, name, directive) { // One error per format is enough. + if !okPrintfArg(pass, call, &maxArgIndex, firstArg, name, directive) { // One error per format is enough. return } if directive.Verb.Verb == 'w' { @@ -513,7 +513,7 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, name string } } // Dotdotdot is hard. - if call.Ellipsis.IsValid() && maxArgNum >= len(call.Args)-1 { + if call.Ellipsis.IsValid() && maxArgIndex >= len(call.Args)-1 { return } // If any formats are indexed, extra arguments are ignored. @@ -521,8 +521,8 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, name string return } // There should be no leftover arguments. - if maxArgNum != len(call.Args) { - expect := maxArgNum - firstArg + if maxArgIndex != len(call.Args) { + expect := maxArgIndex - firstArg numArgs := len(call.Args) - firstArg pass.ReportRangef(call, "%s call needs %v but has %v", name, count(expect, "arg"), count(numArgs, "arg")) } @@ -591,7 +591,7 @@ var printVerbs = []printVerb{ // okPrintfArg compares the FormatDirective to the arguments actually present, // reporting any discrepancies it can discern. If the final argument is ellipsissed, // there's little it can do for that. -func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgNum *int, name string, directive *fmtstr.FormatDirective) (ok bool) { +func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgIndex *int, firstArg int, name string, directive *fmtstr.FormatDirective) (ok bool) { verb := directive.Verb.Verb var v printVerb found := false @@ -606,8 +606,8 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgNum *int, name s // Could verb's arg implement fmt.Formatter? // Skip check for the %w verb, which requires an error. formatter := false - if v.typ != argError && directive.Verb.ArgNum < len(call.Args) { - if tv, ok := pass.TypesInfo.Types[call.Args[directive.Verb.ArgNum]]; ok { + if v.typ != argError && directive.Verb.ArgIndex < len(call.Args) { + if tv, ok := pass.TypesInfo.Types[call.Args[directive.Verb.ArgIndex]]; ok { formatter = isFormatter(tv.Type) } } @@ -629,22 +629,23 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgNum *int, name s } } } - var argNums []int - if directive.Width != nil && directive.Width.ArgNum != -1 { - argNums = append(argNums, directive.Width.ArgNum) + + var argIndexes []int + // First check for *. + if directive.Width != nil && directive.Width.ArgIndex != -1 { + argIndexes = append(argIndexes, directive.Width.ArgIndex) } - if directive.Prec != nil && directive.Prec.ArgNum != -1 { - argNums = append(argNums, directive.Prec.ArgNum) + if directive.Prec != nil && directive.Prec.ArgIndex != -1 { + argIndexes = append(argIndexes, directive.Prec.ArgIndex) } - - // Verb is good. If len(argNums)>0, we have something like %.*s and all - // args in argNums must be an integer. - for i := 0; i < len(argNums); i++ { - argNum := argNums[i] - if !argCanBeChecked(pass, call, argNums[i], directive, name) { + // If len(argIndexes)>0, we have something like %.*s and all + // indexes in argIndexes must be an integer. + for i := 0; i < len(argIndexes); i++ { + argIndex := argIndexes[i] + if !argCanBeChecked(pass, call, argIndex, firstArg, directive, name) { return } - arg := call.Args[argNum] + arg := call.Args[argIndex] if reason, ok := matchArgType(pass, argInt, arg); !ok { details := "" if reason != "" { @@ -656,25 +657,28 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgNum *int, name s } // Collect to update maxArgNum in one loop. - if directive.Verb != nil && directive.Verb.ArgNum != -1 && verb != '%' { - argNums = append(argNums, directive.Verb.ArgNum) + if directive.Verb.ArgIndex != -1 && verb != '%' { + argIndexes = append(argIndexes, directive.Verb.ArgIndex) } - for _, n := range argNums { - if n >= *maxArgNum { - *maxArgNum = n + 1 + for _, index := range argIndexes { + if index >= *maxArgIndex { + *maxArgIndex = index + 1 } } + // Special case for '%', go will print "fmt.Printf("%10.2%%dhello", 4)" + // as "%4hello", discard any runes between the two '%'s, and treat the verb '%' + // as an ordinary rune, so early return to skip the type check. if verb == '%' || formatter { return true } // Now check verb's type. - argNum := argNums[len(argNums)-1] - if !argCanBeChecked(pass, call, argNums[len(argNums)-1], directive, name) { + verbArgIndex := directive.Verb.ArgIndex + if !argCanBeChecked(pass, call, verbArgIndex, firstArg, directive, name) { return false } - arg := call.Args[argNum] + arg := call.Args[verbArgIndex] if isFunctionValue(pass, arg) && verb != 'p' && verb != 'T' { pass.ReportRangef(call, "%s format %s arg %s is a func value, not called", name, directive.Format, analysisutil.Format(pass.Fset, arg)) return false @@ -777,24 +781,24 @@ func isFunctionValue(pass *analysis.Pass, e ast.Expr) bool { // argCanBeChecked reports whether the specified argument is statically present; // it may be beyond the list of arguments or in a terminal slice... argument, which // means we can't see it. -func argCanBeChecked(pass *analysis.Pass, call *ast.CallExpr, argNum int, directive *fmtstr.FormatDirective, name string) bool { - if argNum <= 0 { +func argCanBeChecked(pass *analysis.Pass, call *ast.CallExpr, argIndex, firstArg int, directive *fmtstr.FormatDirective, name string) bool { + if argIndex <= 0 { // Shouldn't happen, so catch it with prejudice. - panic("negative arg num") + panic("negative argIndex") } - if argNum < len(call.Args)-1 { + if argIndex < len(call.Args)-1 { return true // Always OK. } if call.Ellipsis.IsValid() { return false // We just can't tell; there could be many more arguments. } - if argNum < len(call.Args) { + if argIndex < len(call.Args) { return true } // There are bad indexes in the format or there are fewer arguments than the format needs. // This is the argument number relative to the format: Printf("%s", "hi") will give 1 for the "hi". - arg := argNum - directive.FirstArg + 1 // People think of arguments as 1-indexed. - pass.ReportRangef(call, "%s format %s reads arg #%d, but call has %v", name, directive.Format, arg, count(len(call.Args)-directive.FirstArg, "arg")) + arg := argIndex - firstArg + 1 // People think of arguments as 1-indexed. + pass.ReportRangef(call, "%s format %s reads arg #%d, but call has %v", name, directive.Format, arg, count(len(call.Args)-firstArg, "arg")) return false } diff --git a/gopls/internal/golang/highlight.go b/gopls/internal/golang/highlight.go index 3aa875a7c4a..8330df4a5ec 100644 --- a/gopls/internal/golang/highlight.go +++ b/gopls/internal/golang/highlight.go @@ -51,7 +51,7 @@ func Highlight(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, po } } } - result, err := highlightPath(path, pgf.File, pkg.TypesInfo(), pos) + result, err := highlightPath(pkg.TypesInfo(), path, pos) if err != nil { return nil, err } @@ -71,7 +71,7 @@ func Highlight(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, po // highlightPath returns ranges to highlight for the given enclosing path, // which should be the result of astutil.PathEnclosingInterval. -func highlightPath(path []ast.Node, file *ast.File, info *types.Info, pos token.Pos) (map[posRange]protocol.DocumentHighlightKind, error) { +func highlightPath(info *types.Info, path []ast.Node, pos token.Pos) (map[posRange]protocol.DocumentHighlightKind, error) { result := make(map[posRange]protocol.DocumentHighlightKind) // Inside a printf-style call, printf("...%v...", arg)? @@ -88,6 +88,7 @@ func highlightPath(path []ast.Node, file *ast.File, info *types.Info, pos token. } } + file := path[len(path)-1].(*ast.File) switch node := path[0].(type) { case *ast.BasicLit: // Import path string literal? @@ -162,17 +163,17 @@ func highlightPrintf(info *types.Info, call *ast.CallExpr, formatPos token.Pos, return } - highlight := func(start, end token.Pos, argNum int) bool { + // highlightPair highlights the directive and its potential argument pair if the cursor is within either range. + highlightPair := func(start, end token.Pos, argIndex int) bool { var ( rangeStart = formatPos + token.Pos(start) + 1 // add offset for leading '"' rangeEnd = formatPos + token.Pos(end) + 1 // add offset for leading '"' arg ast.Expr // may not exist ) - if len(call.Args) > argNum { - arg = call.Args[argNum] + if len(call.Args) > argIndex { + arg = call.Args[argIndex] } - // Highlight the directive and its potential argument if the cursor is within etheir range. if (cursorPos >= rangeStart && cursorPos < rangeEnd) || (arg != nil && cursorPos >= arg.Pos() && cursorPos < arg.End()) { highlightRange(result, rangeStart, rangeEnd, protocol.Write) if arg != nil { @@ -192,26 +193,28 @@ func highlightPrintf(info *types.Info, call *ast.CallExpr, formatPos token.Pos, anyAsterisk := false for _, directive := range directives { width, prec, verb := directive.Width, directive.Prec, directive.Verb - if (prec != nil && prec.Kind != fmtstr.Literal) || - (width != nil && width.Kind != fmtstr.Literal) { + // Try highlight Width if there is a *. + if width != nil && width.ArgIndex != -1 { anyAsterisk = true + if highlightPair(token.Pos(width.Range.Start), token.Pos(width.Range.End), width.ArgIndex) { + return + } } - // Try highlight Width. - if width != nil && width.ArgNum != -1 && highlight(token.Pos(width.Range.Start), token.Pos(width.Range.End), width.ArgNum) { - return - } - - // Try highlight Prec. - if prec != nil && prec.ArgNum != -1 && highlight(token.Pos(prec.Range.Start), token.Pos(prec.Range.End), prec.ArgNum) { - return + // Try highlight Precision if there is a *. + if prec != nil && prec.ArgIndex != -1 { + anyAsterisk = true + if highlightPair(token.Pos(prec.Range.Start), token.Pos(prec.Range.End), prec.ArgIndex) { + return + } } // Try highlight Verb. if verb.Verb != '%' { - if anyAsterisk && highlight(token.Pos(verb.Range.Start), token.Pos(verb.Range.End), verb.ArgNum) { + // If any * is found inside directive, narrow the highlight range. + if anyAsterisk && highlightPair(token.Pos(verb.Range.Start), token.Pos(verb.Range.End), verb.ArgIndex) { return - } else if highlight(token.Pos(directive.Range.Start), token.Pos(directive.Range.End), verb.ArgNum) { + } else if highlightPair(token.Pos(directive.Range.Start), token.Pos(directive.Range.End), verb.ArgIndex) { return } } diff --git a/internal/fmtstr/parse.go b/internal/fmtstr/parse.go index c24cd7eec46..ee15f28cb6a 100644 --- a/internal/fmtstr/parse.go +++ b/internal/fmtstr/parse.go @@ -5,7 +5,6 @@ package fmtstr import ( - _ "embed" "fmt" "go/ast" "go/constant" @@ -13,9 +12,60 @@ import ( "strconv" "strings" "unicode/utf8" - // "golang.org/x/tools/go/analysis/passes/internal/analysisutil" ) +// FormatDirective holds the parsed representation of a printf directive such as "%3.*[4]d". +// It is constructed by [ParsePrintf]. +type FormatDirective struct { + Format string // Full directive, e.g. "%[2]*.3d" + Range posRange // The range of Format within the overall format string + Flags []byte // Formatting flags, e.g. ['-', '0'] + Width *DirectiveSize // Width specifier, if any (e.g., '3' in '%3d') + Prec *DirectiveSize // Precision specifier, if any (e.g., '.4' in '%.4f') + Verb DirectiveVerb // Verb specifier, guaranteed to exist (e.g., '[1]d' in '%[1]d') + + // Parsing state (not used after parsing): + firstArg int // Index of the first argument after the format string + call *ast.CallExpr + argNum int // Which argument we're expecting to format now. + hasIndex bool // Whether the argument is indexed. + index int // The encountered index + indexPos int // The encountered index's offset + indexPending bool // Whether we have an indexed argument that has not resolved. + nbytes int // number of bytes of the format string consumed. +} + +type SizeKind int + +const ( + Literal SizeKind = iota // A literal number, e.g. "4" in "%4d" + Asterisk // A dynamic size from an argument, e.g. "%*d" + IndexedAsterisk // A dynamic size with an explicit index, e.g. "%[2]*d" +) + +// DirectiveSize describes a width or precision in a format directive. +// Depending on Kind, it may represent a literal number, a asterisk, or an indexed asterisk. +type DirectiveSize struct { + Kind SizeKind // Type of size specifier + Range posRange // Position of the size specifier within the directive + Size int // The literal size if Kind == Literal, otherwise -1 + Index int // If Kind == IndexedAsterisk, the argument index used to obtain the size, otherwise -1 + ArgIndex int // The argument index if Kind == Asterisk or IndexedAsterisk relative to CallExpr, otherwise -1 +} + +// DirectiveVerb represents the verb character of a format directive (e.g., 'd', 's', 'f'). +// It also includes positional information and any explicit argument indexing. +type DirectiveVerb struct { + Verb rune + Range posRange // The positional range of the verb in the format string + Index int // If the verb uses an indexed argument, this is the index, otherwise -1 + ArgIndex int // The argument index associated with this verb, relative to CallExpr, otherwise -1 +} + +type posRange struct { + Start, End int +} + // ParsePrintf takes a printf-like call expression, // extracts the format string, and parses out all format directives. Each // directive describes flags, width, precision, verb, and argument indexing. @@ -28,7 +78,7 @@ import ( func ParsePrintf(info *types.Info, call *ast.CallExpr) ([]*FormatDirective, error) { idx := FormatStringIndex(info, call) if idx < 0 || idx >= len(call.Args) { - return nil, fmt.Errorf("%s", "can't parse") + return nil, fmt.Errorf("not a valid printf-like call") } format, ok := StringConstantExpr(info, call.Args[idx]) if !ok { @@ -73,7 +123,7 @@ func parsePrintfVerb(call *ast.CallExpr, format string, firstArg, argNum int) (* state := &FormatDirective{ Format: format, Flags: make([]byte, 0, 5), - FirstArg: firstArg, + firstArg: firstArg, call: call, argNum: argNum, hasIndex: false, @@ -105,26 +155,26 @@ func parsePrintfVerb(call *ast.CallExpr, format string, firstArg, argNum int) (* } verb, w := utf8.DecodeRuneInString(state.Format[state.nbytes:]) - // Ensure there msut be a verb. + // Ensure there must be a verb. if state.indexPending { - state.Verb = &DirectiveVerb{ + state.Verb = DirectiveVerb{ Verb: verb, Range: posRange{ Start: state.indexPos, End: state.nbytes + w, }, - Index: state.index, - ArgNum: state.argNum, + Index: state.index, + ArgIndex: state.argNum, } } else { - state.Verb = &DirectiveVerb{ + state.Verb = DirectiveVerb{ Verb: verb, Range: posRange{ Start: state.nbytes, End: state.nbytes + w, }, - Index: -1, - ArgNum: state.argNum, + Index: -1, + ArgIndex: state.argNum, } } @@ -169,68 +219,14 @@ func StringConstantExpr(info *types.Info, expr ast.Expr) (string, bool) { return "", false } -// FormatDirective holds the parsed representation of a printf directive such as "%3.*[4]d". -// It is constructed by [ParsePrintf]. -type FormatDirective struct { - Format string // Full directive, e.g. "%[2]*.3d" - Range posRange // The range of Format within the overall format string - FirstArg int // Index of the first argument after the format string - Flags []byte // Formatting flags, e.g. ['-', '0'] - Width *DirectiveSize // Width specifier, if any (e.g., '3' in '%3d') - Prec *DirectiveSize // Precision specifier, if any (e.g., '.4' in '%.4f') - Verb *DirectiveVerb // Verb specifier, if any (e.g., '[1]d' in '%[1]d') - - // Parsing state (not used after parsing): - call *ast.CallExpr - argNum int - hasIndex bool - index int - indexPos int - indexPending bool - nbytes int -} - -type SizeKind int - -const ( - Literal SizeKind = iota // A literal number, e.g. "4" in "%4d" - Asterisk // A dynamic size from an argument, e.g. "%*d" - IndexedAsterisk // A dynamic size with an explicit index, e.g. "%[2]*d" -) - -// DirectiveSize describes a width or precision in a format directive. -// Depending on Kind, it may represent a literal number, a asterisk, or an indexed asterisk. -type DirectiveSize struct { - Kind SizeKind // Type of size specifier - Range posRange // Position of the size specifier within the directive - Size int // The literal size if Kind == Literal, otherwise -1 - Index int // If Kind == IndexedAsterisk, the argument index used to obtain the size - ArgNum int // The argument position if Kind == Asterisk or IndexedAsterisk, relative to CallExpr. -} - -// DirectiveVerb represents the verb character of a format directive (e.g., 'd', 's', 'f'). -// It also includes positional information and any explicit argument indexing. -type DirectiveVerb struct { - Verb rune - Range posRange // The positional range of the verb in the format string - Index int // If the verb uses an indexed argument, this is the index, otherwise -1 - ArgNum int // The argument position associated with this verb, relative to CallExpr. -} - -type posRange struct { - Start, End int -} - // addOffset adjusts the recorded positions in Verb, Width, Prec, and the // directive's overall Range to be relative to the position in the full format string. func (s *FormatDirective) addOffset(parsedLen int) { - if s.Verb != nil { - s.Verb.Range.Start += parsedLen - s.Verb.Range.End += parsedLen + s.Verb.Range.Start += parsedLen + s.Verb.Range.End += parsedLen - s.Range.Start = parsedLen - s.Range.End = s.Verb.Range.End - } + s.Range.Start = parsedLen + s.Range.End = s.Verb.Range.End if s.Prec != nil { s.Prec.Range.Start += parsedLen s.Prec.Range.End += parsedLen @@ -254,23 +250,6 @@ func (s *FormatDirective) parseFlags() { } } -// scanNum advances through a decimal number if present, which represents a [Size] or [Index]. -func (s *FormatDirective) scanNum() (int, bool) { - start := s.nbytes - for ; s.nbytes < len(s.Format); s.nbytes++ { - c := s.Format[s.nbytes] - if c < '0' || '9' < c { - if start < s.nbytes { - num, _ := strconv.ParseInt(s.Format[start:s.nbytes], 10, 32) - return int(num), true - } else { - return 0, false - } - } - } - return 0, false -} - // parseIndex parses an argument index of the form "[n]" that can appear // in a printf directive (e.g., "%[2]d"). Returns an error if syntax is // malformed or index is invalid. @@ -297,19 +276,36 @@ func (s *FormatDirective) parseIndex() error { s.nbytes = s.nbytes + start } arg32, err := strconv.ParseInt(s.Format[start:s.nbytes], 10, 32) - if err != nil || !ok || arg32 <= 0 || arg32 > int64(len(s.call.Args)-s.FirstArg) { + if err != nil || !ok || arg32 <= 0 || arg32 > int64(len(s.call.Args)-s.firstArg) { return fmt.Errorf("format has invalid argument index [%s]", s.Format[start:s.nbytes]) } s.nbytes++ // skip ']' arg := int(arg32) - arg += s.FirstArg - 1 // We want to zero-index the actual arguments. + arg += s.firstArg - 1 // We want to zero-index the actual arguments. s.argNum = arg s.hasIndex = true s.indexPending = true return nil } +// scanNum advances through a decimal number if present, which represents a [Size] or [Index]. +func (s *FormatDirective) scanNum() (int, bool) { + start := s.nbytes + for ; s.nbytes < len(s.Format); s.nbytes++ { + c := s.Format[s.nbytes] + if c < '0' || '9' < c { + if start < s.nbytes { + num, _ := strconv.ParseInt(s.Format[start:s.nbytes], 10, 32) + return int(num), true + } else { + return 0, false + } + } + } + return 0, false +} + type sizeType int const ( @@ -334,8 +330,8 @@ func (s *FormatDirective) parseSize(kind sizeType) { Start: s.indexPos, End: s.nbytes, }, - Index: s.index, - ArgNum: s.argNum, + Index: s.index, + ArgIndex: s.argNum, } switch kind { case Width: @@ -356,8 +352,8 @@ func (s *FormatDirective) parseSize(kind sizeType) { Start: s.nbytes - 1, End: s.nbytes, }, - Index: -1, - ArgNum: s.argNum, + Index: -1, + ArgIndex: s.argNum, } switch kind { case Width: @@ -381,8 +377,8 @@ func (s *FormatDirective) parseSize(kind sizeType) { Start: start, End: s.nbytes, }, - Index: -1, - ArgNum: -1, + Index: -1, + ArgIndex: -1, } switch kind { case Width: