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.

Sign up for GitHub

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: references regression when tests involved #59851

Closed
danp opened this issue Apr 26, 2023 · 5 comments
Closed

x/tools/gopls: references regression when tests involved #59851

danp opened this issue Apr 26, 2023 · 5 comments
Labels
FrozenDueToAge gopls/incremental gopls Issues related to the Go language server, gopls. release-blocker Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.12.0

Comments

@danp
Copy link
Contributor

danp commented Apr 26, 2023 •
edited

Since #58506 was closed I'm having trouble with references for interface methods with implementations in tests. This test setup, when placed alongside the test added for #58506 seems to show it:

-- go.mod --
module example.com
go 1.12

-- a/a.go --
package a

type Aer interface {
	TheMethod()
}

-- a/a_test.go --
package a

type fakeAer struct {}

// marker.go:429: a/a_test.go:8:33 (references/issue58506_2.txt:20:33): refs(includeDeclaration=false) failed: package example.com/a does not contain "fakeAer"
// marker.go:429: a/a_test.go:8:33 (references/issue58506_2.txt:20:33): refs(includeDeclaration=true) failed: package example.com/a does not contain "fakeAer"

func (fakeAer) TheMethod() {} //@refs("TheMethod")

I verified with git bisect that this started as part of the change that closed #58506.
@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 Apr 26, 2023
@gopherbot gopherbot added this to the Unreleased milestone Apr 26, 2023
@danp
Copy link
Contributor Author

danp commented Apr 26, 2023

I started out calling the method A which does produce references, though might be some fuzz at work.

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.12.0 Apr 26, 2023
@findleyr
Copy link
Contributor

Thanks for reporting!

CC @adonovan

@adonovan
Copy link
Member

FWIW the reason the method name A didn't work is that it matches the letter A within fakeAer. You would need something like this to match only an A following a space:

func (fakeAer) A() {} //@loc(def, re" (A)"), refs(def, def)

I confirm the bug (two bugs actually). The first is that the references operation fails because it can't find object fakeAer in the ordinary (non-test-variant) package a. The second bug is that, if the first error is ignored, the algorithm returns too few references.

There are a number of problems in the references algorithm that I should probably fix together.

Thanks again for the very helpful bug report.

@gopherbot
Copy link

Change https://go.dev/cl/491015 mentions this issue: gopls/internal/lsp/source: fix another bug in reference expansion

@danp
Copy link
Contributor Author

danp commented May 2, 2023

Tried out the CL above on the project that prompted this issue and it fixes things for me, thanks!

@adonovan adonovan mentioned this issue May 9, 2023
Closed
@golang golang locked and limited conversation to collaborators May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls/incremental gopls Issues related to the Go language server, gopls. release-blocker Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.12.0
Development

No branches or pull requests
4 participants
@danp @adonovan @gopherbot @findleyr