1
0
mirror of https://github.com/golang/go synced 2024-10-01 09:28:37 -06:00
Commit Graph

156 Commits

Author SHA1 Message Date
Muir Manders
92d517f97b internal/lsp/source: suppress completions in ellipsis
Editors typically trigger completion automatically after ".". This
pops up annoying, useless completions after "..." variadic param, such
as "foo(bar...<>)". We now suppress completions in or directly after
the ellipsis.

Fixes golang/go#37358.

Change-Id: I9fc94fbdf69429bd787bcb2c643f4f730e24154d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/222200
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-09 18:42:16 +00:00
Muir Manders
2047c2d578 internal/lsp/source: complete keywords as types
Offer "struct", "interface", "map", "chan", and "func" keywords when
we expect a type. For example "var foo i<>" will offer "interface".

Because "struct" and "interface" are more often used when declaring
named types, they get a higher score in type declarations. Otherwise,
"map", "chan" and "func" get a higher score.

I also got rid of the special keyword scoring. Now keywords just use
stdScore and highScore. This makes the interplay with other types of
candidates more predictable. Keywords are offered in pretty limited
contexts, so I don't think they will be annoying.

Finally, keyword candidate score is now to be scaled properly based on
how well they match the prefix. Previously they weren't penalized for
not matching well, so there were probably some situations where
keywords were ranked too high.

Updates golang/go#34009.

Change-Id: I0b659c00a8503cd72da28853dfe54fcb67f734ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220503
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-03-08 01:33:38 +00:00
Muir Manders
8d77031968 internal/lsp/source: fix completion crash with untyped assignee
We were panicking when a LHS assignee's type was nil, such as:

    // "foo" has not been declared
    foo = 123

A recent refactoring changed (*completer).typeMatches() to no longer
gracefully handle a nil types.Type for its first parameter. That
behavior seems fine, so fix the problematic caller of typeMatches to
check for nil before calling.

Change-Id: Ie11e4a2d374ab1efbf6fd13fbe214e06d359fca0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221020
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-02 21:29:26 +00:00
Anthony Fok
26f6a1b680 internal/lsp/source: fix typo: identifer → identifier
Change-Id: I37da513eb4a28df5054066b94ff1d915b9dce239
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221022
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-26 18:09:45 +00:00
Muir Manders
023911ca70 internal/lsp/source: untangle completion type comparison
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>
2020-02-24 18:12:40 +00:00
Muir Manders
8a925fa4c0 internal/lsp/source: improve completions at file scope
Add support for var/func/const/type/import keywords at the file scope.
I left out "package" because, currently, if you are completing
something that means you must already have a package declaration. The
main hurdle was that anything other than a decl keyword shows up as
an *ast.BadDecl at the file scope. To properly detect the prefix we
manually scan for the token containing the position.

I also made a couple small drive-by improvements:
 - Also suggest "goto" and "type" keywords in functions.
 - Allow completing directly before a comment, e.g. "foo<>//". I
   needed this for a test that would have been annoying to write
   otherwise.

Updates golang/go#34009.

Change-Id: I290e7bdda9e66a16f996cdc291985a54bf375231
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217500
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-20 22:48:06 +00:00
Muir Manders
ea829e2eb2 internal/lsp/source: handle completing into variadic params
For example:

func foo(string, ...int)

func a() string
func b() (string, int)
func c() (string, int, int)

// Prefer "a()" and "b()" and "c()". Previously we didn't prefer any of
// them.
foo(<>)

Fixes golang/go#36540.

Change-Id: I144b3f63942b7699d3034efcc9ad8535a7fa3165
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215538
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-14 18:12:26 +00:00
Muir Manders
49b8ac185c internal/lsp/cache: add file contents to ParseGoHandle
Currently there is no need for this because the file contents are part
of the file handle. This change is in preparation for an impending
improvement that tweaks the source code during the parse stage to fix
certain kind of terminal parse errors. Any code that wants to use
an *ast.File or *token.File in conjunction with the file contents
needs access to the doctored source code so things line up.

Change-Id: I59d83d3d6150aa1264761aa2c1f6c1269075a2ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218979
Run-TryBot: Muir Manders <muir@mnd.rs>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 05:05:14 +00:00
Muir Manders
cbc0cc175f internal/lsp/source: fix completion crash in append()
We were crashing in cases like:

    var _ []byte = append([]byte{}, ""...<>)

