Dagger2 covariant Set multibinding

[Raman Gupta]

Dagger2 is unable to inject a `Set<T>` into a covariant `Set<? extends T>` field. Is there any particular reason for this, or could this be considered a bug?

As context: I found this while prototyping Dagger2 with Kotlin, in which `Set<T>` is read-only and can therefore accept covariant types. One has to fall-back to declaring the injection site as a `MutableSet<T>` to get the invariant type, even if the `Set` is intended to be read-only.

Regards,

Raman

[David P. Baker]

Can you give a more explicit example? Do you have a field like @Inject Set<? extends Foo> set, and a binding for Set<FooImpl>? Dagger in fact considers those to be different keys. You are free to bind one to the other, for instance by using a @Binds method:

/** Satisfies requests for Set<? extends Foo> with Set<FooImpl>. */

@Binds abstract Set<? extends Foo> bind(Set<FooImpl> set);

--
You received this message because you are subscribed to the Google Groups "Dagger Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to dagger-discus...@googlegroups.com.
To post to this group, send email to dagger-...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Raman Gupta]

On Tuesday, October 4, 2016 at 10:13:47 AM UTC-4, David P. Baker wrote:

Can you give a more explicit example? Do you have a field like @Inject Set<? extends Foo> set, and a binding for Set<FooImpl>?

Yes, though I have multiple implementations of `Foo`, and in this case they are actually anonymous classes (lambdas).

 

Dagger in fact considers those to be different keys. You are free to bind one to the other, for instance by using a @Binds method:

 

/** Satisfies requests for Set<? extends Foo> with Set<FooImpl>. */

@Binds abstract Set<? extends Foo> bind(Set<FooImpl> set);

Why would this be the case?

In any case, this only works for one type of `Foo`, so its not a sufficient solution:

/home/raman/tmp/experiment-dagger2/src/main/java/experiment/dagger2/JavaDagger.java:40: error: java.util.Set<? extends experiment.dagger2.SetInstance> is bound multiple times:

    TestJ test();

          ^

      @Binds Set<? extends experiment.dagger2.SetInstance> experiment.dagger2.TestModuleBinds.bindOne(Set<experiment.dagger2.SetInstanceOne>)

      @Binds Set<? extends experiment.dagger2.SetInstance> experiment.dagger2.TestModuleBinds.bindTwo(Set<experiment.dagger2.SetInstanceTwo>)

I can make it work simply by declaring the injection site as (no @Binds required):

@Inject Set<Foo> set

but as I said in my initial post, the type constraints on this set are not as tights as they could be -- at the injection site, given I only want (and should) read from this Set, `Set<? extends Foo>` is a more appropriate type signature than `Set<Foo>`.

Regards,

Raman

[Thomas Broyer]

On Friday, October 14, 2016 at 5:24:03 AM UTC+2, Raman Gupta wrote:

/home/raman/tmp/experiment-dagger2/src/main/java/experiment/dagger2/JavaDagger.java:40: error: java.util.Set<? extends experiment.dagger2.SetInstance> is bound multiple times:

    TestJ test();

          ^

      @Binds Set<? extends experiment.dagger2.SetInstance> experiment.dagger2.TestModuleBinds.bindOne(Set<experiment.dagger2.SetInstanceOne>)

      @Binds Set<? extends experiment.dagger2.SetInstance> experiment.dagger2.TestModuleBinds.bindTwo(Set<experiment.dagger2.SetInstanceTwo>)

Use @Binds @ElementsIntoSet to "merge" both sets?

[Raman Gupta]

No luck:

/home/raman/tmp/tmp/experiment-dagger2/src/main/java/experiment/dagger2/JavaDagger.java:32: error: @Binds methods must return a primitive, an array, a type variable, or a declared type.

  @Binds @ElementsIntoSet abstract Set<? extends SetInstance> bindOne(Set<SetInstanceOne> set);

                                                              ^

