diff --git a/gopls/doc/release/v0.18.0.md b/gopls/doc/release/v0.18.0.md index b785b2fae9c..155570d5a2f 100644 --- a/gopls/doc/release/v0.18.0.md +++ b/gopls/doc/release/v0.18.0.md @@ -97,3 +97,15 @@ The Definition query now supports additional locations: When invoked on a return statement, hover reports the types of the function's result variables. + +## Improvements to "DocumentHighlight" + +When your cursor is inside a printf-like function, gopls now highlights the relationship between +formatting verbs and arguments as visual cues to differentiate how operands are used in the format string. + +```go +fmt.Printf("Hello %s, you scored %d", name, score) +``` + +If the cursor is either on `%s` or `name`, gopls will highlight `%s` as a write operation, +and `name` as a read operation. diff --git a/gopls/internal/golang/codeaction.go b/gopls/internal/golang/codeaction.go index 627ba1a60d6..9a7bee7f817 100644 --- a/gopls/internal/golang/codeaction.go +++ b/gopls/internal/golang/codeaction.go @@ -335,9 +335,9 @@ func quickFix(ctx context.Context, req *codeActionsRequest) error { req.addApplyFixAction(msg, fixMissingCalledFunction, req.loc) } - // "undeclared name: x" or "undefined: x" compiler error. - // Offer a "Create variable/function x" code action. - // See [fixUndeclared] for command implementation. + // "undeclared name: X" or "undefined: X" compiler error. + // Offer a "Create variable/function X" code action. + // See [createUndeclared] for command implementation. case strings.HasPrefix(msg, "undeclared name: "), strings.HasPrefix(msg, "undefined: "): path, _ := astutil.PathEnclosingInterval(req.pgf.File, start, end) diff --git a/gopls/internal/golang/fix.go b/gopls/internal/golang/fix.go index 7e83c1d6700..e812c677541 100644 --- a/gopls/internal/golang/fix.go +++ b/gopls/internal/golang/fix.go @@ -112,7 +112,7 @@ func ApplyFix(ctx context.Context, fix string, snapshot *cache.Snapshot, fh file fixInvertIfCondition: singleFile(invertIfCondition), fixSplitLines: singleFile(splitLines), fixJoinLines: singleFile(joinLines), - fixCreateUndeclared: singleFile(CreateUndeclared), + fixCreateUndeclared: singleFile(createUndeclared), fixMissingInterfaceMethods: stubMissingInterfaceMethodsFixer, fixMissingCalledFunction: stubMissingCalledFunctionFixer, } diff --git a/gopls/internal/golang/highlight.go b/gopls/internal/golang/highlight.go index 1174ce7f7d4..096cd7b77da 100644 --- a/gopls/internal/golang/highlight.go +++ b/gopls/internal/golang/highlight.go @@ -10,12 +10,16 @@ import ( "go/ast" "go/token" "go/types" + "strconv" + "strings" + "unicode/utf8" "golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/gopls/internal/cache" "golang.org/x/tools/gopls/internal/file" "golang.org/x/tools/gopls/internal/protocol" "golang.org/x/tools/internal/event" + "golang.org/x/tools/internal/fmtstr" ) func Highlight(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, position protocol.Position) ([]protocol.DocumentHighlight, error) { @@ -49,7 +53,7 @@ func Highlight(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, po } } } - result, err := highlightPath(path, pgf.File, pkg.TypesInfo()) + result, err := highlightPath(pkg.TypesInfo(), path, pos) if err != nil { return nil, err } @@ -69,8 +73,22 @@ 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) (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 call to a printf-like function (as identified + // by a simple heuristic). + // Treat each corresponding ("%v", arg) pair as a highlight class. + for _, node := range path { + if call, ok := node.(*ast.CallExpr); ok { + lit, idx := formatStringAndIndex(info, call) + if idx != -1 { + highlightPrintf(call, idx, pos, lit, result) + } + } + } + + file := path[len(path)-1].(*ast.File) switch node := path[0].(type) { case *ast.BasicLit: // Import path string literal? @@ -131,6 +149,166 @@ func highlightPath(path []ast.Node, file *ast.File, info *types.Info) (map[posRa return result, nil } +// formatStringAndIndex returns the BasicLit and index of the BasicLit (the last +// non-variadic parameter) within the given printf-like call +// expression, returns -1 as index if unknown. +func formatStringAndIndex(info *types.Info, call *ast.CallExpr) (*ast.BasicLit, int) { + typ := info.Types[call.Fun].Type + if typ == nil { + return nil, -1 // missing type + } + sig, ok := typ.(*types.Signature) + if !ok { + return nil, -1 // ill-typed + } + if !sig.Variadic() { + // Skip checking non-variadic functions. + return nil, -1 + } + idx := sig.Params().Len() - 2 + if idx < 0 { + // Skip checking variadic functions without + // fixed arguments. + return nil, -1 + } + // We only care about literal format strings, so fmt.Sprint("a"+"b%s", "bar") won't be highlighted. + if lit, ok := call.Args[idx].(*ast.BasicLit); ok && lit.Kind == token.STRING { + return lit, idx + } + return nil, -1 +} + +// highlightPrintf highlights operations in a format string and their corresponding +// variadic arguments in a (possible) printf-style function call. +// For example: +// +// fmt.Printf("Hello %s, you scored %d", name, score) +// +// If the cursor is on %s or name, it will highlight %s as a write operation, +// and name as a read operation. +func highlightPrintf(call *ast.CallExpr, idx int, cursorPos token.Pos, lit *ast.BasicLit, result map[posRange]protocol.DocumentHighlightKind) { + format, err := strconv.Unquote(lit.Value) + if err != nil { + return + } + if !strings.Contains(format, "%") { + return + } + operations, err := fmtstr.Parse(format, idx) + if err != nil { + return + } + + // fmt.Printf("%[1]d %[1].2d", 3) + // + // When cursor is in `%[1]d`, we record `3` being successfully highlighted. + // And because we will also record `%[1].2d`'s corresponding arguments index is `3` + // in `visited`, even though it will not highlight any item in the first pass, + // in the second pass we can correctly highlight it. So the three are the same class. + succeededArg := 0 + visited := make(map[posRange]int, 0) + + // highlightPair highlights the operation and its potential argument pair if the cursor is within either range. + highlightPair := func(rang fmtstr.Range, argIndex int) { + rangeStart, err := posInStringLiteral(lit, rang.Start) + if err != nil { + return + } + rangeEnd, err := posInStringLiteral(lit, rang.End) + if err != nil { + return + } + visited[posRange{rangeStart, rangeEnd}] = argIndex + + var arg ast.Expr + if argIndex < len(call.Args) { + arg = call.Args[argIndex] + } + + // cursorPos can't equal to end position, otherwise the two + // neighborhood such as (%[2]*d) are both highlighted if cursor in "*" (ending of [2]*). + if rangeStart <= cursorPos && cursorPos < rangeEnd || + arg != nil && arg.Pos() <= cursorPos && cursorPos < arg.End() { + highlightRange(result, rangeStart, rangeEnd, protocol.Write) + if arg != nil { + succeededArg = argIndex + highlightRange(result, arg.Pos(), arg.End(), protocol.Read) + } + } + } + + for _, op := range operations { + // If width or prec has any *, we can not highlight the full range from % to verb, + // because it will overlap with the sub-range of *, for example: + // + // fmt.Printf("%*[3]d", 4, 5, 6) + // ^ ^ we can only highlight this range when cursor in 6. '*' as a one-rune range will + // highlight for 4. + hasAsterisk := false + + // Try highlight Width if there is a *. + if op.Width.Dynamic != -1 { + hasAsterisk = true + highlightPair(op.Width.Range, op.Width.Dynamic) + } + + // Try highlight Precision if there is a *. + if op.Prec.Dynamic != -1 { + hasAsterisk = true + highlightPair(op.Prec.Range, op.Prec.Dynamic) + } + + // Try highlight Verb. + if op.Verb.Verb != '%' { + // If any * is found inside operation, narrow the highlight range. + if hasAsterisk { + highlightPair(op.Verb.Range, op.Verb.ArgIndex) + } else { + highlightPair(op.Range, op.Verb.ArgIndex) + } + } + } + + // Second pass, try to highlight those missed operations. + for rang, argIndex := range visited { + if succeededArg == argIndex { + highlightRange(result, rang.start, rang.end, protocol.Write) + } + } +} + +// posInStringLiteral returns the position within a string literal +// corresponding to the specified byte offset within the logical +// string that it denotes. +func posInStringLiteral(lit *ast.BasicLit, offset int) (token.Pos, error) { + raw := lit.Value + + value, err := strconv.Unquote(raw) + if err != nil { + return 0, err + } + if !(0 <= offset && offset <= len(value)) { + return 0, fmt.Errorf("invalid offset") + } + + // remove quotes + quote := raw[0] // '"' or '`' + raw = raw[1 : len(raw)-1] + + var ( + i = 0 // byte index within logical value + pos = lit.ValuePos + 1 // position within literal + ) + for raw != "" && i < offset { + r, _, rest, _ := strconv.UnquoteChar(raw, quote) // can't fail + sz := len(raw) - len(rest) // length of literal char in raw bytes + pos += token.Pos(sz) + raw = raw[sz:] + i += utf8.RuneLen(r) + } + return pos, nil +} + type posRange struct { start, end token.Pos } diff --git a/gopls/internal/golang/undeclared.go b/gopls/internal/golang/undeclared.go index 35a5c7a1e57..0615386e9bf 100644 --- a/gopls/internal/golang/undeclared.go +++ b/gopls/internal/golang/undeclared.go @@ -68,8 +68,8 @@ func undeclaredFixTitle(path []ast.Node, errMsg string) string { return fmt.Sprintf("Create %s %s", noun, name) } -// CreateUndeclared generates a suggested declaration for an undeclared variable or function. -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) { +// createUndeclared generates a suggested declaration for an undeclared variable or function. +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) { pos := start // don't use the end path, _ := astutil.PathEnclosingInterval(file, pos, pos) if len(path) < 2 { diff --git a/gopls/internal/test/marker/testdata/highlight/highlight_printf.txt b/gopls/internal/test/marker/testdata/highlight/highlight_printf.txt new file mode 100644 index 00000000000..05fc86c0ee1 --- /dev/null +++ b/gopls/internal/test/marker/testdata/highlight/highlight_printf.txt @@ -0,0 +1,55 @@ + +This test checks functionality of the printf-like directives and operands highlight. +-- flags -- +-ignore_extra_diags +-- highlights.go -- +package highlightprintf +import ( + "fmt" +) + +func BasicPrintfHighlights() { + fmt.Printf("Hello %s, you have %d new messages!", "Alice", 5) //@hiloc(normals, "%s", write),hiloc(normalarg0, "\"Alice\"", read),highlightall(normals, normalarg0) + fmt.Printf("Hello %s, you have %d new messages!", "Alice", 5) //@hiloc(normald, "%d", write),hiloc(normalargs1, "5", read),highlightall(normald, normalargs1) +} + +func ComplexPrintfHighlights() { + fmt.Printf("Hello %#3.4s, you have %-2.3d new messages!", "Alice", 5) //@hiloc(complexs, "%#3.4s", write),hiloc(complexarg0, "\"Alice\"", read),highlightall(complexs, complexarg0) + fmt.Printf("Hello %#3.4s, you have %-2.3d new messages!", "Alice", 5) //@hiloc(complexd, "%-2.3d", write),hiloc(complexarg1, "5", read),highlightall(complexd, complexarg1) +} + +func MissingDirectives() { + fmt.Printf("Hello %s, you have 5 new messages!", "Alice", 5) //@hiloc(missings, "%s", write),hiloc(missingargs0, "\"Alice\"", read),highlightall(missings, missingargs0) +} + +func TooManyDirectives() { + fmt.Printf("Hello %s, you have %d new %s %q messages!", "Alice", 5) //@hiloc(toomanys, "%s", write),hiloc(toomanyargs0, "\"Alice\"", read),highlightall(toomanys, toomanyargs0) + fmt.Printf("Hello %s, you have %d new %s %q messages!", "Alice", 5) //@hiloc(toomanyd, "%d", write),hiloc(toomanyargs1, "5", read),highlightall(toomanyd, toomanyargs1) +} + +func VerbIsPercentage() { + fmt.Printf("%4.2% %d", 6) //@hiloc(z1, "%d", write),hiloc(z2, "6", read),highlightall(z1, z2) +} + +func SpecialChars() { + fmt.Printf("Hello \n %s, you \t \n have %d new messages!", "Alice", 5) //@hiloc(specials, "%s", write),hiloc(specialargs0, "\"Alice\"", read),highlightall(specials, specialargs0) + fmt.Printf("Hello \n %s, you \t \n have %d new messages!", "Alice", 5) //@hiloc(speciald, "%d", write),hiloc(specialargs1, "5", read),highlightall(speciald, specialargs1) +} + +func Escaped() { + fmt.Printf("Hello %% \n %s, you \t%% \n have %d new m%%essages!", "Alice", 5) //@hiloc(escapeds, "%s", write),hiloc(escapedargs0, "\"Alice\"", read),highlightall(escapeds, escapedargs0) + fmt.Printf("Hello %% \n %s, you \t%% \n have %d new m%%essages!", "Alice", 5) //@hiloc(escapedd, "%s", write),hiloc(escapedargs1, "\"Alice\"", read),highlightall(escapedd, escapedargs1) + fmt.Printf("%d \nss \x25[2]d", 234, 123) //@hiloc(zz1, "%d", write),hiloc(zz2, "234", read),highlightall(zz1,zz2) + fmt.Printf("%d \nss \x25[2]d", 234, 123) //@hiloc(zz3, "\\x25[2]d", write),hiloc(zz4, "123", read),highlightall(zz3,zz4) +} + +func Indexed() { + fmt.Printf("%[1]d", 3) //@hiloc(i1, "%[1]d", write),hiloc(i2, "3", read),highlightall(i1, i2) + fmt.Printf("%[1]*d", 3, 6) //@hiloc(i3, "[1]*", write),hiloc(i4, "3", read),hiloc(i5, "d", write),hiloc(i6, "6", read),highlightall(i3, i4),highlightall(i5, i6) + fmt.Printf("%[2]*[1]d", 3, 4) //@hiloc(i7, "[2]*", write),hiloc(i8, "4", read),hiloc(i9, "[1]d", write),hiloc(i10, "3", read),highlightall(i7, i8),highlightall(i9, i10) + fmt.Printf("%[2]*.[1]*[3]d", 4, 5, 6) //@hiloc(i11, "[2]*", write),hiloc(i12, "5", read),hiloc(i13, ".[1]*", write),hiloc(i14, "4", read),hiloc(i15, "[3]d", write),hiloc(i16, "6", read),highlightall(i11, i12),highlightall(i13, i14),highlightall(i15, i16) +} + +func MultipleIndexed() { + fmt.Printf("%[1]d %[1].2d", 3) //@hiloc(m1, "%[1]d", write),hiloc(m2, "3", read),hiloc(m3, "%[1].2d", write),highlightall(m1, m2, m3) +}