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

go/types,x/tools/go/packages: types.Implements() fails for interfaces with named types #61412

Closed
momilo opened this issue Jul 18, 2023 · 4 comments
Closed

go/types,x/tools/go/packages: types.Implements() fails for interfaces with named types #61412

momilo opened this issue Jul 18, 2023 · 4 comments
Labels
Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Backlog

Comments

@momilo
Copy link

momilo commented Jul 18, 2023 •
edited

Problem summary

It seems like the function types.Implements will fail if the Interface provided contains methods returning custom types (even if everything is defined in the exact same package).

Sample minimal code can be found here:

Details

Specifically, given the below which compiles fine (note the assertion that the struct "something" implements both interfaces):

type MyInterface interface {
	ID() ID
	Description() string
}

type MySimplerInterface interface {
	Description() string
}

type something struct{}

var _ MyInterface = &something{}
var _ MySimplerInterface = &something{}
...

the check types.Implements(types.NewPointer(typesObj.Type()), interfaceSearched) will return true only for MySimplerInterface, where the types details have been loaded using go/packages

What version of Go are you using (go version)?

go1.20.5

What operating system and processor architecture are you using (go env)?

darwin / arm64
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jul 18, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jul 18, 2023
@bcmills bcmills changed the title x/tools/go/packages & go/types: types.Implements() fails for interfaces with custom types go/types,x/tools/go/packages: types.Implements() fails for interfaces with named types Jul 18, 2023
@bcmills
Copy link
Contributor

bcmills commented Jul 18, 2023

(attn @griesemer @findleyr)

@bcmills bcmills modified the milestones: Unreleased, Backlog Jul 18, 2023
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 18, 2023
@griesemer griesemer mentioned this issue Jul 18, 2023
Open
@griesemer griesemer removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 18, 2023
@griesemer
Copy link
Contributor

This is working as expected.

Note that types.Implements is the same function that the compiler is using (it's an auto-generated copy of it), so it's unlikely that types.Implements fails when using it as a library function when the compiler accepts the equivalent source.

Here is your example, a bit further simplified so I could see what's going on (in general when sending a repro case, please minimize by removing all unnecessary code - it greatly helps us diagnosing problems).

The problem is that you are loading the same packages twice (interfacePks and searchedPks). User-defined types, in this case, the type ID gets a different "identity" in different loads. As a consequence, the type ID used in the interface method doesn't match the type ID in the struct method and then Implements fails. The problem goes away if you use say string in case of ID (not a user-defined type), or if you correct the code and load the packages only once. You can see in the simplified example (line 50, 51) that loading the packages only once solves the issue.

The type-identity properties should be better documented. I filed #61418.

Closing as working as intended.

@griesemer
Copy link
Contributor

PS: Here is your code with the respective correction: playground.

@momilo
Copy link
Author

momilo commented Jul 19, 2023 •
edited

Thank you for the quick reply and help - that is much appreciated!

That is not ideal (e.g. the interface could be defined in a completely different place than the code I am trying to analyse against it). I wonder if this could be resolved by having the "identity" defined in a more deterministic way. I do, however, appreciate that that's the imitation of the current toolset (which I did not appreciate earlier), so nothing more to do here :-).

Thank you once again for your help!

@fbbdev fbbdev mentioned this issue Mar 30, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@bcmills @griesemer @gopherbot @momilo