/home/raman/tmp/tmp/experiment-dagger2/src/main/java/experiment/dagger2/JavaDagger.java:33: error: @Binds methods must return a primitive, an array, a type variable, or a declared type.

  @Binds @ElementsIntoSet abstract Set<? extends SetInstance> bindTwo(Set<SetInstanceTwo> set);

                                                              ^

I'm not sure this is the right approach anyway -- how would I use @Binds when my `SetInstance` is a lambda or anonymous class?

Regards,

Raman

[David P. Baker]

On Thu, Oct 13, 2016 at 11:24 PM Raman Gupta <rocke...@gmail.com> wrote:

On Tuesday, October 4, 2016 at 10:13:47 AM UTC-4, David P. Baker wrote:

Can you give a more explicit example? Do you have a field like @Inject Set<? extends Foo> set, and a binding for Set<FooImpl>?

Yes, though I have multiple implementations of `Foo`, and in this case they are actually anonymous classes (lambdas).

OK, so that's not what you're trying to do. Sorry.

 

 

Dagger in fact considers those to be different keys. You are free to bind one to the other, for instance by using a @Binds method:

 

/** Satisfies requests for Set<? extends Foo> with Set<FooImpl>. */

@Binds abstract Set<? extends Foo> bind(Set<FooImpl> set);

Why would this be the case?

In any case, this only works for one type of `Foo`, so its not a sufficient solution:

Right; my answer was for a question that it turns out you didn't have. :)

 

/home/raman/tmp/experiment-dagger2/src/main/java/experiment/dagger2/JavaDagger.java:40: error: java.util.Set<? extends experiment.dagger2.SetInstance> is bound multiple times:

    TestJ test();

          ^

      @Binds Set<? extends experiment.dagger2.SetInstance> experiment.dagger2.TestModuleBinds.bindOne(Set<experiment.dagger2.SetInstanceOne>)

      @Binds Set<? extends experiment.dagger2.SetInstance> experiment.dagger2.TestModuleBinds.bindTwo(Set<experiment.dagger2.SetInstanceTwo>)

I can make it work simply by declaring the injection site as (no @Binds required):

@Inject Set<Foo> set

but as I said in my initial post, the type constraints on this set are not as tights as they could be -- at the injection site, given I only want (and should) read from this Set, `Set<? extends Foo>` is a more appropriate type signature than `Set<Foo>`.

So you're saying that what Dagger currently does for multibindings works for you, but you want to know why it doesn't instead do use a wildcard element type for multibound sets?

I'm not sure whether doing that is worth requiring all injection points to add the "? extends" boilerplate. The injected Set is unmodifiable. Dagger doesn't assume Guava is on the classpath, so we don't provide ImmutableSet.

[Raman Gupta]

On Monday, October 17, 2016 at 11:34:33 AM UTC-4, David P. Baker wrote:

On Thu, Oct 13, 2016 at 11:24 PM Raman Gupta <rocke...@gmail.com> wrote:

I can make it work simply by declaring the injection site as (no @Binds required):

@Inject Set<Foo> set

but as I said in my initial post, the type constraints on this set are not as tights as they could be -- at the injection site, given I only want (and should) read from this Set, `Set<? extends Foo>` is a more appropriate type signature than `Set<Foo>`.

So you're saying that what Dagger currently does for multibindings works for you, but you want to know why it doesn't instead do use a wildcard element type for multibound sets?

 Right.

I'm not sure whether doing that is worth requiring all injection points to add the "? extends" boilerplate.

Does it really need the extra boilerplate? Since `Set<? extends T>` is assignable from `Set<T>`, I think Dagger's validation routines just have to avoid throwing an error when the inject site declares `Set<? extends T>` rather than `Set<T>`. Otherwise the injection should work exactly the same.

 

The injected Set is unmodifiable. Dagger doesn't assume Guava is on the classpath, so we don't provide ImmutableSet.

I do see that the injected set is unmodifiable, which is nice. But when my injection site code declares `Set<? extends T>`, I get compile-time errors when I try to modify the set rather than runtime errors, which is even better.

[David P. Baker]