We were type asserting the type of append's second param
to *types.Slice, but in this case it is a string (*types.Basic). Fix
by checking the type assert was successful.

Note that we still don't attempt to give string completions when
appending to a byte slice. We can add that special case later once
everyone is clamoring for it.

Change-Id: I1d2fbd7f538e580d33c2dab4ef127a88e16d7ced
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219144
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-13 02:33:03 +00:00
Muir Manders
ea181f53ac internal/lsp/source: filter candidates when type name required
Now when we expect a type name at the cursor, we omit non-type name
completion candidates. For example:

inch := 1
var foo in<> // don't offer "inch"

I also added expected type name detection for value specs:

// Expect a type name at <>
var foo <>

Fixes golang/go#32806.

Change-Id: I32477cb286d2050bac5ccc767f8a608124fa5acd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216400
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-02-12 15:05:39 +00:00
Muir Manders
2de505fc53 internal/lsp: support multi-dereferencing completion candidates
Now we keep a count of how many times to dereference a candidate. For
example:

    var foo ***int
    var _ int = f<> // Now we offer "***foo" instead of "*foo".

Change-Id: I14edc40aeec6884399eceb3dd3b4f85dc74a773c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218580
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-11 04:52:51 +00:00
Muir Manders
1e13d9580f internal/lsp/source: fix type modifier detection in composite literals
When completing a composite literal value, we were returning from
candidate inference before we recorded type modifiers such as prefix
"&" or "*". This was causing funny completions like:

