This change allows us to hanel cancel messages as they go into the queue, and
cancel messages that are ahead of them in the queue but not being processed yet.
This should reduce the amount of redundant work that we do when we are handling
a cancel storm.
Change-Id: Id1a58991407d75b68d65bacf96350a4dd69d4d2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200766
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
A continuation of CL 202298, only for analysis errors.
Change-Id: I957d52cef31938ef66be73463e92695a5b56869c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202540
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Since a user's module cache is generally going to be much bigger than
their main module, one would expect that caching just information about
the module cache would be sufficient. It turns out that's not correct.
When we discover something in the module cache, we have to make sure
that a different version of it isn't already in scope. Doing that can
require information about the main module or replace targets, so that
needs to be cached too.
Concretely, when I'm working in x/tools, if a scan discovers a version
of x/tools in the module cache, it should usually ignore that version.
But that might not be true in more complicated cases, especially those
involving nested modules whose boundaries change.
So, cache everything except GOROOT. Since the new data is mutable,
we store it separately from the module cache data so that it can be
discarded easily between runs.
Change-Id: I47364f6c0270fee03af8898fec6c85d1b9c8d780
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202045
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Scan most sources, including GOPATH, the module cache, the main module,
and replace targets as appropriate. Use the cached stdlib instead of
scanning GOROOT.
We heavily cache the contents of the module cache, so performance is
decent. But we have to look at all the modules not in the module cache
too to get the right versions of modules (see
(*ModuleResolver).canonicalize), which currently isn't cached at all,
even just for a single run. That ends up being pretty expensive.
The implementation changes are relatively small; add package name
loading to scan(), cache that result, and allow callers to control what
directories are scanned so that it can skip GOROOT.
I also cleared out most of the stdlib from the unimported completion
test and added a simple external completion to it for safety's sake.
Change-Id: Id50fd4703b1126be35a000fe90719e19c3ab84bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199178
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This function was removed in CL 202298, but used in CL 200597.
Analysis diagnostics should be converted to the source.Error type in the
analysis runner as a complete fix, but this is fine for now.
Change-Id: Ie5f3f566719073d7df6ab4646f855c9f9ce22ad7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202539
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Dominik Honnef <dominik@honnef.co>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL adds support for "related information", which allows
associating additional source positions and messages with a
diagnostic.
Change-Id: Ifc0634f68c9f3724b6508dc6331c62c819a24f78
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200597
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change adds a source.Error type which is used to collect the error
information that comes out of the loading, parsing, and type checking
stages. We also add specific sources per-error, rather than having them
all be labeled as "LSP".
This change will enable follow-ups that do a better job of extracting
error ranges.
Change-Id: I3fbb5e42d66aa2c5bb1b2f41d1eadfc45f3a749b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202298
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We were caching the package before setting the handle, so a
checkPackageHandle might be in use before it was fully created.
Change-Id: Ic98e7c351cbed5e4caa87098e95ad04d4f54f8df
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202040
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
A package really should always be associated with its snapshot rather
than its view. This eliminates some extra parameters in a few utility
functions.
Change-Id: I60f9b7286e0072d3268602f6bd32052a3d2e5559
Reviewed-on: https://go-review.googlesource.com/c/tools/+/202039
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently array and slice literals don't work very well for
completion. When go/parser is not expecting a type, it often turns
array types (e.g. "[]int") into *ast.BadExpr, which messes up
completion because we can't figure out the prefix from *ast.BadExpr,
and *ast.BadExprs don't get type checked.
This change addresses the first problem of not being able to figure
out the prefix. If we see an *ast.BadExpr, we now blindly try to
reparse it as a composite literal by adding on "{}". If we end up with
an *ast.CompositeLit with an *ast.ArrayType "Type", we swap
the *ast.BadExpr for the *ast.ArrayType. This approach is dumb but
simple, and fixes lexical completions in array types.
Change-Id: Ifa42e646bcbf2a30170d73e6dd11982384d40b43
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197437
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
There was a regression where gopls would not type-check any package with
a bad import. This change fixes the regression and adds a test to make
sure it doesn't happen again.
Change-Id: I3acf0917d46e9444c20135559f057f0ecd20e15b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201539
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is specifically necessary to test CL 197879.
Change-Id: I2b4bbdd322d52097fc1444242d3e26a3d8ea75e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201520
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We now continue deep completion search across function calls. The
function must take no arguments and return a single argument. For
example, when completing "fo<>" you might get candidates such as
"foo.bar().baz()".
Previously we would stop searching for deep completions when we hit a
function call. For example, we would stop at "foo.bar()", never
finding "foo.bar().baz()". At the time I was worried about the search
scope growing too large, but now that we dynamically limit the search
scope there isn't much left to worry about.
Change-Id: I48772c154400662876682503c1f58ef6e3dca688
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201222
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: 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>
Now we offer completion candidates for labels when completing "break",
"continue", and "goto" statements. We are reasonably smart about
filtering unusable labels, except we don't filter "goto" candidates
that jump across variable definitions.
Fixesgolang/go#33987.
Change-Id: If296a7579845aba5d86c7050ab195c35d4b147ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Recently been noticing errors where we don't have full metadata for a
given package. It seems to me that, since we added the context to the
packages.Config, there have been cases where the context is canceled on
the first load, and then we type-check with incomplete data. I'm still
not sure if allowing go/packages to be canceled is the correct approach.
Change-Id: I6767ce763538bd579458c8f8db07f15c9eec7b4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201518
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Muir Manders <muir@mnd.rs>
Reviewed-by: Michael Matloob <matloob@golang.org>
This span resulted in misleading information, since it would appear any
time you called the Check function. This function often returns cached
results, so it's not particularly useful.
Change-Id: I26be652a99b906fb2e8703ff9af86fbe2ef5fc5d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201219
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The -rpc.trace flag in gopls enables logging in "lsp inspector format".
Currently sent responses isn't parsed by the lsp inspector so it fails to
parse gopls logs.
The lsp inspector regexp matcher requires the duration to be prefixed with
"took" instead of "in", e.g. "took <d>ms.".
This change updates gopls to match the log parser in lsp inspector,
see 9aff7a6939/lsp-inspector/src/logParser/rawLogParser.ts (L88)
Change-Id: I3696faf34ba4f0b3d4e205693eaf378941f2f68f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199517
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Analyzer names are not guaranteed to be unique.
Change-Id: I4d4cc9fa746cbfbea9926f2cae0eb5dfc41027a5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201217
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change makes sure that actionHandles are cached within the
snapshot, to minimize the cost of re-computing action handles on each
run of go/analysis.
Change-Id: If6191c5224285eb207aebb997787ea551a47fbaa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/201098
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change moves to the approach of caching the analysis using the
memoize package. This means that we will do less work, as we no longer
need to recompute results that are unchanged. The cache key for an
analysis is simply the key of the CheckPackageHandle, along with the
name of the analyzer.
Change-Id: I0e589ccf088ff1de5670401b7207ffa77a254ceb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200817
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Some notifications, like exit, have no other arguments, a case the code
was not handling correctly.
Change-Id: Ib5ff536352ba6ad0d09cd80c9d5f0a987abbe500
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200298
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Peter Weinberger <pjw@google.com>
Include the context when invoking go/packages.Load so it is
cancelable. Otherwise if the user is manually typing an import or
otherwise messing around with imports, a big queue of potentially very
slow go/packages.Load calls will build up.
Fixesgolang/go#34414.
Change-Id: I80732086b478b908c720739708dd773e81fe2b81
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200058
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This will prevent us from panicking in cases with errors.
Fixesgolang/go#34824
Change-Id: I02c20655f6926ec00c1591a905ff5a107cc44192
Reviewed-on: https://go-review.googlesource.com/c/tools/+/200300
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We don't want to support sub line edits or non line context in the unified diff
printing.
Change-Id: I4267b11b50f12d817dac0fbbae7e1db44ca3dad0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199739
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
also improve the actual verification of the unified diff
Change-Id: I9c23c24e1fc8571cce2a7879463659ec7069fe99
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199738
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We used to return an error from textDocument/highlight if the cursor
wasn't over an identifier. Logging these errors is not useful, as the
cursor is often not on an identifier.
Change-Id: Ibb43908149315c72923a22bdca567aa2b3ee68d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/199640
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This should be a faster but equivalent implementation.
Change-Id: I7bc756644c601b953ba7715e093bfa10ca5ea97b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198878
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Make sure everything is documented and move things to sensible files now all the
cross package shuffling is done
Change-Id: I884053a207d6741cda066afa5da91b00f1dfd31c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198877
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The only exposed symbol is now the ComputeEdits function, all other functionality
is now moved up to the diff layer and using edits instead of operations.
Change-Id: I149e4f3276592e1a7c2c52e6eaffc826cc22a9fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198518
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
It is now in the diff package written in terms of edits, instead of the myers package.
This also means that the unified handling is no longer pluggable because it does
not need to be.
Change-Id: I7141b023e95ed0c1d21cbc81c7420c117fc5ef1a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198517
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This now checks at the diff.TextEdit layer rather than myers.Op
Change-Id: I706657a6c5705f0ad7109aa81f4dce174dda5f2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198380
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This makes it so the diff package is depended on by all implementations, rather
than the diff package having to depend on the default myers implementation.
Change-Id: I04b9caee6ff1017fa8e5476a7434e4b0e17753c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198379
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change includes dependencies in the cache keys for
CheckPackageHandles. This should fix the issue with propagating results
to reverse dependencies.
Updates golang/go#34410
Change-Id: I025b7f0d6b0dcaa89c3461ebd94eadd35de4955f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198317
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We only need one implementation of this, it must cope with all inputs, and it
has no freedom in it's results, so it does not need to be pluggable.
Change-Id: I6fec0c339eb288649a670fc3e2cb00c726467e20
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198377
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Looks like I dropped this line accidentally after resolving a merge
conflict. I restored the original code checking the "deep" option in
addition to my intended drive-by fix of also checking the "fuzzy"
option. In either case the client should not cache completion
candidates.
Change-Id: I586daa28e3e4e4cc64665ba507245be4e91b08f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198490
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In cases like "fmt.Pr<>int()" we previously would replace "Print" with
the new completion, yielding for example "fmt.Println()". Now we no
longer overwrite, yielding "fmt.Println()int()". There are some cases
where overwriting the suffix is what the user wants, but it is hard to
tell, so for now stick with the more expected behavior of not
overwriting.
Fixesgolang/go#34011.
Change-Id: I8c3ccd8948245c27b52408ad508d8e01dc163ef4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196119
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change does not complete the work to handle snapshots correctly,
but it does implement the behavior of re-building the snapshot on each
file invalidation.
It also moves to the approach of caching the FileHandles on the snapshot,
rather than in the goFile object, which is now not necessary.
Finally, this change shifts the logic of metadata invalidation into the
content invalidation step, so there is less logic to decide if we should
re-load a package or not.
Change-Id: I18387c385fb070da4db1302bf97035ce6328b5c3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197799
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The log code left out the case where a notification had no parameters at all.
Change-Id: I1d832edb7b7e85422ef6baba1e05286e69dd0cde
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197724
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This allows us to run a single failing test easily when we need.
It also improves the ability to determine which test fails.
Change-Id: I400212d8c4d8c1f97059260add635ce2015990a1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197737
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The loops are common to all the testing layers, so lift them.
This prepares for more test improvements, without any funcitonal changes.
Change-Id: Ib750c8a7bb4c424a185cb0bd841674a69db1385b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197717
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This makes it much easier to keep them up to date.
It is also less fragile against accidental changes.
Change-Id: If119f8527c0896d210650859960e77f3e0fa5a99
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197505
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In CL 192137 deep fuzzy matching was enabled by default. We also have
options independent options "deepCompletion" and "fuzzyMatching" to
control this. When fuzzy matching is disabled, case insensitive prefix
matching is used.
Provide an option, "caseSensitiveCompletion", which allows for case
sensitive prefix matching when fuzzy matching is disabled.
Change-Id: I17c8fa310b2ef79e36cc2f7303e98870690b5903
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194757
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change uses the missing imports detection to return diagnostics and
warning messages to the user.
Updates golang/go#34484
Change-Id: If7bb7e702b8bdbd7e1ad5e26f93acc50d16209b0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196985
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Now we always expect type names inside of *ast.FieldList. This expands
the previous func signature logic to also work for *ast.StructType
and *ast.InterfaceType. For example, we will now prefer type names in
cases like:
type myStruct struct { i i<> }
Also, fix a check for anonymous fields to make sure the field is
actually embedded. This fixes cases like this to properly have no
completions:
type myStruct struct { i<> i }
where this will still give type name completions:
type myStruct struct { i<> }
I introduced a new error type source.ErrIsDefinition so source_test.go
could avoid erroring out on tests that make sure definition
identifiers have no completions.
Fixesgolang/go#34412.
Change-Id: Ib56cb52af639f2e2b132274d1f04f8074c0d9353
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196560
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fix objects defined in the function signature to only be completable
inside the function body. For example:
func (dog Dog) bark(d<>) { // Don't complete <> to "dog".
d<> // Do complete <> to "dog".
}
Change-Id: Ic9a2dc2ce6771212780f2d6af2221a67d203f35f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196559
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This prevents piping back to the file, a common pattern.
Multi file forms should use the unified diff.
Change-Id: I1ea140c59de24feb74a64b0cb41890536f23cd3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197157
Run-TryBot: Ian Cottrell <iancottrell@google.com>
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>
Suggested fixes was totally broken (invalid command) and many others were not in
correct sorted order.
There were lots of golden entries that were no longer used.
The regeneration script itself was broken
The definition tests are skipped, so the entries were not regenerated.
Files must have been hand edited, we probably need to document how to generate
them somewhere.
Change-Id: I1c021aeadd81f08f4572c2124f0c61912a3cd89e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196987
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
I was messing around with the new diagnostic tags feature and
had to update to get it to work.
Change-Id: I4294513b460ec4806d23af20bf908cee8673f7c8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197117
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
This change moves from caching package information the file object to
caching in a map that gets invalidated when content changes.
This simplifies cache invalidation and reduces the number of fields
guarded by the (*goFile).mu lock.
Change-Id: I33fef6e0b18badb97e49052d9d6e3c15047c4b63
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196984
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Instead of relying on the diagnostics cached on the package, use the
diagnostics sent by the code action when computing suggested fixes.
Change-Id: I77f7fd468b34b824c6c5000a51edbe0f8cc6f637
Reviewed-on: https://go-review.googlesource.com/c/tools/+/197097
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Our completion tests check for a lot of different behaviors. It may be
easier to develop if we have separate tests for things like deep
completion and completion snippets.
Change-Id: I7f4b0c0e52670f2a6c00247199933fd1ffa0096f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196021
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Currently, we cache source.CheckPackageHandles for each file and package
that we are aware of, as well as dependencies. This is not necessary,
since the active packages pin their imports in memory.
Change-Id: Ia0101f4d4a2d36d5baeb890af3d7c8baec297847
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196982
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This allows them to be run from the gopls module as well to test
the code with the hooks installed.
Change-Id: I3079a04ffe3bd221ccc2523e746cbed384e05e2f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196321
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This change shifts our approach to make sure that a top-level package
only ever imports "trimmed" packages.
Change-Id: I63c35791ef6efad7dac248a9ff877835f46df9ed
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196523
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This will allow view configuration to modify the set of analyzers being applied, and also allow the main gopls to inject new analyzers
Change-Id: Ic2a76118c3e29b059e19b31bd1fb54b1d9e15e54
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196320
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This commit adds support for calling rename from the gopls command
line, e.g.
$ gopls rename -w ~/tmp/foo/main.go:8:6
$ gopls rename -w ~/tmp/foo/main.go:#53
Optional arguments are:
- -w, which writes the changes back to the original file; and
- -d, which prints a unified diff to stdout
With no arguments, the changed files are printed to stdout.
It:
- adds internal/lsp/cmd/rename.go, which implements the command;
- adds "rename" to the list of commands in internal/lsp/cmd/cmd.go;
- removes the dummy test from internal/lsp/cmd/cmd_test.go; and
- adds internal/lsp/cmd/rename_test.go, which uses the existing
"golden" data to implement its tests.
Updates #32875
Change-Id: I5cab5a40b4aa26357b26b0caf4ed54dbd2284d0f
GitHub-Last-Rev: fe853d325ef91f8f911987790fcba7a5a777b6ce
GitHub-Pull-Request: golang/tools#157
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194878
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This changes adds basic support for running `go mod tidy` as a code
action when a user opens a go.mod file. When we have a command
available like `go mod tidy -check`, we will be able to return edits as
part of the codeAction. For now, we execute the command directly.
This change also required a few modifications to our handling of file
kinds so that we could distinguish between a Go file and a go.mod file.
Change-Id: I343079b8886724b67f90a314e45639545a34f21e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196322
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This change encodes an invariant that, a dependency package will only
ever be parsed with trimmed ASTs.
Updates golang/go#34410
Change-Id: I2ceab3672c0bae0b98cec2a8e60b92a0c01a900f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In it previous implementation, tool.Main was meant to be called from
an applications main(). If it encountered an error it would print the
error to standard error and exit with a non-zero status (2).
It is also called recursively and in various test functions.
Exiting on an error makes testing difficult, unnecessarily.
This change breaks the functionality into to parts: an outer
tool.MustMain() that is intended to be called by main() functions and
an inner Main that is used by MustMain() and by test functions.
None of the existing test functions use Main()'s error value, but the
failure case tests for the command line invocation of rename (#194878)
require it.
Fixes#34291
Change-Id: Id0d80fc4670d56c87398b86b1ad9fdf7a676c95b
GitHub-Last-Rev: cd64995c91c94b997754c8d8b1004afc488bf8b7
GitHub-Pull-Request: golang/tools#159
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195338
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We were recursing infinitely in cases like this:
switch true {
case true:
go foo.F<>
}
There were three things that came together to cause this:
1. We recently starting recursively fixing broken go/defer statements.
2. In this case we were failing to swap in the correct ast.Node in for
the *ast.BadStmt because we were only looking
for *ast.BlockStmt (and *ast.CaseStmt has no block).
3. After 2), we weren't returning an error so the fix() code thought
it should recurse.
Fix 2) by using reflection to swap AST nodes in a generic way. Perhaps
a bit overkill in this case, but I happened to have already written
this for an upcoming change, so I just pulled it in to fix this bug.
Fix 3) by returning an error if we fail to swap the AST nodes.
Fixesgolang/go#34353.
Change-Id: I17ff1afd52ae165c0ba9de5820dcec4cb7d756cb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196137
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>
This was causing microsoft/vscode-go#2749, which was a result of users
having folders named "go", and VSCode-Go returning the "go"
configuration settings (which are all of its settings, not those of
gopls).
Change-Id: Ifbde3e32ad2de79265ed6adea53588c730ecd716
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196197
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
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>
The sortslice analyzer warns the user when they're
using a non-slice parameter to sort.Slice. It can
also suggest fixes for 3 different cases of misuse:
1. The parameter is a pointer to a slice.
2. The parameter is an array
3. The parameter is a function with no arguments
that returns a slice.
Change-Id: If6327b19ab3c476cf17ca3c5fd75bc43bb8683e9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195898
Run-TryBot: Johan Brandhorst <johan.brandhorst@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
If we encounter `go list` errors when loading a user's package, we
should try to see if they've encountered any of our common error cases.
They are: 1) a user has GO111MODULE=off, but is outside of their GOPATH,
and 2) a user is in module mode but doesn't have a go.mod file.
Fortunately, go/packages does a great job handling edge cases so gopls
will work well for most of them. The main issue will be unresolved
imports. These show up in DepErrors in `go list`, so go/packages doesn't
propagate them through to the list of errors. This will require changes
to go/packages.
Updates golang/go#31668
Change-Id: Ibd5253b33b38caffeaad54a403c74c0b861fcc14
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194018
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Parse errors need to be treated separately from actual errors when
parsing a file. Parse errors are treated more like values, whereas
actual errors should not be propagated to the user. This enables us to
delete some of the special handling for context.Canceled errors.
Change-Id: I93a02f22b3f54beccbd6bcf26f04bb8da0202c25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195997
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Also, we were repeating the same code for diagnostics in 4 places.
Extract into a function.
Change-Id: I18d00e512a67679b915347a61126970cf02a7a4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196019
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
We had too many options for functions to use to get type information for
a package. Now we stick with having one option to get the check package
handles, and then the caller can refine the results as needed.
Change-Id: I81f69a670e1539854ee23b6f364159a7de9b782f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This is to remove the confusion around having only handles that have had Get
called pin the value into memory.
Instead now there is a single handle per key, and it is the handle that is
weakly held not the value.
Change-Id: I9e813a0dfe2adf4cb651af9b5cfc8878fa71c041
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186839
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This function incorrectly used cached packages to get ASTs and type
information that should have been directly found from the origin
package. Shift to using pkg.FindFile instead.
Change-Id: I9f73209bb1a1343f53b430150e78ffd180e14a44
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195797
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Tweak a couple things to improve how we reduce our search scope based
on remaining time budget:
- Check our budget on the first candidate rather than waiting for the
1000th candidate. If type checking is slow you can be out of budget
before you even begin.
- Reduce our budget check interval from 1000 candidates to 100
candidates. This just helps us adjust our search scope faster.
The first tweak required me to raise the completion budget for tests
because 100ms is not always enough. I moved the budget into the
completion options so that tests can raise it.
Change-Id: I1aa7909d7baf9c998bc830c960dc579eb33db12a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195419
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Revert my previous change to include fuzzy matches with a score of
zero. Zero scorers have some characters that match, but they are
pretty poor overall. Pulling in all the extra junk candidates was
slowing things down in certain cases.
Change-Id: I560f46903281f21b0628c9b14848cddf1e3c0a38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195418
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change allows to remove some of the special handling for the
builtin package.
Change-Id: I105fcefd8812af2d42ff42edca954824c98db429
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195758
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
A mapper is always uniquely tied to a file at a specific version, so
just build it when we get a new *ast.File. We build the mapper using the
*token.File associated with the particular *ast.File, which is why there
is one per ParseGoHandle instead of FileHandle.
Change-Id: Ida40981ef91f6133cdd07e9793337fcd67510fba
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194517
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Previously we would type check all package handles as we fetched them.
This meant if you only cared about a file's primary package you would
still have to wait for all its packages to be type checked. For
example, when completing in foo.go, you would wait for [foo.go] and
[foo.go, foo_test.go] to be checked (the latter being the test
variant).
Now we don't type check packages as we put together the package
handles.
Change-Id: Ibca0c6b34cf4f0a07bcdeb62959d60158f4ccf17
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195417
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