I think Set<? extends Foo> means something specific (a set of Foo or some unknown subtype of Foo), and not simply an unmodifiable set of Foo objects. It is a different type than Set<Foo>, and I think Dagger is correct to treat those two as different types available for binding.

1. @Provides Set<? extends Foo>
   
2. @Provides Set<Foo>
   
3. @Provides @IntoSet Foo
   

Right now, all three work, but #2 and #3 will conflict because they both bind Set<Foo>. I think what you're asking for is for #1 to conflict with #2 and #3 as well. If the only value is to make it a compile-time error to modify an injected set, I don't think that value is worth making providers for two distinct types conflict.

[Raman Gupta]

I believe having `@IntoSet` binding to `Set<? extends Foo>` would be better.

1) Slightly more flexible multibinding -- allowing bind `Foo` @IntoSet, or some inheritor of `Foo` @IntoSet.

2) The bound set is compile-time read-only which aligns nicely with the @IntoSet definition of immutability.

3) It removes the conflict between #2 (@Provides Set<Foo>) and #3 (@Provides @IntoSet Foo), which adds another capability: the ability to intentionally @Provide a mutable Set<Foo> at the same time as using multibindings.

4) It makes the "normal" case easier (@Provides Set<Foo>, no need to worry about conflicts with multibindings), and the "advanced" case slightly harder to grok but more flexible (multibinding Set<? extends Foo>).

Regards,

Raman

[David P. Baker]

Raman, I really do appreciate your ideas here. Let me respond to each:

1. Doesn't that mean you'll never be able to bind @IntoSet Foo and @IntoSet Bar (which extends Foo) and get two different sets? (Set<Foo> and Set<Bar>)
2. As I said before, I think the unavailability of add() is a mere side effect of the wildcard in Set<? extends Foo>, not its intended meaning.
3. That conflict is by design.
4. Multibinding isn't intended to be an advanced feature. It's a basic and very useful feature of Dagger. I wouldn't want to make that any harder for people to use.

I understand what you're asking for, but I think the current multibinding feature is what we want.

[Raman Gupta]

On Wednesday, October 19, 2016 at 10:21:18 AM UTC-4, David P. Baker wrote:

Raman, I really do appreciate your ideas here. Let me respond to each:

Thank you for saying this. Sometimes in these discussions it feels like one is just being annoying for no good reason :-) I'm glad you don't think so. 

1. Doesn't that mean you'll never be able to bind @IntoSet Foo and @IntoSet Bar (which extends Foo) and get two different sets? (Set<Foo> and Set<Bar>)

Without additional code to distinguish them, yes, that would be true. Wouldn't it be unusual to the point of likely being an error for a multibinding consumer to expect Set<Foo> and Set<Bar> to be bound separately? It seems to me in this case, the user almost certainly intended to bind @IntoSet Bar as Foo, unless *all* of their @IntoSet bind Bar. However, some extra code could support assignment to Set<? extends Foo> and Set<? extends Bar> to support this case as well.

 

1. As I said before, I think the unavailability of add() is a mere side effect of the wildcard in Set<? extends Foo>, not its intended meaning.

I wouldn't say its a "mere" side effect. One could say its a "side effect", but its a side-effect that is *fundamental* to Set being covariant on `Foo`: the consumer of the Set does not know what the type of the objects in the Set are, so it simply cannot add to it.

 

1. That conflict is by design.

That design is what we're discussing here, isn't it? :-)

 

1. Multibinding isn't intended to be an advanced feature. It's a basic and very useful feature of Dagger. I wouldn't want to make that any harder for people to use.

I understand what you're asking for, but I think the current multibinding feature is what we want.

Ok, fair enough. I can see both sides of the issue and certainly understand that the benefits (if any) of what I'm proposing aren't worth the effort required. Thanks for the discussion though!

Regards,

Raman

[David P. Baker]

On Wed, Oct 19, 2016 at 11:43 AM Raman Gupta <rocke...@gmail.com> wrote:

On Wednesday, October 19, 2016 at 10:21:18 AM UTC-4, David P. Baker wrote:

Raman, I really do appreciate your ideas here. Let me respond to each:

