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

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

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Aug 27, 2020

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_.

@pjweinb
Copy link

@pjweinb pjweinb commented Sep 6, 2020

Is this a duplicate of #38866?

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Sep 7, 2020

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

@jadekler
Copy link
Member

@jadekler jadekler commented Sep 8, 2020

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 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

@gopherbot gopherbot commented Oct 13, 2020

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

@jadekler
Copy link
Member

@jadekler jadekler commented Oct 13, 2020

@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
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

@gopherbot gopherbot commented Dec 1, 2020

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
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

@stamblerre stamblerre commented Dec 1, 2020

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

@stamblerre stamblerre closed this Dec 1, 2020
vscode-go: gopls by default automation moved this from Non-critical to Done Dec 1, 2020
marwan-at-work added a commit to marwan-at-work/tools that referenced this issue Dec 23, 2020
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants