The code to check if a candidate object matches our candidate
inference had become complicated, messy, and in some cases incorrect.
The main source of the complexity is the "derived" expected and
candidate types. When considering a candidate object "foo", we also
consider "&foo", "foo()", and "*foo", as appropriate. On the expected
side of things, when completing the a variadic function parameter we
expect either the variadic slice type and the scalar element type.
The code had grown organically to handle the expanding concerns, but
that resulted in confused code that didn't handle the interplay
between the various facets of candidate inference.
For example, we were inappropriately invoking func candidates when
completing variadic args:
func foo(...func())
func bar() {}
foo(bar<>) // oops - expanded to "bar()"
and we weren't type matching functions properly as builtin args:
func myMap() map[string]int { ... }
delete(myM<>) // we weren't preferring (or invoking) "myMap()"
We also had methods like "typeMatches" which took both a "candidate"
object and a "candType" type, which doesn't make sense because the
candidate contains the type already.
Now instead we explicitly iterate over all the derived candidate and
expected types so they are treated the same. There are still some
warts left but I think this is a step in the right direction.
Change-Id: If84a84b34a8fb771a32231f7ab64ca192f611b3d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218877
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
We were recursing infinitely evaluating objects of recursive pointer
types such as "type foo *foo". Now we track named pointer types we
have already seen to avoid trying to dereference such objects forever.
I lazily initialized the "seen" map to avoid the allocation in the
normal case when you aren't dealing with named pointer types.
Fixesgolang/go#37104.
Change-Id: I5f294cfc5a641e7b5fd24e1d9dc55520726ea560
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218579
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This is in preparation for inferring stuff beyond just the expected
candidate type.
Change-Id: I31be9c1e4c82d82b1ff848858042a5edf46594e3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215340
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In cases like:
var foo *someType = bar.(some<>)
We will now complete "some" to "*someType". This involved two changes:
1. Properly detect expected type as *someType in above example. To do
this I just removed *ast.TypeAssertExpr from
breaksExpectedTypeInference() so we continue searching up the AST for
the expected type.
2. If the given type name T doesn't match, also try *T. If *T does
match, we mark the candidate as "makePointer=true" so we know to
prepend the "*" when formatting the candidate.
Change-Id: I05859c68082a798141755b614673a1483d864e3e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212717
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change flattens the completion options type into UserOptions and
DebuggingOptions, which will enable us to generate documentation for
these options more effectively. This results in some modifications in
the tests.
Additionally, the fuzzyMatching and caseSensitive boolean flags are
merged into one setting, matcher, which can be used to specify the type
of matcher that is used for completion. Other requests (notably
workspaceSymbols) may need to use a matcher in the future.
Change-Id: I185875e50351be4090c7a2b3340d40286dc9f4a0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212635
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We were marking all literal candidates as addressable so we were
getting invalid candidates like "&int()". Fix it to only mark literal
struct, array, slice and map types as addressable.
I also fixed the unnamed literal candidate to pass the dereferenced
expected type. For example, if the expected type was "*[]int" we were
passing a literal type of "*[]int" which wasn't working anymore. Now
we pass "[]int" and take its address as "&[]int{}".
Change-Id: I5d0ee074d3cc91c39dd881630583e31be5a05579
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212677
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We now support taking the address of objects to make better completion
candidates. For example:
i := 123
var p *int = <> // now you get a candidate for "&i"
This required that we track addressability better, particularly when
searching for deep candidates. Now each candidate knows if it is
addressable, and the deep search propagates addressability to child
candidates appropriately.
The basic propagation logic is:
- In-scope *types.Var candidates are addressable. This handles your
basic "foo" variable whose address if "&foo".
- Surrounding selector is addressable based on type checker info. This
knows "foo.bar.<>" is addressable but "foo.bar().<>" isn't
- When evaluating deep completions, fields after a function call lose
addressability, but fields after a pointer regain addressability. For
example, "foo.bar()" isn't addressable, but "foo.bar().baz" is
addressable if "bar()" returns a pointer.
Fixesgolang/go#36132.
Change-Id: I6a8659eb8c203262aedf86844ac39a2d1e81ecc4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212399
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In a previous change I inadvertently added completion candidates like:
var f func(int)
f = <> // useless candidate "func(int)(<>)"
Ignoring the fact it is a syntax error without more parens around the
signature, it isn't a useful candidate because you don't need to cast
when assigning a named signature type to an unnamed type.
Change-Id: Ic261817af344ee47193240a11dca5d3a32cbd293
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211319
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When the expected type is a basic type, we will now offer a
corresponding type conversion candidate. For example:
var foo int64
foo = // offer "int64(<>)" as a candidate
The type conversion candidate will be ranked below matching concrete
candidates but above the sea of non-matching candidates.
This change broke almost every completion test. I added a new
completion option for literal candidates so tests can selectively ask
for literal completions.
Updates golang/go#36015.
Change-Id: I63fbdb33436d662a666c1ffd3b2d918d840dccc7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210288
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When //line directives are in play, the ast.File's Offset function will
return offsets in the generated file. We want offsets in the authored
file, so we need to pass a Converter for the authored file, in addition
to the ast.File for the generated file. For the same reason, we have to
start (Range).Span() by translating into positions in the authored file,
then calculate offsets from that.
A lot of call sites outside of the LSP don't pass the Converter, but
they probably don't matter much. I think everything inside does because
it ends up using mappedRange.
Updates golang/go#35720.
Change-Id: I7be09b3a50720b078e862d48cfdb02208f8187ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208501
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In cases like:
var foo []bytes.Buffer
foo = append(foo, <>)
you will now get a literal candidate "bytes.Buffer{}". Previously we
were skipping all literal candidates at the variadic position, but the
intention was to only skip literal slice candidates (i.e.
"[]bytes.Buffer{}" in the above example).
I also improved the literal struct snippet to not leave the cursor
inside the curlies when the struct type has no accessible fields.
Previously it was only checking if the struct had no fields at all.
This means after completing in the above example you will end up with
"bytes.Buffer{}<>" instead of "bytes.Buffer{<>}", where "<>" denotes
the cursor.
Change-Id: Ic2604a4ea65d84ad855ad6e6d98b8ab76eb08d77
Reviewed-on: https://go-review.googlesource.com/c/tools/+/207537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change cleans up internal/lsp/source/view.go to have a more logical
ordering and deletes the view.CheckPackageHandle function. Now, the only
way to get a CheckPackageHandle is through a snapshot (so all of the
corresponding edits).
Also, renamed fuzzy tests to fuzzymatch. Noticed this weird error when
debugging - I had golang.org/x/tools/internal/lsp/fuzzy in my module
cache and it conflicted with the test version.
Change-Id: Ib87836796a8e76e6b6ed1306c2a93e9a5db91cce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208099
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Previously we were erroneously suggesting a "func() {}" literal in
cases like:
http.Handle("/", <>)
This was happening because saw that the http.HandlerFunc type
satisfied the http.Handler interface, and that http.HandlerFunc is a
function type. However, of course, you can't pass a function literal
to http.Handle().
Make a few tweaks to address the problem:
1. Don't suggest literal "func () {}" candidates if the expected type
is an interface type.
2. Suggest named function types that implement an interface. This
causes us to suggest "http.HandlerFunc()" in the above example.
3. Suggest a func literal candidate inside named function type
conversions. This will suggest "func() {}" when completing
"http.HandlerFunc(<>)".
This way the false positive func literal is gone, and you still get
literal candidates that help you use an http.HandlerFunc as an
http.Handler. Note that this particular example is not very compelling
in light of http.HandleFunc() which can take a func literal directly,
but such a convenience function may not exist in other analogous
situations.
Change-Id: Ia68097b9a5b8351921349340d18acd8876554691
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Unimported completions now try to pull Packages from everywhere, not
just the transitive dependencies of the current package. That confused
the import formatting code, which only looked at deps. Pass the Package
along with the import suggestion, and use it when it's present.
Also change some error messages to be different for diagnostic purposes.
Fixesgolang/go#35359.
Change-Id: Ia8ca923e46723e855ddd2da7611e6eb13c02bb4f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205501
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
When our expected type is a named type from another package, we now always
search that other package for completion candidates, even if it is not currently
imported.
Consider the example:
-- foo.go --
import "context"
func doSomething(ctx context.Context) {}
-- bar.go--
doSomething(<>)
"bar.go" doesn't import "context" yet, so normally you need to first import
"context" through whatever means before you get completion items from "context".
Now we notice that the expected type's package hasn't been imported yet and give
deep completions from "context".
Another use case is with literal completions. Consider:
-- foo.go --
import "bytes"
func doSomething(buf *bytes.Buffer) {}
-- bar.go--
doSomething(<>)
Now you will get a literal completion for "&bytes.Buffer{}" in "bar.go" even
though it hasn't imported "bytes" yet.
I had to pipe the import info around a bunch of places so the import is added
automatically for deep completions and literal completions.
Change-Id: Ie86af2aa64ee235038957c1eecf042f7ec2b329b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201207
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Previously we unconditionally qualified literal candidate types with
their package. For example:
var buf *bytes.Buffer
buf = &bytes.Bu<>
would complete to:
buf = &bytes.bytes.Buffer{}
Now we don't qualify the type if the cursor position is in the
selector of an *ast.SelectorExpr. We only generate literal candidates
for type names, so if we are in a selector then we can assume it is a
package qualified type (as opposed to an object field).
We also handle the insertion of "&" for literal pointers better. If you are in
the selector of an *ast.SelectorExpr, we prepend the "&" to the beginning of the
expression rather than the selector. For example, you will end up with
"&bytes.Buffer{}" instead of "bytes.&Buffer{}".
Updates golang/go#34872.
Change-Id: I812aa809cd4e649a429853386789f80033412814
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201200
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Race condition: I deleted the code that acquired the mutex when checking
if a file's imports have changed.
Merge conflict: I submitted a change without rebasing and re-running TryBots.
Change-Id: Iae351f2fff893cfd94db79d9f79578d9827a8c21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197297
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
source.DiagnosticSeverity and source.CompletionItemKind are duplicated
and not worth maintaining.
Change-Id: I8d6c8621a227855309c0977da59d8c9fa53617bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197177
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Normally you don't want literal candidates for basic types (e.g.
"int(0)") since you can type the literal value without the type name.
One exception is if you are creating a named basic type that
implements an interface. For example:
http.Handle("/", http.FileServer(<>))
will now give "http.Dir()" as a candidate since http.Dir is a named
string type that implements the required interface http.FileSystem.
Change-Id: Id2470c45e469ea25cd0f9849cfdad19ac0e784bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195838
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Now we will offer function literal completions when we know the
expected type is a function. For example:
sort.Slice(someSlice, <>)
will offer the completion "func(...) {}" which if selected will
insert:
func(i, j int) bool {<>}
I opted to use an abbreviated label "func(...) {}" because function
signatures can be quite long/verbose with all the type names in there.
The only interesting challenge is how to handle signatures that don't
name the parameters. For example,
func HandleFunc(pattern string, handler func(ResponseWriter, *Request)) {
does not name the "ResponseWriter" and "Request" parameters. I went
with a minimal effort approach where we try abbreviating the type
names, so the literal completion item for "handler" would look like:
func(<rw> ResponseWriter, <r> *Request) {<>}
where <> denote placeholders. The user can tab through quickly if they
like the abbreviations, otherwise they can rename them.
For unnamed types or if the abbreviation would duplicate a previous
abbreviation, we fall back to "_" as the parameter name. The user will
have to rename the parameter before they can use it.
One side effect of this is that we cannot support function literal
completions with unnamed parameters unless the user has enabled
snippet placeholders.
Change-Id: Ic0ec81e85cd8de79bff73314e80e722f10f8c84c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193699
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Add support for literal completion candidates such as "[]int{}" or
"make([]int, 0)". We support both named and unnamed types. I used the
existing type matching logic, so, for example, if the expected type is
an interface, we will suggest literal candidates that implement the
interface.
The literal candidates have a lower score than normal matching
candidates, so they shouldn't be disruptive in cases where you don't
want a literal candidate.
This commit adds support for slice, array, struct, map, and channel
literal candidates since they are pretty similar. Functions will be
supported in a subsequent commit.
I also added support for setting a snippet's final tab stop. This is
useful if you want the cursor to end up somewhere other than the
character after the snippet.
Change-Id: Id3b74260fff4d61703989b422267021b00cec005
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193698
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