Thank you for saying this. Sometimes in these discussions it feels like one is just being annoying for no good reason :-) I'm glad you don't think so. 

I think email tends to make discussions sound harsher than anyone intends them to.

 

1. Doesn't that mean you'll never be able to bind @IntoSet Foo and @IntoSet Bar (which extends Foo) and get two different sets? (Set<Foo> and Set<Bar>)

Without additional code to distinguish them, yes, that would be true. Wouldn't it be unusual to the point of likely being an error for a multibinding consumer to expect Set<Foo> and Set<Bar> to be bound separately?

Not in the least! Otherwise, won't everything just be Set<? extends Object>?

 

It seems to me in this case, the user almost certainly intended to bind @IntoSet Bar as Foo, unless *all* of their @IntoSet bind Bar.

The return type of an @IntoSet is used exactly to specify which set to bind into. After all, I could have the same @Provides @IntoSet method return Bar, Foo, or Object. That's how I choose.

 

However, some extra code could support assignment to Set<? extends Foo> and Set<? extends Bar> to support this case as well.

 

1. As I said before, I think the unavailability of add() is a mere side effect of the wildcard in Set<? extends Foo>, not its intended meaning.

I wouldn't say its a "mere" side effect. One could say its a "side effect", but its a side-effect that is *fundamental* to Set being covariant on `Foo`: the consumer of the Set does not know what the type of the objects in the Set are, so it simply cannot add to it.

A hypothetical Set method could be addIfValid(Object), which would accept any object and add it only if it is a subtype of the accepted type of the Set. (That would require runtime type checking under the hood, of course.) That might not be a great idea, but it wouldn't fundamentally alter the notion of a typed Set. However, it would mean that Set<? extends Foo> is no longer necessarily unmodifiable. That's why I don't think the unmodifiability is a fundamental aspect of Set.

 

 

1. That conflict is by design.

That design is what we're discussing here, isn't it? :-)

Well, you started this thread by asking whether Dagger's considering Set<Foo> and Set<? extends Foo> different keys was a bug. I'm just saying it's not a bug—it's a feature. :)

 

 

1. Multibinding isn't intended to be an advanced feature. It's a basic and very useful feature of Dagger. I wouldn't want to make that any harder for people to use.

I understand what you're asking for, but I think the current multibinding feature is what we want.

Ok, fair enough. I can see both sides of the issue and certainly understand that the benefits (if any) of what I'm proposing aren't worth the effort required. Thanks for the discussion though!

No worries; it's been helpful!

For instance, one thing +Gregory Kick and I discussed as a result of this conversation was that the practice of injecting Provider<Foo> is not really technically correct (the best kind). You really want to inject a Provider<? extends Foo>. If everyone did that, the generated code might not have to suppress warnings when for instance it gives you a Provider<Bar> because of a @Binds. But unfortunately, Java's type system doesn't know that Provider<Bar> is a valid subtype of Provider<Foo>; hence the technical need to inject Provider<? extends Foo>.

We decided that making everyone inject Provider<? extends Foo> was too onerous, since its only practical benefit would be to allow generated code not to suppress some warnings. However, we might allow people to inject Provider<? extends Foo> if they want to—which currently won't work. (We don't actually think anyone wants this enough for us to prioritize implementing it at the moment.)

[Raman Gupta]

Ok I had intended my last message to be my last word on the subject... but as usual I can't resist poking the bear a bit more :-)

On Wednesday, October 19, 2016 at 12:52:08 PM UTC-4, David P. Baker wrote:

On Wed, Oct 19, 2016 at 11:43 AM Raman Gupta <rocke...@gmail.com> wrote:

On Wednesday, October 19, 2016 at 10:21:18 AM UTC-4, David P. Baker wrote:

Raman, I really do appreciate your ideas here. Let me respond to each:

1. Doesn't that mean you'll never be able to bind @IntoSet Foo and @IntoSet Bar (which extends Foo) and get two different sets? (Set<Foo> and Set<Bar>)

