From 6f4f008689da0001e7e1fc8f5f9dcaba40c3bcd3 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Thu, 6 Aug 2020 22:33:47 -0700 Subject: [PATCH] internal/lsp/source: improve completion in switch cases Now we downrank candidates that have already been used in other switch cases. For example: switch time.Now().Weekday() { case time.Monday: case time.<> // downrank time.Monday } It wasn't quite as simple as tracking the seen types.Objects. Consider this example: type foo struct { i int } var a, b foo switch 123 { case a.i: case <> } At <> we don't want to downrank "b.i" even though "i" is represented as the same types.Object for "a.i" and "b.i". To accommodate this, we track having seen ["a", "i"] together. We will downrank "a.i", but not "b.i". I applied only a minor 0.9 downranking when the candidate has already been used in another case clause. It is hard to know what the user is planning. For instance, in the preceding example the user could intend to write "a.i + 1", so we mustn't downrank "a.i" too much. Change-Id: I62debc5be3d5d310deb69d11770cf5f8bd9add1d Reviewed-on: https://go-review.googlesource.com/c/tools/+/247337 Run-TryBot: Muir Manders TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/source/completion.go | 127 ++++++++++++++++++ .../lsp/primarymod/rank/switch_rank.go.in | 25 +++- internal/lsp/testdata/lsp/summary.txt.golden | 4 +- 3 files changed, 150 insertions(+), 6 deletions(-) diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 5010e18c8c..770bfafde6 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -330,6 +330,17 @@ func (c *completer) found(ctx context.Context, cand candidate) { if c.matchingCandidate(&cand) { cand.score *= highScore + + if c.seenInSwitchCase(&cand) { + // If this candidate matches an expression already used in a + // different case clause, downrank. We only downrank a little + // because the user could write something like: + // + // switch foo { + // case bar: + // case ba<>: // they may want "bar|baz" or "bar.baz()" + cand.score *= 0.9 + } } else if isTypeName(obj) { // If obj is a *types.TypeName that didn't otherwise match, check // if a literal object of this type makes a good candidate. @@ -376,6 +387,18 @@ func (c *completer) found(ctx context.Context, cand candidate) { c.deepSearch(ctx, cand) } +// seenInSwitchCase reports whether cand has already been seen in +// another switch case statement. +func (c *completer) seenInSwitchCase(cand *candidate) bool { + for _, chain := range c.inference.seenSwitchCases { + if c.objChainMatches(cand.obj, chain) { + return true + } + } + + return false +} + // candidate represents a completion candidate. type candidate struct { // obj is the types.Object to complete to. @@ -835,6 +858,8 @@ const ( // selector finds completions for the specified selector expression. func (c *completer) selector(ctx context.Context, sel *ast.SelectorExpr) error { + c.inference.objChain = objChain(c.pkg.GetTypesInfo(), sel.X) + // Is sel a qualified identifier? if id, ok := sel.X.(*ast.Ident); ok { if pkgName, ok := c.pkg.GetTypesInfo().Uses[id].(*types.PkgName); ok { @@ -1559,6 +1584,19 @@ type candidateInference struct { // foo(bar<>) // variadicAssignees=true // foo(bar, baz<>) // variadicAssignees=false variadicAssignees bool + + // seenSwitchCases tracks the expressions already used in the + // containing switch statement's cases. Each expression is tracked + // as a slice of objects. For example, "case foo.bar().baz:" is + // tracked as []types.Object{foo, bar, baz}. Tracking the entire + // "chain" allows us to differentiate "a.foo" and "b.foo" when "a" + // and "b" are the same type. + seenSwitchCases [][]types.Object + + // objChain contains the chain of objects representing the + // surrounding *ast.SelectorExpr. For example, if we are completing + // "foo.bar.ba<>", objChain will contain []types.Object{foo, bar}. + objChain []types.Object } // typeNameInference holds information about the expected type name at @@ -1732,6 +1770,21 @@ Nodes: if swtch, ok := findSwitchStmt(c.path[i+1:], c.pos, node).(*ast.SwitchStmt); ok { if tv, ok := c.pkg.GetTypesInfo().Types[swtch.Tag]; ok { inf.objType = tv.Type + + // Record which objects have already been used in the case + // statements so we don't suggest them again. + for _, cc := range swtch.Body.List { + for _, caseExpr := range cc.(*ast.CaseClause).List { + // Don't record the expression we are currently completing. + if caseExpr.Pos() < c.pos && c.pos <= caseExpr.End() { + continue + } + + if objs := objChain(c.pkg.GetTypesInfo(), caseExpr); len(objs) > 0 { + inf.seenSwitchCases = append(inf.seenSwitchCases, objs) + } + } + } } } return inf @@ -1794,6 +1847,46 @@ Nodes: return inf } +// objChain decomposes e into a chain of objects if possible. For +// example, "foo.bar().baz" will yield []types.Object{foo, bar, baz}. +// If any part can't be turned into an object, return nil. +func objChain(info *types.Info, e ast.Expr) []types.Object { + var objs []types.Object + + for e != nil { + switch n := e.(type) { + case *ast.Ident: + obj := info.ObjectOf(n) + if obj == nil { + return nil + } + objs = append(objs, obj) + e = nil + case *ast.SelectorExpr: + obj := info.ObjectOf(n.Sel) + if obj == nil { + return nil + } + objs = append(objs, obj) + e = n.X + case *ast.CallExpr: + if len(n.Args) > 0 { + return nil + } + e = n.Fun + default: + return nil + } + } + + // Reverse order so the layout matches the syntactic order. + for i := 0; i < len(objs)/2; i++ { + objs[i], objs[len(objs)-1-i] = objs[len(objs)-1-i], objs[i] + } + + return objs +} + // applyTypeModifiers applies the list of type modifiers to a type. // It returns nil if the modifiers could not be applied. func (ci candidateInference) applyTypeModifiers(typ types.Type, addressable bool) types.Type { @@ -2127,6 +2220,40 @@ func (c *completer) matchingCandidate(cand *candidate) bool { return false } +// objChainMatches reports whether cand combined with the surrounding +// object prefix matches chain. +func (c *completer) objChainMatches(cand types.Object, chain []types.Object) bool { + // For example, when completing: + // + // foo.ba<> + // + // If we are considering the deep candidate "bar.baz", cand is baz, + // objChain is [foo] and deepChain is [bar]. We would match the + // chain [foo, bar, baz]. + + if len(chain) != len(c.inference.objChain)+len(c.deepState.chain)+1 { + return false + } + + if chain[len(chain)-1] != cand { + return false + } + + for i, o := range c.inference.objChain { + if chain[i] != o { + return false + } + } + + for i, o := range c.deepState.chain { + if chain[i+len(c.inference.objChain)] != o { + return false + } + } + + return true +} + // candTypeMatches reports whether cand makes a good completion // candidate given the candidate inference. cand's score may be // mutated to downrank the candidate in certain situations. diff --git a/internal/lsp/testdata/lsp/primarymod/rank/switch_rank.go.in b/internal/lsp/testdata/lsp/primarymod/rank/switch_rank.go.in index 9e23f6b52c..b828528da8 100644 --- a/internal/lsp/testdata/lsp/primarymod/rank/switch_rank.go.in +++ b/internal/lsp/testdata/lsp/primarymod/rank/switch_rank.go.in @@ -1,12 +1,29 @@ package rank +import "time" + func _() { switch pear { - case : //@complete(":", pear, apple) + case _: //@rank("_", pear, apple) } - switch pear { - case "hi": - //@complete("", apple, pear) + time.Monday //@item(timeMonday, "time.Monday", "time.Weekday", "const"),item(monday ,"Monday", "time.Weekday", "const") + time.Friday //@item(timeFriday, "time.Friday", "time.Weekday", "const"),item(friday ,"Friday", "time.Weekday", "const") + + now := time.Now() + now.Weekday //@item(nowWeekday, "now.Weekday", "func() time.Weekday", "method") + + then := time.Now() + then.Weekday //@item(thenWeekday, "then.Weekday", "func() time.Weekday", "method") + + switch time.Weekday(0) { + case time.Monday, time.Tuesday: + case time.Wednesday, time.Thursday: + case time.Saturday, time.Sunday: + case t: //@rank(":", timeFriday, timeMonday) + case time.: //@rank(":", friday, monday) + + case now.Weekday(): + case week: //@rank(":", thenWeekday, nowWeekday) } } diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden index 493b5f0eb7..35bed3d946 100644 --- a/internal/lsp/testdata/lsp/summary.txt.golden +++ b/internal/lsp/testdata/lsp/summary.txt.golden @@ -1,12 +1,12 @@ -- summary -- CallHierarchyCount = 1 CodeLensCount = 4 -CompletionsCount = 241 +CompletionsCount = 239 CompletionSnippetCount = 81 UnimportedCompletionsCount = 6 DeepCompletionsCount = 5 FuzzyCompletionsCount = 8 -RankedCompletionsCount = 130 +RankedCompletionsCount = 134 CaseSensitiveCompletionsCount = 4 DiagnosticsCount = 44 FoldingRangesCount = 2