type myStruct struct { s *myStruct }
myStruct{s: &mySt<> // completed to "&&myStruct{}"

Now we properly pick up on the "&" prefix so we know our literal
"myStruct{}" candidate does not need a "&".

Change-Id: I908936698cfedfef81bc0c1cbcd93e14dc00e3a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218377
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-11 03:24:02 +00:00
Muir Manders
0bc66720f3 internal/lsp/source: limit to 5 unimported package name candidates
Currently we show up to ~100 unimported package names matching the
completion prefix. This isn't useful since, assuming the user even
wants an unimported package, they will just type more to narrow it
down rather than scroll through 100 options. Having so many candidates
also slows things down due to per-candidate overhead in gopls and in
the LSP client. Now we instead limit to 5 unimported package names.

Unimported package members, on the other hand, make sense to list
many. The user may want to scroll through because they don't remember
the name of what they are looking for. I left the max value at 100.

Change-Id: I00e11fa0420758f8db6c7049f80fa156773a5ee6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218879
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-02-11 00:32:09 +00:00
Muir Manders
2ef0387721 internal/lsp/source: fix unimported member completion ranking
When completing members on a type checked, unimported package, you get
fully typed members. That means you get deep completions. Because we
downrank the initial unimported package members so much, any deep
completions were dominating the rankings. For example

    context.Back<>

yielded "context.Background().Err" ranked above "context.Background".
Fix by scoring context.Background in this example as
stdScore+tinyRelevanceScore instead of just tinyRelevanceScore. I also
changed untyped candidate scores in the same way so they stay
competitive when you have both imported and unimported candidates.

The other option was to propagate the score penalty into deep
candidates, but that wasn't easy. In general I think you are better off
avoiding big score penalties because they complicate the interplay
between different kinds of candidates. Scoring needs an overhaul, but
at least we are building up our test suite in the meantime.

Change-Id: Ia5d32c057b04174229686cec6ac0542c30e186e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218378
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-10 18:17:33 +00:00
Muir Manders
61798d64f0 internal/lsp: fix crash completing recursive pointer types
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.

Fixes golang/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>
2020-02-07 22:44:06 +00:00
Muir Manders
f66ef90017 internal/lsp/source: improve completion after accidental keywords
Completion often fails when the completion prefix happens to be a
keyword. We previously tried to fix this with AST surgery, but
often the accidental keyword is not apparent looking at the AST.
For example:

    chan<>
    foo()

parses as CallExpr{Fun: ChanType{Value: Ident{"foo"}}} with very few
hints that something is wrong, and:

    default
    foo()

is completely omitted from the AST.

Rather than look in the AST, we now instead manually look for a
keyword token that contains the completion position. If we find one,
we treat that as our surrounding identifier.

Updates golang/go#34332.

Change-Id: I68ed0dd905848c0eae61f39ecb8b73adb1e72746
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216961
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 05:08:22 +00:00
Muir Manders
50d618665b internal/lsp/source: improve completion involving multiple return values
For example:

// Prefer functions that return one or two values. Previously
// we had no preference.
foo, bar := <>

// Prefer functions that return "(int)" or "(int, ??)". Previously we
// only preferred the former.
var foo int
foo, bar := <>

// Prefer functions that return "(int)" or "(int, int)". Previously we
// only preferred the former.
var foo func(int, int)
foo(<>)

In the above example, we don't handle "foo" being variadic yet.

I also took the liberty to break up matchingCandidate() into separate
functions since it was getting rather long.

Updates golang/go#36540.

Change-Id: I9140dd989dfde1ddcfcd9d2a14198045c02587f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215537
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-02-06 05:07:08 +00:00
Muir Manders
bc0b458b10 internal/lsp/source: improve completion for "make()" args
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>
2020-02-02 23:20:50 +00:00
Rebecca Stambler
ab094738a9 internal/lsp: fix active parameter for incomplete parentheses
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."

Fixes golang/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>
2020-01-27 19:24:44 +00:00
Rebecca Stambler
1b668f2091 internal/lsp: disable literal completion candidates for some clients
If a client doesn't support the snippet format in completion insert
text, they can't take full advantage of the literal completion
candidates. Disable it in those cases, and remove the setting in
internal/lsp/source/options.go.

Fixes golang/go#36655.

Change-Id: Ibc045a0f2945aab753b0187194a03d0c0398dba5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/216299
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-24 20:07:20 +00:00
Muir Manders
9bae6688e3 internal/lsp/source: support dereferencing for completion
In cases like:

    var j *int
    var i int = <>

We will now provide "*j" as a completion candidate.

Change-Id: I1d35c2dca4864f13f7534e15b17450d784985557
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215358
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-22 21:43:17 +00:00
Muir Manders
3e306b7b28 internal/lsp/source: rename "typeInference" to "candidateInference"
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>
2020-01-21 20:43:48 +00:00
Heschi Kreinick
fe56e63357 internal/lsp/source: fix ranking of untyped completions
Claiming that untyped candidates matched the type of whatever we were
looking for messed up rankings in found(). The only other places that
use it will all work better with false. Return false.

Updates golang/go#36591.

Change-Id: I5e1e8af7cc5c27422740cbb77f9a4a20edb1e447
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215322
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 21:50:04 +00:00
Muir Manders
a20955124b internal/lsp: consolidate completion sorting
Move completion candidate sorting into internal/lsp/source so
source_test.go and internal/lsp don't have to duplicate the logic.

Change-Id: Ifbe7ca5ad6a5b74020fd1260b4d4f775709968cf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-17 20:30:43 +00:00
Heschi Kreinick
6edc0a871e internal/lsp/source: score in-memory unimported candidates
We were assuming that all in-memory packages were equally useful. That's
not true for projects with a large dependency tree. Call into the
imports code to score them.

While I'm here, score the main module above direct deps.

Updates golang/go#36591.

Change-Id: I07c56dd3ff7338e76f3643e18d35abc1b52d6763
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215023
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-17 01:23:04 +00:00
Muir Manders
473961ec04 internal/lsp/source: improve completion for "range" and "<-"
Now that we understand object "kind" for builtin generic functions, we
can apply it to a couple more places as well:

// prefer rangeable object kinds
for i := range <> {
}

// prefer channels
<- <>

Change-Id: If9cfba3a06b3abde073a9d397000bb3f3b0e9853
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214678
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-01-16 06:24:25 +00:00
Muir Manders
49797d04a8 internal/lsp/source: improve completion in type assertions
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>
2020-01-16 01:06:23 +00:00
Rob Findley
7ae403b6b5 internal/lsp: finish renaming CheckPackageHandle to PackageHandle
In golang.org/cl/209419, CheckPackageHandle was renamed to
PackageHandle, but a number of references to CheckPackageHandle remained
in function names and comments.

This CL cleans up most of these, though there was at least one case
(internal/lsp/cache.checkPackageKey) where the obvious renaming
conflicted with another function, so I skipped it.

Change-Id: I517324279ff05bd5b1cab4eeb212a0090ca3e3ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214800
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-14 23:56:10 +00:00
Muir Manders
d31a08c2ed internal/lsp/source: improve completion support for args to builtins
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"

Fixes golang/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>
2020-01-14 05:24:53 +00:00
Rebecca Stambler
533f309ed4 internal/lsp: reload workspace package metadata on demand
This change removes functions from the snapshot that return package IDs.
We prefer PackageHandles, since getting PackageHandles in a granular
fashion is not effective and causes us to spawn many `go list`
processes. By only ever returning PackageHandles, we can batch metadata
reloads for workspace packages. This enables us to add a check to
confirm that the snapshot is in a good state before returning important
data, like reverse dependencies and workspace package handles.

Change-Id: Icffc8d8e0449864f207c15aa211e84cb158c163f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/214383
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-01-13 20:10:07 +00:00
Rebecca Stambler
8f45075ebc internal/lsp: merge completion options into source.Options
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>
2020-01-13 20:09:44 +00:00
Muir Manders
89082a3841 internal/lsp/source: fix some invalid literal candidates
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>
2020-01-08 20:36:44 +00:00
Heschi Kreinick
316d2f2484 internal/lsp/source: only add names to imports when necessary
As usual, I forgot to clear out the import spec's name when it matches
the import path.

Change-Id: I4ddd49b70e0db95fcd30d2968b098327fac39a92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213222
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Muir Manders <muir@mnd.rs>
Reviewed-by: zikaeroh <zikaeroh@gmail.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-08 19:54:15 +00:00
Muir Manders
a222fb47e2 internal/lsp/source: don't downrank builtin constant completions
We downrank untyped constant candidates so that we prefer candidates
whose type matches exactly. However, this was causing builtin
constants like "true" to be outranked by candidates that fuzzily match
"true". Fix by not downranking builtin constants.

Fixes golang/go#36363.

Change-Id: I14801688c96efdbb7ff9fee69f66028530df984c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/213137
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-01-07 18:15:58 +00:00
Heschi Kreinick
ba16e80ae2 internal/imports: filter out self-import completions
Fixes golang/go#36321.

Change-Id: Ic6cdad4b611e5a16e086743f53f85bcb71070a21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212897
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:44:38 +00:00
Heschi Kreinick
9a28a1fa70 internal/lsp/source: scan loaded packages first for completions
Packages that have already been loaded by gopls are more likely to be
used, and have full type information. Check them for completion
candidates before scanning the disk.

Also, minor bug fixes: add a missing mutex, and use a lower-than-usual
score for typed unimported completions.

Change-Id: I46388802913f9a89342fb47290f704b471154ec0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212860
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:21:36 +00:00
Heschi Kreinick
7ec15289dd internal/imports: optimize scan implementations
In scan implementations, stop after cancellation, and swallow the
context's error for convenience.

In the module implementation specifically, try to avoid scanning if the
cache is enough to satisfy the user. When we do have to scan, prioritize
module dependencies before the whole cache.

Change-Id: I23dc98df016f9fca4f31c7ded3d11bc257c29b94
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212857
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:11:33 +00:00
Heschi Kreinick
c2a8f45ada internal/imports,lsp: use callbacks for completion functions
We only need to return a relatively small number of completions to the
user. There's no point continuing once we have those, so switch the
completion functions to be callback-based, and cancel once we've got
what we want.

Change-Id: Ied199fb1f41346819c7237dfed8251fa3ac73ad7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212634
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:11:27 +00:00
Heschi Kreinick
3f7dfa39cf internal/lsp: sort by label after score
I want to stop sorting unimported completions. We still want to show
users something reasonable, so use label as a tiebreaker for score in
the higher level completion function.

To maintain the current sorting, we need to adjust scores by search
depth (height?) for lexical completions. A few tests are really ties,
and need sorting in the test case.

Change-Id: Ie2d09fdcbebf6fda4ab33a2f16c579d12b0f26ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212633
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:11:21 +00:00
Heschi Kreinick
50c778fb86 internal/imports: redesign scan API
We have multiple use cases for scanning: goimports, import completion,
and unimported completions. All three need slightly different features,
and the latter have very different performance considerations. Scanning
everything all at once and returning it was not good enough for them.

Instead, design the API as a series of callbacks for each
directory/package: first we discover its existence, then we load its
package name, then we load its exports. At each step the caller can
choose whether to proceed with the package. Import completion can stop
before loading exports, goimports can apply its directory name
heuristics, and in the future we'll be able to stop the scan short once
we've found all the results we want for completions.

I don't intend any significant changes here but there may be some little
ones around the edges.

Change-Id: I39c3aa08cc0e4793c280242c342770f62e101364
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212631
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:10:22 +00:00
Heschi Kreinick
fd66c7521c internal/lsp/source: don't get unnecessary unimported completions
Unimported completions are always low-priority. If the user already
has 100 completion options, the unimported ones are probably not useful.
There's no point in calculating any of them.

Also, only do unimported completions for package members when they're
enabled. Oops.

Change-Id: I7535a22ad56bed869dceb6cd0ffdfc6390cf8eb5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212629
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-30 21:10:09 +00:00
Muir Manders
3721262b3e internal/lsp: support taking address for completion candidates
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.

Fixes golang/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>
2019-12-23 23:54:10 +00:00
Muir Manders
4d2fe2ba67 internal/lsp/source: move some data onto "candidate" struct
Make the score and import info be fields on "candidate" since they are
properties of the candidate. There shouldn't be a functional change
here; I'm just consolidating things in preparation for an additional
piece of candidate metadata.

Change-Id: I4c7c8ef40e8e5db7b52691cca21490ba13c17642
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212398
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-23 22:26:30 +00:00
Muir Manders
7bd96bd597 internal/lsp/source: improve completion in value spec
If the enclosing value spec specifies a type on the LHS, we now prefer
completions of that type on the RHS. For example:

i := 123
var foo int = // prefer "i" since we know we want an int

I also added a special case to lexical() to know that we can't offer
objects defined on the LHS as completions on the RHS. For example:

var foo int = // don't offer "foo" as completion

Change-Id: I8e24245a2bc86a29887360e7f642a4cbb87fa6ca
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212401
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-23 21:16:02 +00:00
Rebecca Stambler
2208e1677e internal/lsp: eliminate source.File type and move GetFile to snapshot
This change eliminates the extra step of calling GetFile on the view and
getting the FileHandle from the snapshot. It also eliminiates the
redundant source.File type. Follow up changes will clean up the file
kind handling, since it still exists on the fileBase type.

Change-Id: I635ab8632821b36e062be5151eaab425a5698f60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/211778
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2019-12-19 20:51:25 +00:00
Muir Manders
0f69de236b internal/lsp: offer basic type conversion candidates
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>
2019-12-12 22:41:01 +00:00
Muir Manders
825cb06263 internal/lsp: downrank "nil" completion candidate
Having nil ranked normally causes it to show up as the top candidate
in cases like:

context.WithCancel(<>) // "nil" shows up before "context.Background()"

"context.Background()" gets a slight score penalty since it is a deep
completion, so "nil" is ranked highest.

Sometimes you do want "nil", but it's such a short identifier you
probably aren't leaning too heavily on autocompletion. I think it
makes sense to optimize for the case when you want something non-nil.

Change-Id: I537927db2b573535e751380c4cba5c9873dfe524
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210539
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-12 05:12:00 +00:00
Iskander Sharipov
ec14b29651 internal/lsp/source: fix all types in resolveInvalid
This CL teaches lsp to report `**T` instead of `**invalid type`,
`func (badParam) badResult` instead of `func (invalid type) invalid type`, etc.

To do that, we need to detect "invalid type" inside any part of a type.
I've added typeIsValid() function for that.

To simplify type formating code in resolveInvalid(), formatNode
function is added that can also format *ast.StarExpr (of any depth).
Since we already used AST printer in the same file, I
added formatNode function that is now used in both places.
While at it, replaced bytes.Buffer to strings.Builder there.

Change-Id: I3bb84c58c417b175cceefb410e238c48425f7cee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210357
Run-TryBot: Iskander Sharipov <quasilyte@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-11 23:00:51 +00:00
Clint J Edwards
98df123772 internal/lsp: add comment completions for exported vars
This should provide simple name completions for comments
above exported variables.

Can be activated with `ctrl+space` within a comment.

Pretty new, so all help is welcome.

Fixes #34010

Change-Id: I1c8f71baa3beaa22ec5fd9fd4a531284a8d125f3
GitHub-Last-Rev: a9868eb69dc587cb4579268b2c3ae46932702641
GitHub-Pull-Request: golang/tools#166
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197879
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-10 22:11:41 +00:00
Heschi Kreinick
330b9f1384 internal/lsp/source: cap number of unimported completions
Building unimported completions requires re-parsing and formatting at least
some of the file for each one, which adds up. Limit it to 20; I expect
people will just type more rather than scroll through a giant list.

Updates golang/go#36001.

Change-Id: Ib41232b91c327d4b824e6176e30306abf356f5b4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/210198
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2019-12-06 20:21:18 +00:00