Without additional code to distinguish them, yes, that would be true. Wouldn't it be unusual to the point of likely being an error for a multibinding consumer to expect Set<Foo> and Set<Bar> to be bound separately?

Not in the least! Otherwise, won't everything just be Set<? extends Object>?

Ok. But isn't it just as easy to create injections for Set<? extends Foo> and Set<? extends Bar> as it is to create injections Set<Foo> and Set<Bar>, in the same way and for the same reasons?

 

 

It seems to me in this case, the user almost certainly intended to bind @IntoSet Bar as Foo, unless *all* of their @IntoSet bind Bar.

The return type of an @IntoSet is used exactly to specify which set to bind into. After all, I could have the same @Provides @IntoSet method return Bar, Foo, or Object. That's how I choose.

Yes I am well aware of that. Which is why I said that one *could* distinguish between these cases and inject into Set<? extends Foo>, and Set<? extends Bar> and (crazily) Set<? extends Object>. See point above.

 

 

However, some extra code could support assignment to Set<? extends Foo> and Set<? extends Bar> to support this case as well.

 

1. As I said before, I think the unavailability of add() is a mere side effect of the wildcard in Set<? extends Foo>, not its intended meaning.

I wouldn't say its a "mere" side effect. One could say its a "side effect", but its a side-effect that is *fundamental* to Set being covariant on `Foo`: the consumer of the Set does not know what the type of the objects in the Set are, so it simply cannot add to it.

A hypothetical Set method could be addIfValid(Object), which would accept any object and add it only if it is a subtype of the accepted type of the Set. (That would require runtime type checking under the hood, of course.) That might not be a great idea, but it wouldn't fundamentally alter the notion of a typed Set. However, it would mean that Set<? extends Foo> is no longer necessarily unmodifiable. That's why I don't think the unmodifiability is a fundamental aspect of Set.

This is true, but a) this is an unlikely hypothetical, and b) we still have the runtime immutability to handle this case i.e. we can gain guarantees we don't have now, and never lose any guarantees we already have.

 

 

 

1. That conflict is by design.

That design is what we're discussing here, isn't it? :-)

Well, you started this thread by asking whether Dagger's considering Set<Foo> and Set<? extends Foo> different keys was a bug. I'm just saying it's not a bug—it's a feature. :)

Just to clarify -- that is how I started the thread but based on the discussion I realized the error of my ways and moved on from that position :-) Sorry I didn't make that clear. I am now discussing the design and not whether there is a bug in Dagger. But as I already acknowledged, its likely my proposal does not improve the design significantly enough to bother with.

 

 

 

1. Multibinding isn't intended to be an advanced feature. It's a basic and very useful feature of Dagger. I wouldn't want to make that any harder for people to use.

I understand what you're asking for, but I think the current multibinding feature is what we want.

Ok, fair enough. I can see both sides of the issue and certainly understand that the benefits (if any) of what I'm proposing aren't worth the effort required. Thanks for the discussion though!

No worries; it's been helpful!

For instance, one thing +Gregory Kick and I discussed as a result of this conversation was that the practice of injecting Provider<Foo> is not really technically correct (the best kind). You really want to inject a Provider<? extends Foo>. If everyone did that, the generated code might not have to suppress warnings when for instance it gives you a Provider<Bar> because of a @Binds. But unfortunately, Java's type system doesn't know that Provider<Bar> is a valid subtype of Provider<Foo>; hence the technical need to inject Provider<? extends Foo>.

We decided that making everyone inject Provider<? extends Foo> was too onerous, since its only practical benefit would be to allow generated code not to suppress some warnings. However, we might allow people to inject Provider<? extends Foo> if they want to—which currently won't work. (We don't actually think anyone wants this enough for us to prioritize implementing it at the moment.)

Cool, glad this prompted some other substantive and useful discussion. I'd personally favor correctness and would use such a feature, but understand why its priority would be quite low.

[David P. Baker]

On Wed, Oct 19, 2016 at 1:38 PM Raman Gupta <rocke...@gmail.com> wrote:

Ok I had intended my last message to be my last word on the subject... but as usual I can't resist poking the bear a bit more :-)

