internal/lsp/source: improve completion of printf operands
We now rank printf operand candidates according to the corresponding
formatting verb. We follow what fmt allows for the most part, but I
omitted some things because they are uncommon and would result in many
false positives, or didn't seem worth it to support:
- We don't prefer fmt.Stringer or error types for "%x" or "%X".
- We don't prefer pointers for any verbs except "%p".
- We don't prefer recursive application of verbs (e.g. we won't prefer
[]string for "%s").
I decided against sharing code with the printf analyzer. It was
tangled somewhat with go/analysis, and I needed only a very small
subset of the format parsing.
I tweaked candidate type evaluation to accommodate the printf hints.
We now skip expected type of "interface{}" when matching candidate
types because it matches everything and would always supersede the
coarser object kind checks.
Fixes golang/go#40485.
Change-Id: I6440702e33d5ec85d701f8be65453044b5dab746
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246699
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index 2955af6..639e6a6 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -1545,12 +1545,21 @@
type objKind int
const (
+ kindAny objKind = 0
kindArray objKind = 1 << iota
kindSlice
kindChan
kindMap
kindStruct
kindString
+ kindInt
+ kindBool
+ kindBytes
+ kindPtr
+ kindFloat
+ kindComplex
+ kindError
+ kindStringer
kindFunc
)
@@ -1749,6 +1758,9 @@
inf.objType = deslice(inf.objType)
// Record whether we are completing the initial variadic param.
inf.variadic = exprIdx == numParams-1 && len(node.Args) <= numParams
+
+ // Check if we can infer object kind from printf verb.
+ inf.objKind |= printfArgKind(c.pkg.GetTypesInfo(), node, exprIdx)
}
}
}
@@ -2288,6 +2300,7 @@
)
if ci.objType != nil {
expTypes = append(expTypes, ci.objType)
+
if ci.variadic {
variadicType = types.NewSlice(ci.objType)
expTypes = append(expTypes, variadicType)
@@ -2305,31 +2318,11 @@
return true
}
- if len(expTypes) == 0 {
- // If we have no expected type but were able to apply type
- // modifiers to our candidate type, count that as a match. This
- // handles cases like:
- //
- // var foo chan int
- // <-fo<>
- //
- // There is no exected type at "<>", but we were able to apply
- // the "<-" type modifier to "foo", so it matches.
- if len(ci.modifiers) > 0 {
- return true
- }
-
- // If we have no expected type, fall back to checking the
- // expected "kind" of object, if available.
- if ci.kindMatches(candType) {
- if ci.objKind == kindFunc {
- cand.expandFuncCall = true
- }
- return true
- }
- }
-
for _, expType := range expTypes {
+ if isEmptyInterface(expType) {
+ continue
+ }
+
matches, untyped := ci.typeMatches(expType, candType)
if !matches {
continue
@@ -2349,6 +2342,31 @@
return true
}
+ // If we don't have a specific expected type, fall back to coarser
+ // object kind checks.
+ if ci.objType == nil || isEmptyInterface(ci.objType) {
+ // If we were able to apply type modifiers to our candidate type,
+ // count that as a match. For example:
+ //
+ // var foo chan int
+ // <-fo<>
+ //
+ // We were able to apply the "<-" type modifier to "foo", so "foo"
+ // matches.
+ if len(ci.modifiers) > 0 {
+ return true
+ }
+
+ // If we didn't have an exact type match, check if our object kind
+ // matches.
+ if ci.kindMatches(candType) {
+ if ci.objKind == kindFunc {
+ cand.expandFuncCall = true
+ }
+ return true
+ }
+ }
+
return false
})
}
@@ -2382,7 +2400,7 @@
// kindMatches reports whether candType's kind matches our expected
// kind (e.g. slice, map, etc.).
func (ci *candidateInference) kindMatches(candType types.Type) bool {
- return ci.objKind&candKind(candType) > 0
+ return ci.objKind > 0 && ci.objKind&candKind(candType) > 0
}
// assigneesMatch reports whether an invocation of sig matches the
@@ -2508,30 +2526,74 @@
return false
}
+var (
+ // "interface { Error() string }" (i.e. error)
+ errorIntf = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)
+
+ // "interface { String() string }" (i.e. fmt.Stringer)
+ stringerIntf = types.NewInterfaceType([]*types.Func{
+ types.NewFunc(token.NoPos, nil, "String", types.NewSignature(
+ nil,
+ nil,
+ types.NewTuple(types.NewParam(token.NoPos, nil, "", types.Typ[types.String])),
+ false,
+ )),
+ }, nil).Complete()
+
+ byteType = types.Universe.Lookup("byte").Type()
+)
+
// candKind returns the objKind of candType, if any.
func candKind(candType types.Type) objKind {
+ var kind objKind
+
switch t := candType.Underlying().(type) {
case *types.Array:
- return kindArray
+ kind |= kindArray
+ if t.Elem() == byteType {
+ kind |= kindBytes
+ }
case *types.Slice:
- return kindSlice
+ kind |= kindSlice
+ if t.Elem() == byteType {
+ kind |= kindBytes
+ }
case *types.Chan:
- return kindChan
+ kind |= kindChan
case *types.Map:
- return kindMap
+ kind |= kindMap
case *types.Pointer:
+ kind |= kindPtr
+
// Some builtins handle array pointers as arrays, so just report a pointer
// to an array as an array.
if _, isArray := t.Elem().Underlying().(*types.Array); isArray {
- return kindArray
+ kind |= kindArray
}
case *types.Basic:
- if t.Info()&types.IsString > 0 {
- return kindString
+ switch info := t.Info(); {
+ case info&types.IsString > 0:
+ kind |= kindString
+ case info&types.IsInteger > 0:
+ kind |= kindInt
+ case info&types.IsFloat > 0:
+ kind |= kindFloat
+ case info&types.IsComplex > 0:
+ kind |= kindComplex
+ case info&types.IsBoolean > 0:
+ kind |= kindBool
}
case *types.Signature:
return kindFunc
}
- return 0
+ if types.Implements(candType, errorIntf) {
+ kind |= kindError
+ }
+
+ if types.Implements(candType, stringerIntf) {
+ kind |= kindStringer
+ }
+
+ return kind
}
diff --git a/internal/lsp/source/completion_printf.go b/internal/lsp/source/completion_printf.go
new file mode 100644
index 0000000..e13a938
--- /dev/null
+++ b/internal/lsp/source/completion_printf.go
@@ -0,0 +1,172 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package source
+
+import (
+ "go/ast"
+ "go/constant"
+ "go/types"
+ "strconv"
+ "strings"
+ "unicode/utf8"
+)
+
+// printfArgKind returns the expected objKind when completing a
+// printf-like operand. call is the printf-like function call, and
+// argIdx is the index of call.Args being completed.
+func printfArgKind(info *types.Info, call *ast.CallExpr, argIdx int) objKind {
+ // Printf-like function name must end in "f".
+ fn := exprObj(info, call.Fun)
+ if fn == nil || !strings.HasSuffix(fn.Name(), "f") {
+ return kindAny
+ }
+
+ sig, _ := fn.Type().(*types.Signature)
+ if sig == nil {
+ return kindAny
+ }
+
+ // Must be variadic and take at least two params.
+ numParams := sig.Params().Len()
+ if !sig.Variadic() || numParams < 2 || argIdx < numParams-1 {
+ return kindAny
+ }
+
+ // Param preceding variadic args must be a (format) string.
+ if !types.Identical(sig.Params().At(numParams-2).Type(), types.Typ[types.String]) {
+ return kindAny
+ }
+
+ // Format string must be a constant.
+ strArg := info.Types[call.Args[numParams-2]].Value
+ if strArg == nil || strArg.Kind() != constant.String {
+ return kindAny
+ }
+
+ return formatOperandKind(constant.StringVal(strArg), argIdx-(numParams-1)+1)
+}
+
+// formatOperandKind returns the objKind corresponding to format's
+// operandIdx'th operand.
+func formatOperandKind(format string, operandIdx int) objKind {
+ var (
+ prevOperandIdx int
+ kind = kindAny
+ )
+ for {
+ i := strings.Index(format, "%")
+ if i == -1 {
+ break
+ }
+
+ var operands []formatOperand
+ format, operands = parsePrintfVerb(format[i+1:], prevOperandIdx)
+
+ // Check if any this verb's operands correspond to our target
+ // operandIdx.
+ for _, v := range operands {
+ if v.idx == operandIdx {
+ if kind == kindAny {
+ kind = v.kind
+ } else if v.kind != kindAny {
+ // If mulitple verbs refer to the same operand, take the
+ // intersection of their kinds.
+ kind &= v.kind
+ }
+ }
+
+ prevOperandIdx = v.idx
+ }
+ }
+ return kind
+}
+
+type formatOperand struct {
+ // idx is the one-based printf operand index.
+ idx int
+ // kind is a mask of expected kinds of objects for this operand.
+ kind objKind
+}
+
+// parsePrintfVerb parses the leading printf verb in f. The opening
+// "%" must already be trimmed from f. prevIdx is the previous
+// operand's index, or zero if this is the first verb. The format
+// string is returned with the leading verb removed. Multiple operands
+// can be returned in the case of dynamic widths such as "%*.*f".
+func parsePrintfVerb(f string, prevIdx int) (string, []formatOperand) {
+ var verbs []formatOperand
+
+ addVerb := func(k objKind) {
+ verbs = append(verbs, formatOperand{
+ idx: prevIdx + 1,
+ kind: k,
+ })
+ prevIdx++
+ }
+
+ for len(f) > 0 {
+ // Trim first rune off of f so we are guaranteed to make progress.
+ r, l := utf8.DecodeRuneInString(f)
+ f = f[l:]
+
+ // We care about three things:
+ // 1. The verb, which maps directly to object kind.
+ // 2. Explicit operand indices like "%[2]s".
+ // 3. Dynamic widths using "*".
+ switch r {
+ case '%':
+ return f, nil
+ case '*':
+ addVerb(kindInt)
+ continue
+ case '[':
+ // Parse operand index as in "%[2]s".
+ i := strings.Index(f, "]")
+ if i == -1 {
+ return f, nil
+ }
+
+ idx, err := strconv.Atoi(f[:i])
+ f = f[i+1:]
+ if err != nil {
+ return f, nil
+ }
+
+ prevIdx = idx - 1
+ continue
+ case 'v', 'T':
+ addVerb(kindAny)
+ case 't':
+ addVerb(kindBool)
+ case 'c', 'd', 'o', 'O', 'U':
+ addVerb(kindInt)
+ case 'e', 'E', 'f', 'F', 'g', 'G':
+ addVerb(kindFloat | kindComplex)
+ case 'b':
+ addVerb(kindInt | kindFloat | kindComplex | kindBytes)
+ case 'q', 's':
+ addVerb(kindString | kindBytes | kindStringer | kindError)
+ case 'x', 'X':
+ // Omit kindStringer and kindError though technically allowed.
+ addVerb(kindString | kindBytes | kindInt | kindFloat | kindComplex)
+ case 'p':
+ addVerb(kindPtr | kindSlice)
+ case 'w':
+ addVerb(kindError)
+ case '+', '-', '#', ' ', '.', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9':
+ // Flag or numeric width/precicision value.
+ continue
+ default:
+ // Assume unrecognized rune is a custom fmt.Formatter verb.
+ addVerb(kindAny)
+ }
+
+ if len(verbs) > 0 {
+ break
+ }
+ }
+
+ return f, verbs
+}
diff --git a/internal/lsp/source/completion_printf_test.go b/internal/lsp/source/completion_printf_test.go
new file mode 100644
index 0000000..ab20f27
--- /dev/null
+++ b/internal/lsp/source/completion_printf_test.go
@@ -0,0 +1,72 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package source
+
+import (
+ "fmt"
+ "testing"
+)
+
+func TestFormatOperandKind(t *testing.T) {
+ cases := []struct {
+ f string
+ idx int
+ kind objKind
+ }{
+ {"", 1, kindAny},
+ {"%", 1, kindAny},
+ {"%%%", 1, kindAny},
+ {"%[1", 1, kindAny},
+ {"%[?%s", 2, kindAny},
+ {"%[abc]v", 1, kindAny},
+
+ {"%v", 1, kindAny},
+ {"%T", 1, kindAny},
+ {"%t", 1, kindBool},
+ {"%d", 1, kindInt},
+ {"%c", 1, kindInt},
+ {"%o", 1, kindInt},
+ {"%O", 1, kindInt},
+ {"%U", 1, kindInt},
+ {"%e", 1, kindFloat | kindComplex},
+ {"%E", 1, kindFloat | kindComplex},
+ {"%f", 1, kindFloat | kindComplex},
+ {"%F", 1, kindFloat | kindComplex},
+ {"%g", 1, kindFloat | kindComplex},
+ {"%G", 1, kindFloat | kindComplex},
+ {"%b", 1, kindInt | kindFloat | kindComplex | kindBytes},
+ {"%q", 1, kindString | kindBytes | kindStringer | kindError},
+ {"%s", 1, kindString | kindBytes | kindStringer | kindError},
+ {"%x", 1, kindString | kindBytes | kindInt | kindFloat | kindComplex},
+ {"%X", 1, kindString | kindBytes | kindInt | kindFloat | kindComplex},
+ {"%p", 1, kindPtr | kindSlice},
+ {"%w", 1, kindError},
+
+ {"%1.2f", 1, kindFloat | kindComplex},
+ {"%*f", 1, kindInt},
+ {"%*f", 2, kindFloat | kindComplex},
+ {"%*.*f", 1, kindInt},
+ {"%*.*f", 2, kindInt},
+ {"%*.*f", 3, kindFloat | kindComplex},
+ {"%[3]*.[2]*[1]f", 1, kindFloat | kindComplex},
+ {"%[3]*.[2]*[1]f", 2, kindInt},
+ {"%[3]*.[2]*[1]f", 3, kindInt},
+
+ {"foo %% %d", 1, kindInt},
+ {"%#-12.34f", 1, kindFloat | kindComplex},
+ {"% d", 1, kindInt},
+
+ {"%s %[1]X %d", 1, kindString | kindBytes},
+ {"%s %[1]X %d", 2, kindInt},
+ }
+
+ for _, c := range cases {
+ t.Run(fmt.Sprintf("%q#%d", c.f, c.idx), func(t *testing.T) {
+ if got := formatOperandKind(c.f, c.idx); got != c.kind {
+ t.Errorf("expected %d (%[1]b), got %d (%[2]b)", c.kind, got)
+ }
+ })
+ }
+}
diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go
index d52f90b..a043a17 100644
--- a/internal/lsp/source/util.go
+++ b/internal/lsp/source/util.go
@@ -457,11 +457,11 @@
return nil
}
-// typeConversion returns the type being converted to if call is a type
-// conversion expression.
-func typeConversion(call *ast.CallExpr, info *types.Info) types.Type {
+// exprObj returns the types.Object associated with the *ast.Ident or
+// *ast.SelectorExpr e.
+func exprObj(info *types.Info, e ast.Expr) types.Object {
var ident *ast.Ident
- switch expr := call.Fun.(type) {
+ switch expr := e.(type) {
case *ast.Ident:
ident = expr
case *ast.SelectorExpr:
@@ -470,8 +470,14 @@
return nil
}
+ return info.ObjectOf(ident)
+}
+
+// typeConversion returns the type being converted to if call is a type
+// conversion expression.
+func typeConversion(call *ast.CallExpr, info *types.Info) types.Type {
// Type conversion (e.g. "float64(foo)").
- if fun, _ := info.ObjectOf(ident).(*types.TypeName); fun != nil {
+ if fun, _ := exprObj(info, call.Fun).(*types.TypeName); fun != nil {
return fun.Type()
}
diff --git a/internal/lsp/testdata/lsp/primarymod/printf/printf.go b/internal/lsp/testdata/lsp/primarymod/printf/printf.go
new file mode 100644
index 0000000..6e56549
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/printf/printf.go
@@ -0,0 +1,33 @@
+package printf
+
+import "fmt"
+
+func myPrintf(string, ...interface{}) {}
+
+func _() {
+ var (
+ aInt int //@item(printfInt, "aInt", "int", "var")
+ aFloat float64 //@item(printfFloat, "aFloat", "float64", "var")
+ aString string //@item(printfString, "aString", "string", "var")
+ aBytes []byte //@item(printfBytes, "aBytes", "[]byte", "var")
+ aStringer fmt.Stringer //@item(printfStringer, "aStringer", "fmt.Stringer", "var")
+ aError error //@item(printfError, "aError", "error", "var")
+ aBool bool //@item(printfBool, "aBool", "bool", "var")
+ )
+
+ myPrintf("%d", a) //@rank(")", printfInt, printfFloat)
+ myPrintf("%s", a) //@rank(")", printfString, printfInt),rank(")", printfBytes, printfInt),rank(")", printfStringer, printfInt),rank(")", printfError, printfInt)
+ myPrintf("%w", a) //@rank(")", printfError, printfInt)
+ myPrintf("%x %[1]b", a) //@rank(")", printfInt, printfString)
+
+ fmt.Printf("%t", a) //@rank(")", printfBool, printfInt)
+
+ fmt.Fprintf(nil, "%f", a) //@rank(")", printfFloat, printfInt)
+
+ fmt.Sprintf("%[2]q %[1]*.[3]*[4]f",
+ a, //@rank(",", printfInt, printfFloat)
+ a, //@rank(",", printfString, printfFloat)
+ a, //@rank(",", printfInt, printfFloat)
+ a, //@rank(",", printfFloat, printfInt)
+ )
+}
diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden
index 654d04c..8ff02af 100644
--- a/internal/lsp/testdata/lsp/summary.txt.golden
+++ b/internal/lsp/testdata/lsp/summary.txt.golden
@@ -6,7 +6,7 @@
UnimportedCompletionsCount = 6
DeepCompletionsCount = 5
FuzzyCompletionsCount = 8
-RankedCompletionsCount = 139
+RankedCompletionsCount = 152
CaseSensitiveCompletionsCount = 4
DiagnosticsCount = 44
FoldingRangesCount = 2
