Apply now checks that its edits are valid
(not out of bounds or overlapping),
and reports an error if not.
It also sorts them, if necessary, using (start, end)
as the key, to ensure that insertions (end=start)
are ordered before deletions at the same point
(but without changing the relative order of insertions).
Two other implementations of the diff.Apply algorithm
have been eliminated. (One of them failed to sort edits,
requiring the protocol sender to do so; that burden
is now gone.)
Change-Id: Ia76e485e6869db4a165835c3312fd14bc7d43db2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/439278
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
The span package is properly part of gopls, and we'd like to move
it into that module. However, the packagestest package unfortunately
depends on span.Span as an alternative notation for Ranges.
This change decouples span.Span from packagestest.Range using a
new (unexported for now) rangeSetter interface, which Span implements.
Neither package depends on the other.
Technically this is a breaking API change:
all the Range methods have gone away, as have the Span, URI,
and Point types and their methods, which were accessible via
Range.Span(). However, clients would not be able to access
these internal types, and I think it is highly unlikely that
anyone depends on it. Nonethless this is a cautionary tale about
the risks from one innocuous-looking type alias declaration.
Change-Id: I8acb03f4acb1f798f304b03648445e37a44f9c45
Reviewed-on: https://go-review.googlesource.com/c/tools/+/439715
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
The analysis driver uses a hack to reconstruct type
errors (which contain a token.Pos) from packages.Errors
(which contain file:line:col strings). This hack will
be obviated by https://go.dev/cl/425095 but in the meantime
it creates a dependency on the span package, which is
really part of gopls.
This change replaces the span.Parse function with simple
calls to the standard library to extract the line and column.
(Dependency aside, span.Parse was always overkill for this task,
but perhaps packages.Error should have a File,Line,Col accessor.)
Change-Id: I55108337f8a753523db68fe9e2aea15bb059cf15
Reviewed-on: https://go-review.googlesource.com/c/tools/+/439755
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Make the test run only with go1.18+.
Change-Id: I8a2645929070bc1a63401ee942de1e88b5c083fa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/439275
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
These functions are now both expressed in terms of OffsetPosition,
which converts a file offset to a protocol (UTF-16) position.
No span.Point intermediaries are allocated.
Also, note some TODOs for further simplification.
Change-Id: I7e95c1b3ab16e4acfe4e461ee0aaa260efb6eec5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/439276
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This package hosts helper utilities for vulncheck features. This package requires go1.18+.
Most of the code were adopted from golang.org/x/vulndb/internal.
The first batch is NewDatabase reads YAML-format vulnerability
information files
(https://github.com/golang/vulndb/blob/master/doc/format.md)
packaged in txtar, and creates a filesystem-based vuln database
that can be used as a data source of golang.org/x/vuln/client
APIs.
See db_test.go for example.
* Source of the code
db.go:
golang.org/x/vulndb/internal/database#Generate
report.go:
golang.org/x/vulndb/internal/report#Report
stdlib.go:
golang.org/x/vulndb/internal/stdlib
This change adds a new dependency on "gopkg.in/yaml.v3"
for parsing YAMLs in testing
Change-Id: Ica5da4284c38a8a9531b1f943deb4288a2058c9b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/435358
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This is to pick up the fix for the file-scheme url handling bug
cd gopls
GOPROXY=direct go get golang.org/x/vuln@2aa0553d353b
go mod tidy -compat=1.16
Also updates the tests to use span.URIFromPath to generate
correct test database file uris.
Change-Id: I6de296cd21f3b98d72700ea57d1aa867658e7ac3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/438756
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff.TextEdit (now called simply Edit) no longer has a Span,
and no longer depends on the span package, which is really
part of gopls. Instead, it records only start/end byte
offsets within an (implied) file.
The diff algorithms have been simplified to avoid the need to
map offsets to line/column numbers (e.g. using a token.File).
All the conditions actually needed by the logic can be derived
by local string operations on the source text.
This change will allow us to move the span package into the
gopls module.
I was expecting that gopls would want to define its own
Span-augmented TextEdit type but, surprisingly, diff.Edit
is quite convenient to use throughout the entire repo:
in all places in gopls that manipulate Edits, the implied
file is obvious. In most cases, less conversion boilerplate
is required than before.
API Notes:
- diff.TextEdit -> Edit (it needn't be text)
- diff.ApplyEdits -> Apply
- source.protocolEditsFromSource is now private
Change-Id: I4d7cef078dfbd189b4aef477f845db320af6c5f6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436781
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Computing the difference between two strings is logically an
infallible operation. This change makes the code reflect that. The
actual failures were unreachable given consistent inputs, but that was
hard to see from the complexity of the logic surrounding span.Span.
(The problem only occurs when converting offsets beyond the end of the
file to Spans, but the code preserves the integrity of offsets.)
gopls' "old" hooks.ComputeEdits impl (based on sergi/go-diff) now
reports a bug and returns a single diff for the entire file if it
panics.
Also, first steps towards simpler API and a
reusable diff package in x/tools:
- add TODO for new API. In particular, the diff package shouldn't care
about filenames, spans, and URIs. These are gopls concerns.
- diff.String is the main diff function.
- diff.Unified prints edits in unified form;
all its internals are now hidden.
- the ComputeEdits func type is moved to gopls (source.DiffFunction)
- remove all non-critical uses of myers.ComputeEdits. The only
remaining one is in gopls' defaults, but perhaps that gets
overridden by the default GoDiff setting in hooks, to BothDiffs
(sergi + pjw impls), so maybe it's now actually unused in practice?
Change-Id: I6ceb5c670897abbf285b243530a7372dfa41edf6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436778
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Diagnostics are populated with vuln details that are already
formatted with newlines to limit line length. The client UI has
its own wrapping logic, so we can remove this to allow the client
to do its own formatting.
Change-Id: Ida0ce71886add995fad7815e6a302d4b44de65e8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436775
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Metadata.IsIntermediateTestVariant can be derived from PkgPath and
ForTest, so make it a method. Along the way, document the easily missed
fact that intermediate test variants are not workspace packages.
For golang/go#55293
Change-Id: Ie03011aef9c91ebce89e8aad01ef39b65bdde09a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/438497
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
When searching for declaration information about a position, it should
suffice to search the narrowest fully type-checked package containing
the file.
This should significantly improve performance when there are many test
variants of the current package, that have not yet been type-checked in
the ParseFull mode (as reported in golang/go#55293).
For golang/go#55293
Change-Id: I89a1999f9fe82dea51dd47db769c90b69be5e454
Reviewed-on: https://go-review.googlesource.com/c/tools/+/438496
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>
In all cases where we call FindPackageFromPos, we know that the given
position must be in the forward transitive closure of an originating
package. Refactor to use this information, potentially saving
significant type-checking when searching for a package.
As a result of this change, we never need to search intermediate test
variants when querying PackagesForFile.
Also replace snapshot arguments with token.FileSet arguments, when the
snapshot is only needed for its FileSet.
For golang/go#55293
Change-Id: Icf6236bea76ab5105a6bab24ce3afc574147882b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/438495
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We recently started using bug.Errorf to report analyzer crashes,
causing tests to fail sporadically. Unfortunately the log included
only the "panicked" message but not the relevant stack of the panic
or of the other goroutines, giving limited evidence on which to
investigate the underlying cause, suspected to be in package loading,
not the analyzer itself.
This change causes such crashes to display all stacks so that we
can gather more evidence over the next few days before we suppress
the crashes again.
Updates golang/go#54762
Updates golang/go#56035
Change-Id: I2d29e05b5910540918816e695b458463a05f94d9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/439116
Reviewed-by: Bryan Mills <bcmills@google.com>
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>
Users can now rename packages by performing a rename on the package name in package declarations. The editor will prompt user with a dialogue text field to change the package name. This rename feature then do the following:
- Rename all the external imports of the renaming package. In case there is renaming conflicts within a file, the feature repeatedly try fresh names constructed by appending new package name with number of try times so far until succeed.
- Rename all the internal references to the renaming package from its files.
- Rename the directory of the renamed package and update the imports' paths of any packages nested under the renamed directory.
- Rename the test package with the new package name and suffix "_test" with the current test package name ends with "_test", or just the new package name otherwise.
The regression tests accounts for these edge cases:
- Only rename other known packages if the module path is same for both the renaming package and the other known packages.
- Prevent renaming main package.
- Test if the renaming package name is different from its path base.
Todo:
- Add a test for the case when the renaming package's path contains "internal" as a segment.
- Allow edit to the whole package path not just only the last segment of the package path
- Reject renaming if the renamed subpackages don't belong to the same module with the renaming package
- Check the go.mod files in the workspace to see if any replace directives need to be fixed if the renaming affects the locations of any go.mod files
Change-Id: I4ccb700df5a142c3fc1c06f3e5835f0f23da1ec5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420958
Run-TryBot: Dylan Le <dungtuanle@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
GNU diff -u prints -0,0 for the special case of inserting
into an empty file. (I don't understand why this case is
special, but it's fair to assume it has been debugged;
perhaps patch(1) has a bug.)
This change matches that special behavior, and adds a test.
It also adds a (disabled) test for another bug I encountered.
Also, remove some overzealous uses of t.Helper() that served
to obscure the failure.
Change-Id: I3e0c389c478cde45163353130e36f2f8741a8659
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436782
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
The code that generates a name for a struct type,
and a default value for a struct field, now
supports the case where either contains a type
parameter.
Also:
- use type-checker's name for the struct type unless
it is a bare struct.
- populateValue: return *new(T) for a type parameter.
- various minor cleanups.
- various TODO comments for clarifications.
- fuzzy.FindBestMatch: use strings not identifiers.
Remove Find prefix (also FindMatchingIdentifiers).
Fixesgolang/go#54836
Change-Id: I4f6132598b4ac7e72ea1405e4a14d6a23c1eeeaa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436777
Auto-Submit: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
To help indicate that the diagnostic is for an error in a dependency,
not in the user's code prefix the diagnostic with
"[module] has a known vulnerability:"
Change-Id: Ib48f2e2cb2620fc6d56c4b0fd5d125cb4789283c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436217
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
+ The JSON output now contains SuggestedFixes from the Diagnostics. The edits
are encoded as replacements for ranges defined by 0 based byte start and end
indicies into the original file.
+ This change also exports the structs that are used for JSON encoding
and thus documents the JSON schema.
Fixesgolang/go#55138
Change-Id: I93411626279a866de2986ff78d93775a86ae2213
Reviewed-on: https://go-review.googlesource.com/c/tools/+/435255
Reviewed-by: Tim King <taking@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Report a bug when an analysis dependency is found to be
neither "vertical" (for facts) or "horizontal" (for results).
This has been observed in practise: the lookup from a package
ID to a package object is not consistent.
The bug report is suppressed for command-line-arguments
packages to see whether it occurs on ordinary packages too.
Also, add disabled logic to disable recovery and dump all
goroutines, for temporary use when debugging.
Unrelated things found during debugging:
- simplify package key used in analysis cache: the mode is
always the same constant.
- move TypeErrorAnalyzers logic closer to where needed.
Updates golang/go#54655
Updates golang/go#54762
Change-Id: I0c2afd9ddd4fcc21748a03f8fa0769a2de09a56f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426018
Reviewed-by: Robert Findley <rfindley@google.com>
If there is a fixed version of the module with a vulnerability,
provide a code action to upgrade to that version.
Change-Id: I2e0d72e7a86dc097f139d60893c204d1ec55dad1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436216
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
It is noisy and not useful.
Change-Id: I9576e921f6dd177dc573dbf1b10c1701eda104d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/436215
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Show the vulnerabilities found by the runvulncheck codelens in the
go.mod file using diagnostics. This uses a similar strategy to
upgrade codelenses to store the vulnerabilities in the view so the
diagnostics can be calculated at a later time.
Change-Id: Ie9744712d9a7f8d78cbe3b54aa4cd3a380a304bc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/433537
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
It looks like this source was added but never used. Use it.
Change-Id: I19d7c49ab1fda11d078a7850a7c0c3a78133ff52
Reviewed-on: https://go-review.googlesource.com/c/tools/+/435361
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Delete some gopls code that is no longer used.
Change-Id: Ib211b01b6c2ac8a03700b1e47d32ac69fccd1b83
Reviewed-on: https://go-review.googlesource.com/c/tools/+/435360
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
The source.Session interface is not needed: the source package should
never need to know about sessions. Remove it in favor of the concrete
*cache.Session type.
Change-Id: I220a1fb1525c57f9fa2a4a4f80152c31a81565ff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/435359
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Deprecate the following features, which never made it out of
experimental:
- experimentalWorkspaceModule (golang/go#52897)
- experimentalWatchedFileDelay (golang/go#55268)
- experimentalUseInvalidMetadata (golang/go#54180)
See the associated issues for rationale behind the deprecations.
With this change, any users configuring these settings will get a
ShowMessageRequest warning them of the deprecation.
Fixesgolang/go#52897Fixesgolang/go#55268Fixesgolang/go#54180
Change-Id: I49f178e68793df4e4f9edb63e9b03cad127c5f51
Reviewed-on: https://go-review.googlesource.com/c/tools/+/434640
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
An application that links in version 1.18 of (say)
go/types cannot process Go source files reported
by version 1.19 of 'go list'. There is no way to
tell go list to behave as if it was go1.18, so
for now we proceed with parsing/typechecking but,
in case of errors, we issue an additional informative
diagnostic.
Fixesgolang/go#55883Fixesgolang/go#55045
Updates golang/go#50825
Updates golang/go#52078
Change-Id: I5fd99b09742c136f4db7f71d2a75e4cdb730460d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/435356
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
Tolerate the new form of the "... imported but not used" error message,
to allow landing this change in go/types.
Along the way, improve the test output when comparing diagnostics, and
formatting results.
For golang/go#54845
Change-Id: I998d539f82e0f70c85bdb4c40858be5e01dd08b6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/435355
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Update gopls to handle the new form of "undeclared name: ..." error
messages, "undefined: ...", and update tests to be tolerant of both.
Also do some minor cleanup of test error messages related to mismatching
diagnostics.
With this change, TryBots should succeed at CL 432556.
Updates golang/go#54845
Change-Id: I9214d00c59110cd34470845b72d3e2f5e73291c1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/434636
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We're almost certain that the go process shown by ps
is not the one that we're waiting for in runCmdContext,
but only by printing the pid can we be sure.
Updates golang/go#54461
Change-Id: I1ce9580de6ee6bc4557d76c2a6b05f5a3e2eb6c2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/434637
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Update the reset_golden.sh script to run the ./test and ./internal/lsp
directories, following the deprecation of ./internal/lsp/cmd and
./internal/lsp/source tests. Also, fix it to generate golden data for
tests that only run at 1.17 and earlier.
Use the newly fixed script to sync the golden data.
For golang/go#54845
Change-Id: I2b42dac91cf1c7e2e8e25fd2aa8ab23c91e05c6c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/434635
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The previous explanation from CL 230312 suggested that multiple
Analyzers be used to distinguish severity levels, which is
unwieldy to implement and sounds more like a workaround than a
principle. I argue there is a principle here: that severity is
best determined by the user, not the analyzer itself.
This does require that drivers support some kind of filtering
or ranking based on Analyzer.Name and Diagnostic.Category.
Some already do; perhaps the unitchecker and multichecker
drivers should too.
Change-Id: I484877baf7963c444285f35c9f03ee3c5e6d7729
Reviewed-on: https://go-review.googlesource.com/c/tools/+/433536
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
After experimenting with the new t.Run+t.Parallel check in the
loopclosure analyzer, we discovered that users rely on the fact that
statements before the call to t.Parallel are executed synchronously, for
example by declaring test := test inside the function literal, but
before the call to t.Parallel.
To avoid such false positives, only consider statements occurring after
the first call to t.Parallel.
Change-Id: I88466ea3bfd318d42d734c320677fbe5e3f6cb00
Reviewed-on: https://go-review.googlesource.com/c/tools/+/433535
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
The marker function collectCompletionItems required at least three
arguments. Express this in its signature, leaving a final variadic
argument for documentation. I considered just making this final argument
mandatory, but opted for minimizing the diff given that there are 400+
existing @item annotations.
With this change the only use of tests.Data.t is in mustRange. Since
conversion to range should always succeed, I switched this usage to a
panic and removed the t field.
For golang/go#54845
Change-Id: I407f07cb85fa1356ceb6dba366007f69d1b6a068
Reviewed-on: https://go-review.googlesource.com/c/tools/+/432337
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In order to narrow usage of tests.Data.t, use the mustRange helper in
more places.
For golang/go#54845
Change-Id: I446ca520fa76afb2bc10c1fd5a5765859176dd6a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/432336
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>
- simplify collection to use an expected span.Span and use mustRange
- remove the source_test.go version of the test, as it is redundant
For golang/go#54845
Change-Id: I3a7da8547e27dc157fb513486a151031ec135746
Reviewed-on: https://go-review.googlesource.com/c/tools/+/432138
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>
The documentSymbols handler joined syntax information with type
information, meaning that it was only able to satisfy requests for files
in valid workspace packages. However, the value added by type
information was limited, and in many cases could be derived from syntax
alone. For example, while generating symbols for a const declaration, we
don't need the type checker to tell us that the symbol kind is const.
Refactor the documentSymbols handler to derive symbols from syntax
alone. This leads to some simplifications from the code, in addition to
eliminating the dependency on package data. Also, simplify symbol
details to just use types.ExprString, which includes some missing
information such as function return values. Also, update handling to
support Go 1.18 type embedding in interfaces.
Notably, this reverts decisions like golang/go#31202, in which we went to
effort to make the symbol kind more accurate. In my opinion (and the
opinion expressed in golang/go#52797), the cost of requiring type
information is not worth the minor improvement in accuracy of the symbol
kind, which (as far as I know) is only used for UI elements.
To facilitate testing (and start to clean up the test framework), make
several simplifications / improvements to the marker tests:
- simplify the collection of symbol data
- eliminate unused marks
- just use cmp.Diff for comparing results
- allow for arbitrary nesting of symbols.
- remove unnecessary @symbol annotations from workspace_symbol tests --
their data is not used by workspace_symbol handlers
- remove Symbol and WorkspaceSymbol handlers from source_test.go. On
inspection, these handlers were redundant with lsp_test.go.
Notably, the collection and assembly of @symbol annotations is still way
too complicated. It would be much simpler to just have a single golden
file summarizing the entire output, rather than weaving it together from
annotations. However, I realized this too late, and so it will have to
wait for a separate CL.
Fixesgolang/go#52797Fixesgolang/vscode-go#2242
Updates golang/go#54845
Change-Id: I3a2c2d39f59f9d045a6cedf8023ff0c80a69d974
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405254
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Alan Donovan
Hana (Hyang-Ah) Kim
Tim King
cui fliter
Suzy Mueller
Robert Findley
Dylan Le
Lasse Folger
Zvonimir Pavlinovic
Damien Neil