On Wednesday, October 19, 2016 at 12:52:08 PM UTC-4, David P. Baker wrote:

On Wed, Oct 19, 2016 at 11:43 AM Raman Gupta <rocke...@gmail.com> wrote:

On Wednesday, October 19, 2016 at 10:21:18 AM UTC-4, David P. Baker wrote:

Raman, I really do appreciate your ideas here. Let me respond to each:

1. Doesn't that mean you'll never be able to bind @IntoSet Foo and @IntoSet Bar (which extends Foo) and get two different sets? (Set<Foo> and Set<Bar>)

Without additional code to distinguish them, yes, that would be true. Wouldn't it be unusual to the point of likely being an error for a multibinding consumer to expect Set<Foo> and Set<Bar> to be bound separately?

Not in the least! Otherwise, won't everything just be Set<? extends Object>?

Ok. But isn't it just as easy to create injections for Set<? extends Foo> and Set<? extends Bar> as it is to create injections Set<Foo> and Set<Bar>, in the same way and for the same reasons?

In that case, won't @IntoSet Bar be in both sets? As a user, how do I prevent that?

 

 

 

It seems to me in this case, the user almost certainly intended to bind @IntoSet Bar as Foo, unless *all* of their @IntoSet bind Bar.

The return type of an @IntoSet is used exactly to specify which set to bind into. After all, I could have the same @Provides @IntoSet method return Bar, Foo, or Object. That's how I choose.

Yes I am well aware of that. Which is why I said that one *could* distinguish between these cases and inject into Set<? extends Foo>, and Set<? extends Bar> and (crazily) Set<? extends Object>. See point above.

OK, maybe I'm not understanding you. I thought you wanted @IntoSet Bar to contribute to Set<? extends Bar> as well as Set<? extends Foo>, etc. If not, then fine.

[Raman Gupta]

On Wednesday, October 19, 2016 at 2:59:24 PM UTC-4, David P. Baker wrote:

On Wed, Oct 19, 2016 at 1:38 PM Raman Gupta <rocke...@gmail.com> wrote:

Ok I had intended my last message to be my last word on the subject... but as usual I can't resist poking the bear a bit more :-)

On Wednesday, October 19, 2016 at 12:52:08 PM UTC-4, David P. Baker wrote:

On Wed, Oct 19, 2016 at 11:43 AM Raman Gupta <rocke...@gmail.com> wrote:

On Wednesday, October 19, 2016 at 10:21:18 AM UTC-4, David P. Baker wrote:

Raman, I really do appreciate your ideas here. Let me respond to each:

1. Doesn't that mean you'll never be able to bind @IntoSet Foo and @IntoSet Bar (which extends Foo) and get two different sets? (Set<Foo> and Set<Bar>)

Without additional code to distinguish them, yes, that would be true. Wouldn't it be unusual to the point of likely being an error for a multibinding consumer to expect Set<Foo> and Set<Bar> to be bound separately?

Not in the least! Otherwise, won't everything just be Set<? extends Object>?

Ok. But isn't it just as easy to create injections for Set<? extends Foo> and Set<? extends Bar> as it is to create injections Set<Foo> and Set<Bar>, in the same way and for the same reasons?

In that case, won't @IntoSet Bar be in both sets? As a user, how do I prevent that?

No this was a misunderstanding, see below.

 

 

 

 

It seems to me in this case, the user almost certainly intended to bind @IntoSet Bar as Foo, unless *all* of their @IntoSet bind Bar.

The return type of an @IntoSet is used exactly to specify which set to bind into. After all, I could have the same @Provides @IntoSet method return Bar, Foo, or Object. That's how I choose.

Yes I am well aware of that. Which is why I said that one *could* distinguish between these cases and inject into Set<? extends Foo>, and Set<? extends Bar> and (crazily) Set<? extends Object>. See point above.

OK, maybe I'm not understanding you. I thought you wanted @IntoSet Bar to contribute to Set<? extends Bar> as well as Set<? extends Foo>, etc. If not, then fine.

