Consider a mechanism to permit automatically binding a type to interfaces it implements

bradleypeabody commented 4 years ago

Is your feature request related to a problem? Please describe.

If I am wiring a complex application which uses interfaces to describe dependencies, it is cumbersome to have to provide wire.Bind to describe mappings from interface to implementation even when the method names are clearly designed to be unambiguous.

If I have a LoginController which needs a TryLogin(u, p string) error method, there is only going to be one type in the system which implements such a call and it will be available via some function in wire.Build(...). I don't see why we need a wire.Bind to indicate what implements that TryLogin method when there is only one.

Describe the solution you'd like

For interfaces and implementations that are unambiguous (there is only one concrete type from the provider list that matches the interface being resolved), a wire.Bind should not be needed. I understand the use of wire.Bind to clarify ambiguous matches but requiring it in all cases adds tedium for no benefit.

Describe alternatives you've considered

I don't see any alternative other than writing out wire.Bind for each mapping, which is the tedium I'm trying to avoid.

Additional context

The Go language itself resolves interfaces based solely on method signatures, not an explicit implements mechanism - I don't see why wire wouldn't follow the same behavior. wire.Bind can still be used to resolve ambiguous cases.

Example

Basically, I feel like this example should just work:

type LoginStore interface {
    TryLogin(u, p string) error
}

type LoginCtrl struct {
    LoginStore LoginStore
}

func NewLoginCtrl(store LoginStore) *LoginCtrl { return &LoginCtrl{LoginStore: store} }

type UserDataStore struct{}

// TryLogin makes UserDataStore implement LoginStore
func (uds *UserDataStore) TryLogin(u, p string) error { panic("TODO") }

func NewUserDataStore() *UserDataStore { return &UserDataStore{} }

// ... {
    wire.Build(NewUserDataStore, NewLoginCtrl)
// }

If there is agreement on the feature I can dig into the code and start seeing what it would take to put together a PR.

zombiezen commented 4 years ago

Thanks for the report, and I'm sorry to hear it's been a source of friction for you.

I do think the explicitness adds benefit, but we may not have communicated the benefit well (might warrant an FAQ). The reason the binding is explicit is to avoid scenarios where adding a new type to the provider graph that implements the same interface causes the graph to break, because that can be surprising. While this does result in more typing, the end-effect is that the developer's intent is more explicit in the code, which we felt was most consistent with the Go philosophy.

I'm closing this issue out because we won't accept PRs for your proposal as written, but if you have other ideas on how to reduce tedium, please feel free to open a new issue or ask questions here.

bradleypeabody commented 4 years ago

Thanks @zombiezen, I get where you are coming from. It is certainly a trade-off and I certainly get that adding items to a wire set causing other earlier items to fail could be unexpected. The only thing I would point out is that Go does something very similar regarding method name matching when embedding structs: You can call T.B() and B can be on an embedded struct as long as it's unambiguous; adding another embed to T with another B method will cause previously working T.B() references to fail. I do understand the scope is different as we're talking about structs and code that references these directly whereas in the topic of this issue interfaces will tend to connect more disparate parts of code.

But fair enough, if this has already been thought through and decided then I understand.

I have an alternate proposal, which is to allow this feature optionally through something like: wire.AutoBind(new(LoginStore)). In this case it flags LoginStore as a candidate for automatically satisfying interfaces. This would not change the behavior of any existing code but would allow people who wanted automatic interface matching to use it. It puts the decision in the hands of the wirer.

What do you think? Is that a possibility?

zombiezen commented 4 years ago

I think that would work, but would likely need to take lower precedence than explicit bindings. My instinct here is that there may be some complexity for larger provider graphs, but the explicit AutoBind marker would make it easier for static analysis to detect the behavior is occurring.

In the interest of trying to set realistic expectations and unblocking you, you should know that I and the other Wire maintainers are constrained on bandwidth right now due to the COVID-19 situation. I encourage you to prototype and experiment with this idea on a fork, but I won't have time to review over the next few weeks. Hearing your experience with the experiment in a realish-world scenario would be helpful in weighing whether or not we upstream this. Does that work for you?

bradleypeabody commented 4 years ago

Okay great.

Agreed on the lower precedence - explicit bindings should always be honored.
Understood on the possible difficulties. I'm willing to give it a shot.
Completely understood on the timeline and no worries. I'm in no rush at all, I just mainly wanted to ensure that if I do work on this that there's a reasonable possibility it would get merged. I think we're good on that for now. I'll fork and get together a prototype of this feature when I can and let you know.

bradleypeabody commented 4 years ago

Would you be okay with re-opening this issue? Or should I just make a PR when I have one and we can do any further discussion around the actual code?

zombiezen commented 4 years ago

Retitled to match new scope and reopened.

ProximaB commented 4 years ago

@bradleypeabody How it's going? 😄

bradleypeabody commented 4 years ago

Unfortunately I have not yet gotten to this. I was hoping to run into a specific case where I need it for Vugu (https://github.com/vugu/vugu but I haven't run into it again, yet). This is definitely still on my radar and I hope to look into it further soon.

dabbertorres commented 3 years ago

@bradleypeabody have you gotten to this by chance? If not, would you mind if I shared my attempt at adding this feature?

bradleypeabody commented 3 years ago

@dabbertorres I have not had a chance to work on this. Definitely no objection to someone else taking it on. I'll try to provide feedback/suggestions to whatever extent possible/needed.

google / wire