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 19, 2024
1 parent b6dbcdb commit 3b8c118
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 148 deletions.
76 changes: 40 additions & 36 deletions go/analysis/passes/printf/printf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' {
Expand All @@ -513,16 +513,16 @@ 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.
if anyIndex {
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"))
}
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, maxArgIndex *int, firstArg int, name string, directive *fmtstr.FormatDirective) (ok bool) {
verb := directive.Verb.Verb
var v printVerb
found := false
Expand All @@ -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)
}
}
Expand All @@ -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 != "" {
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
39 changes: 21 additions & 18 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 @@ -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 {
Expand All @@ -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
}
}
Expand Down
Loading

0 comments on commit 3b8c118

Please sign in to comment.