go icon indicating copy to clipboard operation
go copied to clipboard

x/tools/gopls: code action "Extract interface for type"

Open martskins opened this issue 1 year ago • 4 comments

gopls version

Irrelevant

go env

Irrelevant

What did you do?

Given the following code:

type Server struct {
    api *api.Client
}

func (s *Server) CreateUser(ctx context.Context, userID string) {
    err := s.api.CreateUser(ctx, userID)
    // ...
}

func (s *Server) DeleteUser(ctx context.Context, userID string) {
    err := s.api.DeleteUser(ctx, userID)
    // ...
}

What did you see happen?

It would be nice if gopls offered an action to extract an interface out of the used methods on the api field. So for the example given above, it would offer an edit to do something like this:

type Client interface {
    CreateUser(context.Context, string) error
    DeleteUser(context.Context, string) error
}

type Server struct {
    api Client
}

Where the method set of the generated interface is the methods that are called on the field that the action was initiated on, and if there are uses that are not method calls on that field then it would either error or just not offer the action.

What did you expect to see?

Nothing

Editor and settings

No response

Logs

No response

martskins avatar Feb 15 '24 14:02 martskins

This looks very similar to #46665, though that is more about extracting the interface of an argument, not an entire type. I think both may be useful, so they are distinct issues.

findleyr avatar Feb 15 '24 15:02 findleyr

Oh sorry! missed that one! It does look very similar, and I don't really see the difference to be honest! In any case, I'll probably take a stab at this one at some point in the next few days, so I'll send a CL your way once I have something that kinda works!

martskins avatar Feb 15 '24 15:02 martskins

If the interface is to be based on the set of methods actually used, as opposed to all those defined, then this requires a global query on the field, analogous to a "references" query on a field, which is quite a complex operation and one not factored as a building block for other tasks. Embedding further complicates matters, because it's not enough to find all references to f in type S struct { f T }, you may have to find all places that embed an S as well.

In short, I don't think this is an easy project.

adonovan avatar Feb 15 '24 18:02 adonovan

@adonovan I didn't see your comment until now when I came back to fetch the link for the issue to add it to the PR description :sweat_smile:

I opened a draft to try and get some feedback on it. I essentially re-used the references query you mentioned (actually the one that wraps that one). I didn't consider embedded fields to be honest, and it doesn't offer the action in that case now (not a conscious decision I took though). I'll take a look at embedded fields later though!

I'm sure re-using references like I did is far from ideal, so if you think that PR is very far from what you think is an acceptable implementation I'm happy to just close it and let someone with more experience take care of that one!

martskins avatar Feb 15 '24 23:02 martskins