Nope, I don't think I ever implied that, though I don't think I was explicit about it. When I said "I believe having `@IntoSet` binding to `Set<? extends Foo>` would be better" I should have added "using the same logic as is used today for determining the bindings of Set<Foo>". I like Dagger's ability to choose the binding type at the provider.

[Gregory Kick]

I've been watching this tread, and now that it's settled down a bit I figured I'd chime in with a few thoughts.

1) For injection all types are treated as invariant.  No notion of assignability is ever considered when performing injection for sets or any other type.  In order for Foo to be injected, Foo must be bound.  There are no plans to change that generally or for any particular subset of bindings.

2) While it is technically true that we could bind sets with '? extends',  in practice all sets of our sets are actually Set<Foo>.  Due to the invariance in binding, all @IntoSet bindings are bound as Foo and all @ElementsIntoSet bindings are Set<Foo>.  Even sets contributed with @Binds need a defensive copy, so those are also Set<Foo>.  So, the ability to request a Set of something that extends Foo is fine enough, but only matters in theory -- in all cases that you might be able to accept something else at your injection site, you're only going to get an instance declared as Set<Foo> anyway.

3) That said, there's no reason to preclude binding Set<? extends Foo>, but IMO @Binds is the appropriate way to do that:

  @Binds Set<? extends Foo> bind(Set<Foo> set);

4) I think the assertion has been made that somehow adding a wildcard precludes the use of the modification operations.  This is not the case because wildcard capture can make that work for add().  In fact, that is exactly the example in https://docs.oracle.com/javase/tutorial/java/generics/capture.html .  Other modification methods like remove accept Object, so the type variable doesn't make any difference at all.  Adding a wildcard does little more that make a particular form of modification more difficult, but doesn't actually protect against anything.

Hope that clarifies some of the thinking here.

[Raman Gupta]

On Wednesday, October 19, 2016 at 3:26:55 PM UTC-4, Gregory Kick wrote:

I've been watching this tread, and now that it's settled down a bit I figured I'd chime in with a few thoughts.

Thanks for your thoughts Gregory.

 

1) For injection all types are treated as invariant.  No notion of assignability is ever considered when performing injection for sets or any other type.  In order for Foo to be injected, Foo must be bound.  There are no plans to change that generally or for any particular subset of bindings.

I admit my thinking on this was originally muddied a bit, but this has been clear to me now for a while, and makes total sense.

 

2) While it is technically true that we could bind sets with '? extends',  in practice all sets of our sets are actually Set<Foo>.  Due to the invariance in binding, all @IntoSet bindings are bound as Foo and all @ElementsIntoSet bindings are Set<Foo>.  Even sets contributed with @Binds need a defensive copy, so those are also Set<Foo>.  So, the ability to request a Set of something that extends Foo is fine enough, but only matters in theory -- in all cases that you might be able to accept something else at your injection site, you're only going to get an instance declared as Set<Foo> anyway.

Right, that makes sense, and is in line with the latest proposals.

 

3) That said, there's no reason to preclude binding Set<? extends Foo>, but IMO @Binds is the appropriate way to do that:

  @Binds Set<? extends Foo> bind(Set<Foo> set);

Whoa, this works. Cool. I think that pretty much makes this a non-issue!

 

4) I think the assertion has been made that somehow adding a wildcard precludes the use of the modification operations.  This is not the case because wildcard capture can make that work for add().  In fact, that is exactly the example in https://docs.oracle.com/javase/tutorial/java/generics/capture.html .  Other modification methods like remove accept Object, so the type variable doesn't make any difference at all.  Adding a wildcard does little more that make a particular form of modification more difficult, but doesn't actually protect against anything.

Hope that clarifies some of the thinking here.

Thanks again!

Regards,

Raman

[Raman Gupta]

On Monday, October 3, 2016 at 11:51:40 PM UTC-4, Raman Gupta wrote:

Dagger2 is unable to inject a `Set<T>` into a covariant `Set<? extends T>` field. Is there any particular reason for this, or could this be considered a bug?

