When using Go workspaces, the go.work file should be used to determine
which packages are workspace packages.
For golang/go#48929
Change-Id: I1a8753ab7887daf193e093fca5070b4cc250a245
Reviewed-on: https://go-review.googlesource.com/c/tools/+/400822
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This change replaces various uses of FileSet with either
nothing (when the parameter wasn't really needed) or token.File.
Notably, astutil.Imports was being used to extract the imports
of a file (available at ast.File.Imports), forcing a number
of wrappers to have a FileSet parameter.
Also, simplify various expressions file.Position().Line to file.Line().
Change-Id: I19fe86a18aba50352275f77ed737513744d3930c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/410366
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
The TokenConverter has been trimmed down to a thin wrapper around
token.File, and can now be removed.
Change-Id: I9985492274c88e6a13e6d62dadab5595c75c7840
Reviewed-on: https://go-review.googlesource.com/c/tools/+/406134
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Eliminate the need to work around newline terminated files in
completion, by storing selection ranges as token.Pos and using an
lsppos.TokenMapper derived from the file content, which does not have
problems with newline termination.
This simplifies the completion logic, and removes the last use of
MappedRange.SpanRange, which is an inconisitent API in that it returns
positions in the compiled source, rather than edited source.
Change-Id: I65232787956325172b48fd42d85cbb598039ee5a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/407889
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Due to not having a pointer receiver, memoization of the computed
MappedRange.protocolRange had no effect.
Rather than fix the memoization, just remove it since it has apparently
not affected performance significantly.
Change-Id: Ib34becef44dca4074dcc7c8af93883d9a1060f8d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/408716
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
CL 405546 introduced a latent bug in MappedRange, because it naively
used the wrong TokenConverter to convert mapped positions to offsets.
This was detected via related clean-up work in another CL. Fix this by
passing the correct converter from MappedRange.Range. Add a test that
would have demonstrated the breakage.
More cleanup is needed here. It is subtle that MappedRange.Converter
maps the adjusted position for its start and end, and there may be some
places where this invariant has been broken over the years.
Add additional documentation and bug reports.
Change-Id: If7f177894bac1242ddcc1786e79c7559455e9291
Reviewed-on: https://go-review.googlesource.com/c/tools/+/407887
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
The only real implementation of position conversion was via a
*token.File, so refactor the converter logic to eliminate the Converter
interface, and just use a single converter implementation that uses a
*token.File to convert between offsets and positions.
This change is meant to be a zero-impact refactoring for non-test code.
As such, I abstained from panicking in several places where it would
make sense. In later CLs, once the bug reporting API lands, we can
insert bug reports in these places.
Change-Id: Id2e503acd80d089bc5d73e983215784015471f04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405546
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In passing I noticed that this three-way comparison
is not (anti)symmetric. Such comparisons should consist
of a list of pairs of tests of this form:
if x.key1 < y.key1 { return -1 }
if x.key1 > y.key1 { return +1 }
...key2, etc...
return 0
Also in passing:
- simplify panic-proof debug string function.
- augment doc comment of (*Server).beginFileRequest
Change-Id: Idcd0844ea4e96fc2dee5bbc270f5a200b5c27aa0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405480
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
This avoids an import cycle that prevented these wrappers from being
used in the lsppos package.
Change-Id: I9eedd256db983dfcf962edba39e3d4f3a1aabdeb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/403680
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
As of golang/go#50827, gopls no longer supports building at 1.12, and so
usage of golang.org/x/xerrors can be replaced with the native support for
error wrapping introduced in Go 1.13.
Remove this usage as a step toward eliminating the xerrors dependency
from x/tools.
For golang/go#52442
Change-Id: Ibf459cc72402a30a6c2735dc620f76ed8a5e2525
Reviewed-on: https://go-review.googlesource.com/c/tools/+/401097
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Gofmt to update doc comments to the new formatting.
(There are so many files in x/tools I am breaking up the
gofmt'ing into multiple CLs.)
For golang/go#51082.
Change-Id: Ife11502fe1e59a04d53dba9edccd3043e57f9ae8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/399358
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Show the module path of the module being used.
For golang/go#50930
Change-Id: I90a67e12182ffb457876b1fbc95aeb56ff632878
Reviewed-on: https://go-review.googlesource.com/c/tools/+/389301
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Report diagnostics on use lines where the directory doesn't have a
go.mod file, and on syntax errors in go.work.
For golang/go#50930
Change-Id: Idab36b43d86c4842f8eecd5c071ce0587e6f27b3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/389317
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Thread through an adjusted object position when we format an re-parsed
AST that wasn't type-checked to produce the object. Also remove some
unnecessary handling of the object type.
Fixesgolang/go#46158
Change-Id: Ic8bee2bb4cb992d577d085599213fca1deab3b4e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/384697
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Honor the file kind provided by clients for overlays, by passing the
FileHandle into View.FileKind and checking for overlays.
For golang/vscode-go#1957
Change-Id: I4a767cb64dc5205f1d10c3126a2cbe67c21a34e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/378314
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Make the language id (sent from the client) 'gotmpl' equivalent to 'tmpl'
Wherever a view is known, use its options to determine which files
are template files. Whenever the client sends an explicit
languageID, use that.
Partially fixesgolang/vscode-go#1957
Change-Id: I04cd630d6c6c80e0a78c2fafb6ddc1166ce86829
Reviewed-on: https://go-review.googlesource.com/c/tools/+/376854
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This isn't strictly necessary for some of the cases, but it's better to
use it in all cases. Also added a test to ensure that we avoid
(*token.File).Offset in all of gopls--test was probably overkill, but it
was quick to write.
Change-Id: I6dd0126e2211796d5de4e7a389386d7aa81014f0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/353890
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When resolving a position to a package we must consider all packages,
including intermediate test variants. This manifests, for example, when
jumping to definition in a package that is imported as a test variant
(see golang/go#47825).
For now, fix this by threading through an 'includeTestVariants' flag to
PackagesForFile. This isn't pretty, but should be a trivially safe
change: the only effect will be to increase the number of packages
considered in FindPackageFromPos. Since we are discussing future changes
to the API for querying packages from the snapshot, now did not seem
like a good time to undertake significant refactoring.
A regtest based on the original issue is included.
This CL is joint with rstambler@golang.org.
Fixesgolang/go#47825
Co-authored-by: Rebecca Stambler <rstambler@golang.org>
Change-Id: I4693ec69b50ed4acd569cff87883769c1edf332b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/347563
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Check that a position is in range before using it.
Fixesgolang/vscode-go#1656
Change-Id: I1598ebab76a1775afd8f63b9849049b31fb74a8b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339169
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
When we open a file in a package, independent of whether it is in the
workspace, we type check in ParseFull mode. However, several other
code paths don't find this better parse mode.
We need a better abstraction, but for now improve a couple code paths
specifically for the purpose of fixing Hover content.
Updates golang/go#46158
Updates golang/go#46902
Change-Id: I34c0432fdba406d569ea963ab4366336068767a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/333689
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
This CL adds two new commands that let a client request a list of importable packages relative to a Go file and then select which import a programmer would like to add to said file.
Updates golang/go#43351
Change-Id: If12518874a92ed4167bdd711a92e03ee21c7b949
Reviewed-on: https://go-review.googlesource.com/c/tools/+/281412
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Despite the name, ParseExported only hollowed out declarations -- it
didn't actually drop any from the AST. This leaves a fair amount of
unexported crud behind. Unfortunately, there are a *lot* of ways to
expose an unexported declaration from an exported one, and it can be
done across files. Because of that, discarding unexported declarations
requires a lot of work.
This CL implements a decent attempt at pruning as much as possible from
the AST in ParseExported mode.
First, we analyze the AST of all the files in the package for exported
uses of unexported identifiers, iterating to a fixed point. Then, we
type check those ASTs. If there are missing identifiers (probably due to
a bug in the dependency analysis) we use those errors to re-parse. After
that we give up and fall back to the older, less effective trimming. The
pkg type changes slightly to accomodate the new control flow.
We have to analyze all the files at once because an unexported type
might be exposed in another file. Unfortunately, that means we can't
parse a single file at a time any more -- the result of parsing a file
depends on the result of parsing its siblings. To avoid cache
corruption, we have to do the parsing directly in type checking,
uncached.
This, in turn, required changes to the PosTo* functions. Previously,
they operated just on files, but a file name is no longer sufficient to
get a ParseExported AST. Change them to work on Packages instead. I
squeezed in a bit of refactoring while I was touching them.
Change-Id: I61249144ffa43ad645ed38d79e873e3998b0f38d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/312471
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Provide some support for template files, implementing most of
https://docs.google.com/document/d/1clKAywucZVBXvL_v4mMhLQXso59lmQPMk1gtSpkV-Xw
Template support is controlled by the option 'experimentalTemplateSupport'
which defaults to false.
Most of the code is in a new 'template' package. Implemented are
semantic tokens, diagnostics, definitions, hover, and references,
and there is a stub for completions.
This code treats all the template files of a package together, so as
to follow cross-references.
Change-Id: I793606d8a0c9e96a0c015162d68f56b5d8599294
Reviewed-on: https://go-review.googlesource.com/c/tools/+/297871
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Peter Weinberger <pjw@google.com>
The previous fix (d1362d7) is not sufficient for all cyclic types.
This change updates Deref function to support more complex cases.
We use a map with underlying types to detect cycles.
Fixesgolang/go#45510
Change-Id: I28f655a9c1d4f363cb7ae3f47db3e8567fe6e80a
GitHub-Last-Rev: 4c898741c0cf07cb765b759b1edf15e004fc1a0e
GitHub-Pull-Request: golang/tools#305
Reviewed-on: https://go-review.googlesource.com/c/tools/+/310311
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Return a pointer type if the type refers to itself (for example, type a *a).
Fixesgolang/go#45510
Change-Id: Ifaf9c0fe9df8a1cab300479394a7127dfb820a88
GitHub-Last-Rev: 009802341673cfd24c04d6a115a6082029dfa2a2
GitHub-Pull-Request: golang/tools#302
Reviewed-on: https://go-review.googlesource.com/c/tools/+/310050
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Fully switch to the new generated command API, and remove the old
dynamic command configuration.
This involved several steps:
+ Switch the command dispatch in internal/lsp/command.go to go through
the command package. This means that all commands must now use the new
signature.
+ Update commandHandler to use the new command signatures.
+ Fix some errors discovered in the command interface now that we're
actually using it.
+ Regenerate bindings.
+ Update all code lens and suggested fixes to new the new command
constructors.
+ Generate values in the command package to hold command names and the
full set of commands, so that they may be referenced by name.
+ Update any references to command names to use the command package.
+ Delete command metadata from the source package. Rename command.go to
fix.go.
+ Update lsp tests to execute commands directly rather than use an
internal API. This involved a bit of hackery to collect the edits.
+ Update document generation to use command metadata. Documenting the
arguments is left to a later CL.
+ Various small fixes related to the above.
This change is intended to be invisible to users. We have changed the
command signatures, but have not (previously) committed to backwards
compatibility for commands. Notably, the gopls.test and gopls.gc_details
signatures are preserved, as these are the two cases where we are aware
of LSP clients calling them directly, not from a code lens or
diagnostic.
For golang/go#40438
Change-Id: Ie1b92c95d6ce7e2fc25fc029d1f85b942f40e851
Reviewed-on: https://go-review.googlesource.com/c/tools/+/290111
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We currently have partial, untested support for symlinks. Remove all
support for now until we come up with a decision.
I thought we could re-enable the OpenBSD trybot since we think it was
failing because of the latency of evaluating symlinks, but it seems like
it's still failing.
Updates golang/go#42833
Change-Id: Iece1741def4eaaeffe5fecd10f884c1cce87f449
Reviewed-on: https://go-review.googlesource.com/c/tools/+/273190
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
The current DidModifyFiles logic does not handle the possibility of a
directory URI in a didChangeWatchedFiles request. For creation and
changes, this is fine because only file URIs matter in those cases, but
for deletion, the deletion of a directory signifies the deletion of all
of the files in it.
Add handling for directories by looking up all of the known files in a
given directory. We then create file modifications for all of those
files, instead of the directory.
Fixesgolang/go#38422
Change-Id: I26d7b93285367fe116fd1cb3e8ec13a276f9e517
Reviewed-on: https://go-review.googlesource.com/c/tools/+/266138
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
VS Code's file watching API doesn't send notifications about directory
deletions unless you register for them explicitly (see
https://github.com/microsoft/vscode/issues/109754). Rather than watch
every file in the workspace, keep track of every relevant directory and
register file watchers for it.
This CL moves the snapshot's WorkspaceDirectories function to the
session and changes it to a KnownDirectories function. It returns all of
the directories and subdirectories known the session at a given moment.
Top-level directories are marked as such so that their *.{go,mod,sum}
contents can be watched, while subdirectories are just watched by path
so we can be notified of deletions.
Fixesgolang/go#42348
Change-Id: If80a26cf6af5a2c2f7d87bb57b9ef5cb3dcf04ac
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267120
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The logic that checked if a file was already being watched by the
default glob pattern was incorrect. Fix it, and use the newly added
InDir function.
Change-Id: Ia7e3851ab5b9fa1fa7590cae3b370676201a9141
Reviewed-on: https://go-review.googlesource.com/c/tools/+/267119
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change moves package selection to before type checking so we don't
unnecessarily type-check both variants of a package. As a result, exec
time and memory usage for features making calls to GetParsedFile are cut
by half since we only type check either the narrowest or the widest
package.
Change-Id: Ifd076f8c38e33de2bd3509fe17feafccd59d7419
Reviewed-on: https://go-review.googlesource.com/c/tools/+/257240
Run-TryBot: Danish Dua <danishdua@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Danish Dua <danishdua@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Completion is slowly becoming a large part of internal/lsp/source and it
makes sense to move to its own seperate package inside source to make
future refactors easier. As a part of this change, any unexported
members from source required by completion are now exported. Util
functions only required by completion are moved from
internal/lsp/source/util.go to internal/lsp/source/completion/util.go.
Change-Id: I6b7405ec598c910545e649bb0e6aa02ffa653b38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/253178
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
formatZeroValue is currently only used when formatting return values for
statement completion. Per golang/go#40956, it must be possible to hit
this codepath with an invalid type.
In this case, the empty string seems like a reasonable value. Perhaps we
could do better, but fix the panic for now.
Fixesgolang/go#40956
Change-Id: I45b559d41001c857cef34aea2a5ac4a9096fe950
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249818
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
We now rank printf operand candidates according to the corresponding
formatting verb. We follow what fmt allows for the most part, but I
omitted some things because they are uncommon and would result in many
false positives, or didn't seem worth it to support:
- We don't prefer fmt.Stringer or error types for "%x" or "%X".
- We don't prefer pointers for any verbs except "%p".
- We don't prefer recursive application of verbs (e.g. we won't prefer
[]string for "%s").
I decided against sharing code with the printf analyzer. It was
tangled somewhat with go/analysis, and I needed only a very small
subset of the format parsing.
I tweaked candidate type evaluation to accommodate the printf hints.
We now skip expected type of "interface{}" when matching candidate
types because it matches everything and would always supersede the
coarser object kind checks.
Fixesgolang/go#40485.
Change-Id: I6440702e33d5ec85d701f8be65453044b5dab746
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246699
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
CL 248380 forced all type checking to be in the default workspace mode.
In that CL, I said I couldn't think of any features that would break. It
appears I didn't think very hard. Navigation features inside of
dependencies are something I use all the time and they broke.
Reintroduce the ability to get packages in a particular mode, and make
it convenient to get them in all relevant modes. Update some critical
features to do so, and add regression tests.
Fixesgolang/go#40809.
Change-Id: I96279f4ff994203694629ea872795246c410b206
Reviewed-on: https://go-review.googlesource.com/c/tools/+/249120
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
snapshot.View().Session().Cache().FileSet() has been driving me crazy
for a while. Add it to snapshot. Along the way, discover that the Cache
interface is now totally unused and delete it.
I also changed a bunch of View arguments to Snapshot while I was in the
area.
Change-Id: I1064d0020b1567c2ed28d2d55e0f4649eb94c060
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245324
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Just like ParseGoHandle, PackageHandle isn't very useful as part of the
public API. Remove it.
Having PackagesForFile take a URI rather than a FileHandle seems
reasonable, and made me wonder if that logic applies to other calls like
ParseGo. For now I'm going to stop here. I could also revert that part
of the change.
Change-Id: Idba8e9fdba0b0c48e841a698eb97e47fd5f23cf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244637
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
ParseGoHandles serve two purposes: they pin cache entries so that
redundant calculations are cached, and they allow users to obtain the
actual parsed AST. The former is an implementation detail, and the
latter turns out to just be an annoyance.
Parsed Go files are obtained from two places. By far the most common is
from a type checked package. But a type checked package must by
definition have already parsed all the files it contains, so the PGH
is already computed and cannot have failed. Type checked packages can
simply return the parsed file without requiring a separate Check
operation. We do want to pin the cache entries in this case, which I've
done by holding on to the PGH in cache.pkg.
There are some cases where we directly parse a file, such as for the
FoldingRange LSP call, which doesn't need type information. Those parses
can actually fail, so we do need an error check. But we don't need the
PGH; in all cases we are immediately using and discarding it.
So it turns out we don't actually need the PGH type at all, at least not
in the public API. Instead, we can pass around a concrete struct that
has the various pieces of data directly available.
This uncovered a bug in typeCheck: it should fail if it encounters any
real errors.
Change-Id: I203bf2dd79d5d65c01392d69c2cf4f7744fde7fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244021
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Due to the runtime's inability to collect cycles involving finalizers,
we can't close over handles in memoize.Functions without causing memory
leaks. Up until now we've dealt with that by closing over all the bits
of the snapshot that we want, but it distorts the design of all the code
used in the Functions.
We can solve the problem another way: instead of closing over the
snapshot/view, we can force the caller to pass it in. This is somewhat
scary: there is no requirement that the argument matches the data that
we're working with. But the reality is that this is not a new problem:
the Function used to calculate a cache value is not necessarily the one
that the caller expects. As long as the cache key fully identifies all
the inputs to the Function, the output should be correct. And since the
caller used the snapshot/view to calculate that cache key, it should
always be safe to pass in that snapshot/view. If it's not, then we
already had a bug.
The Arg type in memoize is clumsy, but I thought it would be nice to
have at least a little bit of type safety. I'm open to suggestions.
Change-Id: I23f546638b0c66a4698620a986949087211f4762
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244019
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The logic for extracting a function is quite signficant, and the code
is expensive enough that we should only call it when requested by the
user. This means that we should support extracting through a command
rather than text edits in the code action.
To that end, we create a new struct for commands. Features like extract
variable and extract function can supply functions to determine if they
are relevant to the given range, and if so, to generate their text
edits. source.Analyzers now point to Commands, rather than
SuggestedFixFuncs. The "canExtractVariable" and "canExtractFunction"
functions still need improvements, but I think that can be done in a
follow-up.
Change-Id: I9ec894c5abdbb28505a0f84ad7c76aa50977827a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244598
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Our approach to commands and their arguments has been ad-hoc until this
point. This CL creates a standard way of defining and passing the
arguments to different commands. The arguments to a command are now
json.RawMessages, so that we don't have to double encode. This also
allows us to check the expected number of arguments without defining
a struct for every command.
Change-Id: Ic765c9b059e8ec3e1985046d13bf321be21f16ab
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242697
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Completion could be slow due to calls to astutil.PathEnclosingInterval
for every candidate during formatting. There were two reasons we
called PEI:
1. To properly render type alias names, we must refer to the AST
because the alias name is not available in the typed world.
Previously we would call PEI to find the *type.Var's
corresponding *ast.Field, but now we have a PosToField cache that
lets us jump straight from the types.Object's token.Pos to the
corresponding *ast.Field.
2. To display an object's documentation we must refer to the AST. We
need the object's declaring node and any containing ast.Decl. We
now maintain a special PosToDecl cache so we can avoid the PEI call
in this case as well.
We can't use a single cache for both because the *ast.Field's position
is present in both caches (but points to different nodes). The caches
are memoized to defer generation until they are needed and to save
work creating them if the *ast.Files haven't changed.
These changes speed up completing the fields of
github.com/aws/aws-sdk-go/service/ec2 from 18.5s to 45ms on my laptop.
Fixesgolang/go#37450.
Change-Id: I25cc5ea39551db728a2348f346342ebebeddd049
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221021
Run-TryBot: Muir Manders <muir@mnd.rs>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Ignore ignored the builtin package and files that start with _. The
latter should already be ignored by "go list". The former seems
like too much effort to me. People shouldn't edit random parts of the
stdlib, and ignoring changes to (e.g.) the Error interface seems like
the least of the trouble they can get themselves into.
Remove it for now. If we get complains I'll re-add it, probably by
rejecting the write entirely somewhere.
We incidentally relied on this in the identifier functions; change those
to treat the builtin package slightly more specially.
Change-Id: I005b02a66b1a987c50a3074d53a2d28ff07d3324
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237597
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
We use file identities pervasively throughout gopls. Prior to this
change, the identity is the modification date of an unopened file, or
the hash of an opened file. That means that opening a file changes its
identity, which causes unnecessary churn in the cache.
Unfortunately, there isn't an easy way to fix this. Changing the
cache key to something else, such as the modification time, means that
we won't unify cache entries if a change is made and then undone. The
approach here is to read files eagerly in GetFile, so that we know their
hashes immediately. That resolves the churn, but means that we do a ton
of file IO at startup.
Incidental changes:
Remove the FileSystem interface; there was only one implementation and
it added a fair amount of cruft. We have many other places that assume
os.Stat and such work.
Add direct accessors to FileHandle for URI, Kind, and Version. Most uses
of (FileHandle).Identity were for stuff that we derive solely from the
URI, and this helped me disentangle them. It is a *ton* of churn,
though. I can revert it if you want.
Change-Id: Ia2133bc527f71daf81c9d674951726a232ca5bc9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237037
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Robert Findley
Alan Donovan
Russ Cox
Michael Matloob
pjw
Rebecca Stambler
Marwan Sulaiman
Heschi Kreinick
Shoshin Nikita
yangwenmai
Danish Dua
Muir Manders