gopls/internal/highlight: DocumentHighlight for format strings

This CL introduces functionality for highlighting
printf-style directives and their associated variadic arguments.

Also fix some comments/names in CL 623156

Updates golang/go#70050

Change-Id: I1c4e6678317aa8cf522f41765f5a6600793f3746
GitHub-Last-Rev: d15a3b3
GitHub-Pull-Request: #555
Reviewed-on: https://go-review.googlesource.com/c/tools/+/642095
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: xzbdmw

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.

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)

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,
 	}

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
 }

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 {

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)
+}