As context: I found this while prototyping Dagger2 with Kotlin, in which `Set<T>` is read-only and can therefore accept covariant types. One has to fall-back to declaring the injection site as a `MutableSet<T>` to get the invariant type, even if the `Set` is intended to be read-only.

For the final purposes of tying the thread back to the original question:

We can mark this thread as "SOLVED" -- Dagger considers bindings for `Set<? extends Foo>` and `Set<Foo>` to be different, for good reason. Gregory Kick stated one can simply add a binding as follows to make this work:

@Binds Set<? extends Foo> bind(Set<Foo> set);

and indeed that does work.

For any future searchers who are interested in the solution for Kotlin (which is what set me on this path in the first place), there is one caveat: in Kotlin one has to do the bind as:

@Binds fun bind(set: MutableSet<Foo>): Set<@JvmWildcard Foo>

The @JvmWildcard is necessary to workaround Kotlin's default behavior to not generate wildcards in return values for maximum interop with java code:

To make Kotlin APIs work in Java we generate Box<Super> as Box<? extends Super> for covariantly defined Box (or Foo<? super Bar> for contravariantly defined Foo) when it appears as a parameter. When it's a return value, we don't generate wildcards, because otherwise Java clients will have to deal with them (and it's against the common Java coding style). -- Source

 

Regards,

Raman

[Daivid Silverio]

Hello There,

I found this thread when I was trying to convert some examples from this repository (https://github.com/heliofvj/dagger-sandbox) to kotlin.
I got stuck on the example20 (https://github.com/heliofvj/dagger-sandbox/blob/master/src/main/java/com/dagger/sandbox/Example20.java) where @IntoSet and @ElementsIntoSet are being used and working fine on Java, but when try something like this:

@Module
class SimpleModuleA {
    @Provides
    @Singleton
    @IntoSet
    fun provideDependency11() = Dependency11()
}

 
or

@Module
class SimpleModuleB {
    @Provides
    @Singleton
    @ElementsIntoSet
    fun provideDependencies13(): Set<Dependency1> {
        val dependencies = mutableSetOf<Dependency1>()
        for (i in 1..2) {
            dependencies.add(Dependency13())
        }

        return dependencies
    }
}

And injecting like:

@Inject
lateinit var dependencies: Set<Dependency1>

or

@Singleton
class Dependency2 @Inject constructor(dep1s: Set<Dependency1>)

Dagger will complain about the Set not being provided.

Any idea on how to make this work? Or is it just wrong?

I didn't get the idea on how to use @Binds fun bind(set: MutableSet<Foo>): Set<@JvmWildcard Foo>, or if it even applies to my case.

[Christian Edward Gruber]

My own situation as of last night is that when I bind @IntoSet I get Map<String, Action>, but when I attempt to inject Map<String, Provider<Action>>, Kotlin's (or kapt's) interpretation of...

  fun convert(actions: Map<String, Action>): Map<Regex, Action> {...}

turns into

   Map<Regex, Action> convert(Map<String, ? extends Action> actions) {...}

That's not strictly Dagger's problem, really - but it makes it awkward since Map<String, ? extends Action> wasn't bound.  I was able to play with @JvmSuppressWildcards, which saved the day (thanks Ron for the hint).

There's a really helpful Stackoverflow post about this issue, which helped me both frame it and see what options I had.  https://stackoverflow.com/questions/28480666/how-to-enforce-generic-type-with-kotlin-interop

In your case listed below, I suspect if you check your errors that it's not the @Binds statement that's hte problem, but your constructor, because Kotlin is being more fastidious than you want it to be, and you can suppress that as I did above. 

Christian.

P.S.  I konw I don't want Map<String, Action> but Map<String, Provider<Action>>.  I do, in the real code, but I wanted this example simple, and I tested it both ways - it fails in both cases, wiht the injection sites expecting "Map<String, ? extends Provider<Action>>", because: reasons. :/

[Thomas Broyer]

See also https://github.com/google/dagger/issues/668

[Christian Edward Gruber]

Perfect. Your comment (penultimate pre-closing comment) basically was the best sum-up. Thanks!

c.