Skip to content

Commit

Permalink
resolve comment
Browse files Browse the repository at this point in the history
  • Loading branch information
@xzbdmw
xzbdmw committed Dec 16, 2024
1 parent 1ff997d commit 76b6597
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 85 deletions.
14 changes: 7 additions & 7 deletions go/analysis/passes/printf/printf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ func checkPrintf(pass *analysis.Pass, kind Kind, call *ast.CallExpr, name string
(directive.Verb != nil && directive.Verb.Index != -1) {
anyIndex = true
}
if !okPrintfArg(pass, call, &maxArgNum, name, directive) { // One error per format is enough.
if !okPrintfArg(pass, call, &maxArgNum, firstArg, name, directive) { // One error per format is enough.
return
}
if directive.Verb.Verb == 'w' {
Expand Down Expand Up @@ -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, maxArgNum *int, firstArg int, name string, directive *fmtstr.FormatDirective) (ok bool) {
verb := directive.Verb.Verb
var v printVerb
found := false
Expand Down Expand Up @@ -641,7 +641,7 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgNum *int, name s
// 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 !argCanBeChecked(pass, call, argNums[i], firstArg, directive, name) {
return
}
arg := call.Args[argNum]
Expand Down Expand Up @@ -671,7 +671,7 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, maxArgNum *int, name s

// Now check verb's type.
argNum := argNums[len(argNums)-1]
if !argCanBeChecked(pass, call, argNums[len(argNums)-1], directive, name) {
if !argCanBeChecked(pass, call, argNums[len(argNums)-1], firstArg, directive, name) {
return false
}
arg := call.Args[argNum]
Expand Down Expand Up @@ -777,7 +777,7 @@ 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 {
func argCanBeChecked(pass *analysis.Pass, call *ast.CallExpr, argNum, firstArg int, directive *fmtstr.FormatDirective, name string) bool {
if argNum <= 0 {
// Shouldn't happen, so catch it with prejudice.
panic("negative arg num")
Expand All @@ -793,8 +793,8 @@ func argCanBeChecked(pass *analysis.Pass, call *ast.CallExpr, argNum int, direct
}
// 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 := argNum - 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
}

Expand Down
7 changes: 4 additions & 3 deletions gopls/internal/golang/highlight.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)?
Expand All @@ -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?
Expand Down Expand Up @@ -172,7 +173,7 @@ func highlightPrintf(info *types.Info, call *ast.CallExpr, formatPos token.Pos,
arg = call.Args[argNum]
}

// Highlight the directive and its potential argument if the cursor is within etheir range.
// Highlight the directive and its potential argument if the cursor is within either range.
if (cursorPos >= rangeStart && cursorPos < rangeEnd) || (arg != nil && cursorPos >= arg.Pos() && cursorPos < arg.End()) {
highlightRange(result, rangeStart, rangeEnd, protocol.Write)
if arg != nil {
Expand Down
148 changes: 73 additions & 75 deletions internal/fmtstr/parse.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,67 @@
package fmtstr

import (
_ "embed"
"fmt"
"go/ast"
"go/constant"
"go/types"
"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, if any (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
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
}

// 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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -169,58 +219,6 @@ 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) {
Expand Down Expand Up @@ -254,23 +252,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.
Expand All @@ -297,19 +278,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 (
Expand Down

0 comments on commit 76b6597

Please sign in to comment.