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>
Fix type inference to expect a type name for the first "make()"
parameter and an integer for later parameters. For example:
// Previously we expected "[]int{}", now we expect "[]int".
var _ []int = make(<>)
Note that we don't currently support actually completing to unnamed
type names like "[]int", but this improvement at least eliminates
nonsensical completion suggestions.
// Previously we had no expectation, now we expect an int.
var _ []int = make([]int, <>)
Change-Id: Ifd349767662ab6902d3a3ea9e52de7df70cb37c7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217310
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ingo Oeser <nightlyone@googlemail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I had originally thought I might be able to use exprAtPos for this,
which is why I ended up eliminating that function when I saw it only had
one use.
One test also had to change in order to fit better with the spec.
Specifically: "If [the active parameter is] omitted or the value
lies outside the range of `signatures[activeSignature].parameters`
it defaults to 0 if the active signature has parameters."
Fixesgolang/go#36766.
Change-Id: I400d5b2db2985bfaa5efbcd91225151ca8b5f46a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216309
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change moves as much view initialization code into the
initialization function, instead of having it happen on view create.
Also, the `go env` variables that are collected at inconsistent times
are all collected on view creation. That is sufficient, since the view
is recreated if the environment changes.
I had originally hoped that the initial call to `go env` and the
-modfile detection could become part of this parallel initialization as
well, but you can't create a *packages.Config until the temporary
modfile has been set up, so it still makes sense to do that on view
create. This is, however, the reasoning behind the refactorings in
the -modfile detection in this CL. The main changes are a few renamings
and a split between snapshot.ModFiles and view.modFiles to maximize the
amount of work done in the view. I changed view.modfiles to moduleInformation
because I thought we might want to store additional information there at some
point. Rohan, please let me know if you disagree with any of the changes I made,
and I can revert them.
Fixesgolang/go#36487
Change-Id: I504db5a4f41b79bee99ebd391e32e7b520a19569
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
We now understand what "kind" of type is expected when using various
builtins. For example, when completing "close(<>)" we prefer channels,
and when completing "delete(<>)" we prefer maps.
I also added some code to infer the expected type for the second
argument to "delete()" and for the args to "copy()":
delete(map[someType]int{}, <>) // expect "someType"
copy([]int{}, <>) // expect "[]int"
copy(<>, []int{}) // expect "[]int"
And I marked "new()" as expected a type name, and it infers the type
name properly:
var _ *int = new(<>) // expected type at "<>" is "int"
Fixesgolang/go#36326.
Change-Id: I4295c8753f8341d47010a0553fd2d0c2586f2efa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212957
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
