This change fixes x/tools tests before CL 298650 lands. After that CL,
the go command reports a different error message when GO111MODULE=on
and it is invoked without a go.mod file (with a command that requires
a go.mod file). We plan to backport that change to 1.16.
For golang/go#44745
Change-Id: Idb3e146828703c89f788ae660ffc95aef16433e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/298792
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
Now that we're generating quick fixes at analysis time, we can use those
in code action requests and delete a fair amount of redundancy. The
codeAction function is a little cluttered, but I want to get it all in
one place before I decide how to split it up.
Change-Id: Icd91e2547542cce0a05c18c02a088833f71232a4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/297532
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: Rebecca Stambler <rstambler@golang.org>
Type error analyzers can be viewed as enhancing type errors, rather
than analyzers in their own right. Create a source.DiagnosePackage
function that combines the list/parse/typecheck diagnostics with type
error analyzers. This allows us to remove some special cases from the
analysis path, and is a first step in removing all the special
handling for analysis quick fixes.
Along the way:
Pass pointers to source.Analyzer after I spent half an hour chasing a
loop capture bug. Spend a further 2-3 hours chasing slowdown in the
command tests as a result.
Move Unnecessary tag generation into diagnostic creation rather than
as a mutating post-processing step that required cloning diagnostics.
Change-Id: Id246667a9dcf484dc79516f92d5524261c435794
Reviewed-on: https://go-review.googlesource.com/c/tools/+/297879
Trust: Heschi Kreinick <heschi@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Most callers of source.Package.GetDiagnostics do it via
GetTypeCheckDiagnostics. Push its logic up or down as appropriate and
delete it.
Rather than requiring fully populated maps of diagnostics, which was
rather subtle, call storeDiagnostics for every Go file in the package.
Change-Id: If43b0cc922af1013e80f969362246538df14985b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/297878
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>
In CL 295413 we fixed the handling of related type checker diagnostics
to correctly identify the primary and secondary errors at a position.
However, on clients that don't support diagnostic related information,
this can lead to confusing primary diagnostics.
Add handling for clients that don't support related information, to
embed the secondary error in the primary error.
Fixesgolang/go#44735
Change-Id: I3d2470d2a4044661e6ed31ac9ffd2f9ff27f7d4b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/297875
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>
CL 295414 suppressed all type checking errors in the presence of parser
errors, but this was a bit overzealous. go/types attempts to suppress
follow-on errors from bad syntax, so as long as we haven't had to modify
the AST and the current file parses, type checking errors can still
provide a decent signal to the user
Fixesgolang/go#44736
Change-Id: Icd89404fbae663b8f934a38908eaaa87c561b64a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/297872
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>
In practice, the only code shared among the various switch cases was the
call to spanToRange, which definitely doesn't justify the giant
function. Split it out into per-type functions.
I removed the unused case for types.Error, and a bit of error checking I
believe to be redundant. I don't intend any functional changes.
At this point it might be worth considering moving the functions into
other files, but I don't think it matters that much.
Change-Id: I05b4d86dd37a9ff1887a116183c915c225faf3a7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/297129
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: Rebecca Stambler <rstambler@golang.org>
Hopefully improve some of the details around parsing that have always
confused me.
- parser.ParseFile will never return an error other than
scanner.ErrorList. Encode that in ParsedGoFile.
- parser.ParseFile will never return a nil file. Eliminate the code
path that handled that.
- Explain why we might fail to find a token.File.
- Trying to fix errors appears quite expensive even if there aren't any
to fix. Don't waste the time.
Change-Id: I87e082ed52c98a438bc7fd3b29e1a486f32fb347
Reviewed-on: https://go-review.googlesource.com/c/tools/+/297069
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: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When a //go:embed or //go:build (//+build) line changes, we need to
invalidate metadata. Do so. It might be preferable to only invalidate on
save, but we don't currently have an approach for doing that. So for now
we'll load on each keystroke in a magic comment.
Fixesgolang/go#38732, golang/go#44342.
Change-Id: Id05fb84f44215ea6242a7cf8b2bca4e85f74680e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/296549
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: Rebecca Stambler <rstambler@golang.org>
go/packages.Error has filenames relative to the go command's working
directory. We need to interpret them as such.
This would perhaps be better done in go/packages but with no release
process in place I'm leery of making changes to it.
Updates golang/go#44342.
Change-Id: I95bcdff0368efe09ec7059394e59a39bf195310b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/295412
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: Rebecca Stambler <rstambler@golang.org>
In typeCheckDiagnostics, we have logic to suppress go list and type
checking errors based on the presence of other errors. Nobody seems to
know why the logic is exactly what it is, and suppressing list errors
means that bad //go:embed patterns don't show up.
Intuitively, it makes sense to me that we don't want to type check if
listing or parsing fails: list errors mean that whole files may be
missing, and parsing errors may wipe out arbitrary chunks of files.
There's little point reporting errors from type checking that. However,
list errors and parse errors should be mostly orthogonal: go list
parses very little of the file and in practice only reports errors
parsing the package statement. So, at least for now, we report both
parse and list errors, and stop there if there are any.
Finally, move the suppression logic to the actual typeCheck function
that generates the diagnostics. typeCheckDiagnostics is the primary
consumer, but I think it's better to do the suppression at the source.
Because we are now showing list errors, and they are prone to getting
stuck due to bad overlay support, a couple of tests now require 1.16.
Change-Id: I7801e44c4d3da30bda61e0c1710a2f52de6215f5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/295414
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>
Due to misuse of the expandErrors function, when we encountered a
continuing error, we would group all the related errors into a single
one, then return both it and all its secondaries individually. That
makes for a strange user experience where you get one complete error and
N-1 incomplete ones.
Instead, create a copy of the complete error for each secondary error,
moving the copy to the location of the secondary error and adding a bit
to the text to (hopefully) clarify what's going on.
Change-Id: I87cf0e3b2cea98317f650d16a78476edf108a934
Reviewed-on: https://go-review.googlesource.com/c/tools/+/295413
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: Rebecca Stambler <rstambler@golang.org>
### gopls
- Add `https` scheme to `CodeDescription.href`
According to the [LSP specification](https://microsoft.github.io/language-server-protocol/specification#diagnostic), `CodeDescription.href` must be [URI type](https://microsoft.github.io/language-server-protocol/specification#uri). As described in the [RFC](https://tools.ietf.org/html/rfc3986#section-3), the scheme is required for URI:
> The scheme and path components are required, though the path may be empty (no characters).
Current `gopls` does not add the scheme to `CodeDescription.href`, and this results in some LSP clients ([at least this client](https://github.com/autozimu/LanguageClient-neovim)) which are strictly validating the URI format to be failed to populate diagnostics.
Change-Id: I73f01c2e97ed1adb62fbed451a7c9b0c9794b66a
GitHub-Last-Rev: 6985bfe60e182ee788082d8fcb9515275d9612fa
GitHub-Pull-Request: golang/tools#277
Reviewed-on: https://go-review.googlesource.com/c/tools/+/294569
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>
Our handling of go command errors was cobbled together, leading to
unexpected gaps and duplication. Refactor it to be more coherent.
Our goal is to turn every go command error into a diagnostic in the
relevant location. The errors don't contain error positions, so we have
to guess where they belong using the module names mentioned in the
error. If we can't find any reference to those modules, we are forced to
add diagnostics to all go.mod files.
I may have destroyed the intent of TestMultiModule_OneBrokenModule but
I'm not sure what to do about it.
Some cleanup along the way:
- Stop parsing modfile.Parse error text: it returns structured errors
and we can just use them.
- Return CriticalErrors from awaitLoadedAllErrors, and do error
extraction lower in the stack. This prevents a ridiculous situation
where initialize formed a CriticalError, then awaitLoadedAllErrors
returned just its MainError, and then GetCriticalError parsed out
a new CriticalError from the MainError we got from a CriticalError.
- In initialize, return modDiagnostics even if load succeeds: we are
missing packages and should not silently fail, I think?
- During testing I tripped over ApplyQuickFixes' willingness to not
actually do anything, so I made that an error.
Fixesgolang/go#44132.
I may also have fixedgolang/go#44204 but I haven't checked.
Change-Id: Ibf819d0f044d4f99795978a28b18915893e50c88
Reviewed-on: https://go-review.googlesource.com/c/tools/+/291192
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: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
findFile has a case that returns early if the file does not exist.
Handle this error in getFile to avoid inconsistently returning errors
when getting file handles for files that don't exist.
Unskip the test, since it is no longer flaky.
Fixesgolang/go#44227
Change-Id: I07a4f860cfc9f852728c31706bd924e419bd54e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/291391
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>
Iterating through the map means that the ordering changes, which may
result in us creating different workspace module files for the same
workspace. We should be careful to always create the same file.
Change-Id: I4ccd3f9ebbbe81bb062285fe9c3ad675bdf2e53a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/291493
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: Robert Findley <rfindley@google.com>
We currently write directly to go.mod/sum via the go command, expecting
that editors will pick up the changes. While that's true for VS Code,
vim doesn't necessarily reload unchanged buffers. Change to send
explicit edits instead, but only if the file is open. Behavior when
using Go versions that don't support -modfile is unchanged.
Fixesgolang/go#44035.
Change-Id: Ie4e5cf60673b860f5dfcbdb726bee0efe6aae405
Reviewed-on: https://go-review.googlesource.com/c/tools/+/290189
Trust: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When go.sum updates are needed in experimental workspace module mode, we
don't necessarily know which module needs the correction. As a fix,
apply all of these fixes to each module in the multi-module workspace.
The "add dependency" quick fix also seems to be broken, but I'll fix
that in a separate CL.
Fixesgolang/go#44097
Change-Id: Id4a6013f2aceb6b902dff3118b027f6cb99eb3c1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289772
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>
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>
Allow fixSrc to run multiple times on a file. For example:
func main() {
var s S
if s.<>
}
type S struct {
i int
}
To properly complete at <>, we need to perform two fixes. We must
insert a phantom underscore after "s." to deal with the dangling
selector, and then we must insert phantom "{}" for the "if" statement
to deal with the incomplete "if" block. Previously we stopped at one
fix, but now we allow for up to 10. I added a limit because I am
afraid there are cases where we could get stuck trying to apply the
same fix again and again.
Also, drive-by set isIncomplete=true when we return early in
lsp/completion.go. I have found my editor incorrectly caches this zero
result in certain cases because it thinks results are "complete".
Fixesgolang/go#43471.
Change-Id: Idd34cc164d579fa12a27920dc3afb372799abf26
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289271
Run-TryBot: Muir Manders <muir@mnd.rs>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The only thing that the mod tidy diagnostics use the network for is
adding dependencies, and we already have quick fixes for those. The one
exception is the case covered by TestBadlyVersionedModule, a dependency
that fails to declare one of its own dependencies and therefore requires
an indirect dependency in the workspace module. That only triggers an
error on the dependency's import statement, which the user will never
see.
Fortunately, the go command does expose these problems in the DepsErrors
field of the list response. Add an internal API to access that, and turn
it into diagnostics on both the file and the controlling go.mod.
Refactor the go get diagnostic generation so that it applies to both
modules and packages.
Fixesgolang/go#38462.
Change-Id: Ie2af940087654682a40de9ecfccd46f404a88b60
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289309
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: Rebecca Stambler <rstambler@golang.org>
We still need to pass -mod=mod when AllowModfileModifications is enabled
on 1.16.
Change-Id: I9cecd30914eb52c9faa877cece25d5722b36df79
Reviewed-on: https://go-review.googlesource.com/c/tools/+/289695
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Collapse Diagnostic.Kind, Source, and Category into just Source. Remove
code that converted from Diagnostic to Diagnostic. Notes on the changes
I had to make along the way:
- We used to use Kind to determine Severity. Set Severity when the
Diagnostic is created instead.
- Use constants for Source as much as possible -- we still need to use
Analyzer.Name for analysis diagnostics. It would be nice to break that
dependency so that Source was totally opaque, but that's a separate
issue.
- Introduce a new Source for gc_details, "optimizer details". It was "go
compiler" previously.
- Some of the assignments are a little arbitrary. Is inconsistent
vendoring really a "go list" error?
- GetTypeCheckDiagnostics had code to cope with diagnostics that had no
URI associated with them. We now spread such diagnostics to all files
when they are generated.
- Analyze modifies Diagnostics by adding a Tag to them. That means it
has to own them, so I had it clone them. I would like to push that logic
down to the diagnostics, per the TODO, but that's another CL.
And some observations:
- It's obviously tempting to combine DiagnosticSource and
diagnosticSource, but they mean very different things. I'm open to a
better name for one or the other.
Change-Id: If2e861d6fe16bfd2e5ba216cf7e29cf338d0fd25
Reviewed-on: https://go-review.googlesource.com/c/tools/+/288215
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: Rebecca Stambler <rstambler@golang.org>
source.Error and source.Diagnostic are almost identical types, used
arbitrarily in different parts of the code. This CL is the first step in
cleaning up that redundancy: it deletes the source.Error type.
To do that, I added the fields from source.Error to source.Diagnostic,
and made absolutely no other semantic code changes -- I just renamed
things that were named Error to Diagnostic. With only aesthetic concerns
in play, I hope this CL will be easy to review. The next CL will clean
up all the stupid-looking code that converts a Diagnostic to a
Diagnostic, etc.
Change-Id: I1298cc8144c686b8a37fc2cc106930105e511353
Reviewed-on: https://go-review.googlesource.com/c/tools/+/288214
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The Typescript source is still at version 3.16, but there are new
requests, more detailed client capabilities, and an attempt to be
more specific about ranges of number in the Typescript code.
Vscode defines types integer and uinteger (32-bit signed and unsigned),
so the Go code now uses int32 and uint32.
They've changed the use of TextDocument, so version information is sometimes
missing. cache/session.go:625 was changed correspondingly.
This CL also make CodeAction.Disabled into a pointer.
New requests or notifications:
DidCreateFiles, DidRenameFiles, DidDeleteFiles (notifications)
ShowDocument, WillCreateFiles,WillRenameFiles, WillDeleteFiles (request)
It's a lot of code; I've probably missed something.
Change-Id: I8449ad8473ac00947d0344c5f6133f9bd73b9e10
Reviewed-on: https://go-review.googlesource.com/c/tools/+/286192
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Using structured errors in gopls has proven to be difficult to manage:
it's hard to know whether a given error return is expected to be
structured without any type information. We have mostly eliminated
them; finish the job.
I don't intend any semantic changes here.
I considered eliminating CriticalError altogether, but it does seem
useful to have a convenient bundle for return values. So I left it
alone for now.
Change-Id: I4b5f85a8de9712babffbc1383088151b596bd815
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287792
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: Rebecca Stambler <rstambler@golang.org>
Now that workspace module mode generates a combined go.sum there are
relatively few blockers to enabling -mod=readonly. Fix them and do it.
This CL is a bit of a grab bag, but the fixes are relatively separate. I
can split it into multiple CLs if desired.
- If module A depends on module B at v1.0.0, the go command will want to
upgrade the workspace module from v0.0.0-goplsworkspace to v1.0.0. To
prevent that, use vN.999999.0 as the base pseudoversion, adjusting v0 to
v1 where appropriate. A few test cases needed updating as a result.
- For old Go versions, sort the generated workspace module and
synthesize a go statement from the maximum go version declared in the
workspace.
- Some regtests need go.sum files created.
- matchErrorToModule created incorrect quick fixes: it would try to
download the top-level module mentioned in the error message, not the
one that actually caused the problem. Now it issues quick fixes for the
lowest-level module.
- TestMultiModuleModDiagnostics accidentally included the same module
in the workspace twice. Fix it, and make that an error.
Fixesgolang/go#43346.
Change-Id: I605f762a4d23bedd914241525e64c1b3ecc42150
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287032
Trust: Heschi Kreinick <heschi@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
In CL 271297, I disabled the constantly-running upgrade check, which
removed the upgrade commands for individual dependencies. This seems to
have been a relatively popular feature. Re-introduce it, but requiring
explicit user interaction.
We now run an upgrade check when the user clicks "Check for upgrades".
Those results are stored on the View and used to show diagnostics on
any requires they apply to. Right now we only check the go.mod the user
has open; in multi-module workspaces it might make sense to check all of
them, but I'm not sure.
Fixesgolang/go#42969.
Change-Id: I65205dc99a4fa9daafdb83145b0294b6f3be5336
Reviewed-on: https://go-review.googlesource.com/c/tools/+/286474
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: Rebecca Stambler <rstambler@golang.org>
We were not locking while iterating s.files in snapshot.knownFilesInDir.
All other accesses of s.files appear to be locked, so this should fix
golang/go#43972.
Fixesgolang/go#43972
Change-Id: I01184c3992c91f8beb4a3239f70cc4487a528ec0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/287573
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
As part of investigating golang/go#43023, I noticed that it's possible
for us to show context cancellation as a critical error--we should not
do this.
Change-Id: I1f7cc13a151e0a161d5a4ea4d5b55fba15270b32
Reviewed-on: https://go-review.googlesource.com/c/tools/+/284937
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: Robert Findley <rfindley@google.com>
This change ensures that packages using exec.LookPath or
exec.Command to find or run binaries do not accidentally run
programs from the current directory when they mean to run programs
from the system PATH instead.
Change-Id: I5907aa630ff64012395a7eb472967a477d90f12e
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/949438
Reviewed-by: Katie Hockman <katiehockman@google.com>
Reviewed-by: Russ Cox <rsc@google.com>
Reviewed-on: https://go-review.googlesource.com/c/tools/+/284773
Run-TryBot: Roland Shoemaker <roland@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Our filesystem caching layer uses file modification time to invalidate
file contents. This is an imperfect heuristic, and on certain operating
systems with low resolution filesystem clocks (such as WSL), this can be
broken in practice.
A proper fix would be to just read the file contents directly and rely
on the snapshot to optimize file access, but we don't know that this is
a safe change. Instead, try to reduce the likelihood of false cache hits
by also checking the file size reported by Stat.
For golang/go#43554
Change-Id: I1af384db532725e84fa6f3a2e5469d10b43fee92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/283053
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>
Trust: Robert Findley <rfindley@google.com>
We can't assume that we're in module mode when we rebuild the imports
cache. (This is a long standing bug, but I suspect it was made more
common by my recent CL that refactored this code.)
Fixesgolang/go#43544.
Change-Id: Ie35c43e6b6e496bd2fbf49cf9bf06c28cf1dab80
Reviewed-on: https://go-review.googlesource.com/c/tools/+/282113
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
The existing test didn't catch it because it doesn't look for specific
warning messages.
Change-Id: I1ec7f7a75c1055c960cdd7545331c2fd655e3aa8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/281860
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Trust: Rebecca Stambler <rstambler@golang.org>
Check that some invariants are met when cloning the snapshot, in an
attempt to catch golang/go#43347
For golang/go#43347
Change-Id: I7404509027a1b0b0085133cba4d21d1006a52a57
Reviewed-on: https://go-review.googlesource.com/c/tools/+/280698
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
The fragmentation of the critical error reporting is getting in the way
of small fixes. This change adds a GetCriticalError function that reports
critical errors, while ModTidy and WorkspacePackages no longer report
critical errors. Any function that wants to process critical errors
should call AwaitLoaded.
Some other smaller changes are made to account for these changes. One
change is that we may report multiple *source.Errors, with the
assumption that duplicate diagnostics will be caught by the diagnostic
caching. Also, any `go list` error message that ends with "to add it" is
now considered an error that gets the "add dependency" suggested fix.
Fixesgolang/go#43338Fixesgolang/go#43307
Change-Id: I056b32e0a0495d9a1dcc64f9f5103649a6102645
Reviewed-on: https://go-review.googlesource.com/c/tools/+/280093
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
golang/go#43335 explains the issues with using `go get module@none`,
which will only be resolved in Go 1.17. In the meantime, we use the
go command whenever possible, but if the module is not tidied, we
have to use textual edits instead. This means the go.sum file will not
be accurately updated to remove the dependency, but unfortunately, I
don't believe there is anything that we can do in that case.
Fixesgolang/go#43335
Change-Id: I771f68f34a6136e73e9dd82b692ed4c235c3b293
Reviewed-on: https://go-review.googlesource.com/c/tools/+/279716
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Invalidating the workspace on any change to a go.mod can render a
workspace unusable. Only invalidate when changes are saved.
Additionally, invalidate the workspace if there is a saved change to
tracked go.sum changes.
Fixesgolang/go#42529Fixesgolang/go#42815
Change-Id: I5d903013b33b932eca4998513e3d0a534b2e5a61
Reviewed-on: https://go-review.googlesource.com/c/tools/+/279720
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Package.ID() is far more useful to tag on events, as it disambiguates
multiple packages with the same path.
Change-Id: I03fd69f64641eb17155ca72ed2ef19641b75f004
Reviewed-on: https://go-review.googlesource.com/c/tools/+/279722
Run-TryBot: Robert Findley <rfindley@google.com>
Trust: 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>
We were getting duplicate diagnostics for certain mod tidy diagnostics.
The correct fix is to separate out the generation of such diagnostics
so that duplicates don't occur, but for now, this is the simplest fix.
I wanted to add a test, but my repro case involves `go clean -modcache`
and checking that a version cannot be downloaded, which doesn't work
with the file-based GOPROXY. I will probably have a CL at some point
that cleans up the way these diagnostics are produced.
Change-Id: I542648b4eae7aced15f720db6233c852879f2a26
Reviewed-on: https://go-review.googlesource.com/c/tools/+/278917
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: Robert Findley <rfindley@google.com>
This CL handles the panic in the sergi/go-diff library which has not
yet been resolved. We add an error return to the ComputeEdits function
and return an error if there is a panic. I'm not sure if this is the
best approach, but it does seem better than allowing the server to
crash.
A concern would be that the user wouldn't know why their code wasn't
being formatted, but hopefully they might look through the logs and
notice the error message. At least, other features would continue
working. The best fix will definitely be the fix for the panic, but that
is not yet available.
Threading through the error return was not pretty, but I thought it was
probably worth doing since it could be needed in other situations.
Updates golang/go#42927
Change-Id: I7f0c05eb296ef9e93b4de8ef071301cdb9dce152
Reviewed-on: https://go-review.googlesource.com/c/tools/+/278775
Run-TryBot: 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>
Trust: Rebecca Stambler <rstambler@golang.org>
These were missing titles, which was showing up empty for users in the
VS Code UI and leading people not to trust the fixes.
Also did a find references on SuggestedFix to confirm that we always
set the title in other cases.
Fixesgolang/go#43234
Change-Id: I8d0f272c383a2e1a364aefcec6650988d18d4fb4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/278778
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: Robert Findley <rfindley@google.com>
This change adds a diagnostic and error message that appears when a user
edits a nested module in legacy mode. At first, I thought a diagnostic
would be enough, but it's actually quite difficult to spot it when you
have a bunch of "undeclared name" diagnostics caused by the nested
module, so I figured a progress bar error message would also be useful.
This error message just indicates to the user that they should open the
nested module as its own workspace folder.
Also, while debugging this, I noticed that command-line-arguments
packages can have test variants, which we were never handling.
So I addressed that in this change.
Fixesgolang/go#42109
Change-Id: Ifa6f6af401a3725835c09b76e35f889ec5cb8901
Reviewed-on: https://go-review.googlesource.com/c/tools/+/275554
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
For every file change, we want to diagnose the resulting snapshots.
However, we also want to diagnose the file in its "best possible" view
(for example, for a nested module, we should use the inner module rather
than the outer module).
Add a bestView function that operates on a set of views, which allows
us to a pick a view out of the *views affected* by a file modification,
not out of *all* of the views in the session. This means that we can
pick the best view, if any, for each file change and then only return
the snapshots that we should diagnose, as well as the changed URIs for
those snapshots.
Fixesgolang/go#42890
Change-Id: Iad23d5ed4832dfd857f1424a7244cf3bd8900e3b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/274235
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 "you are neither in a module nor your GOPATH" warning doesn't fit
our current notification style, so adjust it to appear as a progress
report and clean up the associated tests.
Change-Id: I32b96f17f3b9715403e465e3e0f9705f5d859147
Reviewed-on: https://go-review.googlesource.com/c/tools/+/275537
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>
Users working in large repositories may want to include only selected
directories in their workspace to avoid memory usage and performance
slowdowns. Add support for inclusion/exclusion filters that control what
directories are searched for workspace packages and modules. Packages
that are excluded by the filter may still be loaded as non-workspace
packages if other things depend on them.
For a description of the option's syntax, see the documentation.
Note that because we don't have any way to communicate the filters to
packages.Load, we still run go list on the unfiltered workspace scope,
then throw away the irrelevant packages. That may cost us, especially in
workspaces with many files.
Comments on the naming welcome. Also, if you know any places I may have
missed applying the filter, please do tell. One thing I thought of is
file watching, but that's covered because allKnownSubdirs works off of
workspace files and those are already filtered.
Possible enhancements:
- Support glob patterns.
- Apply filters during the goimports scan.
- Figure out how to apply the filters to packages.Load. I don't know
how to do it while still being build system neutral though.
Closesgolang/go#42473, assuming none of the enhancements are required.
Change-Id: I9006a7a361dc3bb3c11f78b05ff84981813035a0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/275253
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: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Jay Conrod
Heschi Kreinick
Rob Findley
Yuki Ito
Rebecca Stambler
Muir Manders
pjw
Roland Shoemaker