Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/tools/gopls: bad error messages when changing package names #41061

Closed
stamblerre opened this issue Aug 27, 2020 · 8 comments
Closed

x/tools/gopls: bad error messages when changing package names #41061

stamblerre opened this issue Aug 27, 2020 · 8 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
vscode-go: gopls by default
Milestone
gopls/v0.6.0

Comments

@stamblerre
Copy link
Contributor

Repro case:

-- go.mod --
module mod.com
-- foo/foo.go --
package foo
-- foo/bar_test.go --
package foo_

Modifying foo/bar_test.go to contain package foo_test results in bad error messages (it doesn't recover from expecting that its name is foo_.
@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Aug 27, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Aug 27, 2020
@pjweinb
Copy link

pjweinb commented Sep 6, 2020

Is this a duplicate of #38866?

@stamblerre
Copy link
Contributor Author

Possibly, but I think there are also issues with transitioning from a regular package to an x test.

@jeanbza
Copy link
Member

jeanbza commented Sep 8, 2020 •
edited

I've got a repro based on the above testcase, and will start taking a peek soon. Is anyone else working on this, though? Don't want to step on toes. =)

(if I hear nothing back I'll continue as if the answer to that is a "no")

@pjweinb
Copy link

pjweinb commented Sep 9, 2020

Go right ahead. i was going to look at #38866 but there's other things to do.

@gopherbot
Copy link

Change https://golang.org/cl/262019 mentions this issue: x/tools/gopls: add a skipped test for renaming of package

@jeanbza
Copy link
Member

jeanbza commented Oct 13, 2020 •
edited

@pjweinb After a couple fridays afternoons of 20%ing, I unfortunately didn't get enough traction. This one appears to require a lot of institutional knowledge. 🙂 📚 Per @stamblerre suggestion I've parked the test that I wrote at https://go-review.googlesource.com/c/tools/+/262019, which I believe exercises this, but I'll take the pragmatic approach of dropping off this one and trying out an easier feature.

Sincere apologies if I wasted anyone's time trying to tackle this!!

gopherbot pushed a commit to golang/tools that referenced this issue Oct 16, 2020
@jeanbza
x/tools/gopls: add a skipped test for renaming of package
7c89c52 
This adds a test that asserts that gopls gracefully recovers from a bad package
name. It does not include the fix for the test, and so this test is skipped: I
took a stab at it but it was a deep spelunk that I wasn't able to make enough
progress on.

Updates golang/go#41061

Change-Id: I86d817396dae5b0211e633721867e811c7d6cf40
Reviewed-on: https://go-review.googlesource.com/c/tools/+/262019
Run-TryBot: Jean de Klerk <deklerk@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Jean de Klerk <deklerk@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@stamblerre stamblerre added this to Non-critical in vscode-go: gopls by default Nov 10, 2020
@stamblerre stamblerre moved this from Non-critical to Needs Triage in vscode-go: gopls by default Nov 11, 2020
@stamblerre stamblerre moved this from Needs Triage to In progress in vscode-go: gopls by default Nov 11, 2020
@stamblerre stamblerre self-assigned this Nov 11, 2020
@stamblerre stamblerre moved this from In progress to Non-critical in vscode-go: gopls by default Nov 11, 2020
@stamblerre stamblerre removed their assignment Nov 16, 2020
@gopherbot
Copy link

Change https://golang.org/cl/274533 mentions this issue: gopls/internal/regtest: TestRenamePackage passes with go 1.16

gopherbot pushed a commit to golang/tools that referenced this issue Dec 1, 2020
@pjweinbgo
gopls/internal/regtest: TestRenamePackage passes with go 1.16
92771a2 
Instead of skipping the test, insist on 1.16 or later.

See golang/go#41061

Change-Id: Ie13f114039693993d65333ab9194629187a9b6b1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/274533
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@stamblerre
Copy link
Contributor Author

Let's close this now that the test has been enabled.

vscode-go: gopls by default automation moved this from Non-critical to Done Dec 1, 2020
marwan-at-work pushed a commit to marwan-at-work/tools that referenced this issue Dec 23, 2020
@pjweinbgo @marwan-at-work
gopls/internal/regtest: TestRenamePackage passes with go 1.16
2879509 
Instead of skipping the test, insist on 1.16 or later.

See golang/go#41061

Change-Id: Ie13f114039693993d65333ab9194629187a9b6b1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/274533
Run-TryBot: Peter Weinberger <pjw@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@golang golang locked and limited conversation to collaborators Dec 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
No open projects
vscode-go: gopls by default
  
Done
Milestone
gopls/v0.6.0
Development

No branches or pull requests
4 participants
@jeanbza @pjweinb @stamblerre @gopherbot