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.

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

Create a new multibinder alternate for Set<? extends T> #1366

Closed

Conversation

swankjesse
Copy link
Contributor

This is a simple linked binding to the original multibinder set. This is
safe because 'Set<? extends T>' is assignable from 'Set'.

This is useful for projects integrating Guice and Kotlin. In Kotlin there
are different types for read-only vs. read-write Sets. When declaring a
read-only Set, the bytecode includes a wildcard.

In particular, this Kotlin:

var setA: Set<Runnable>
var setB: MutableSet<Runnable>

is equivalent to this Java:

Set<? extends Runnable> setA;
Set<Runnable> setB;

Using Kotlin with Guice multibindings is quite annoying because the binding
to Set<Runnable> is typically provided, but Set<? extends Runnable> is
needed.

The typical Kotlin workaround is also ugly:

var setA: Set<@JvmSuppressWildcards Runnable>

With this fix, Guice multibindings and Guice should just work.

I expect this may be backwards-incompatible with a very small number of Kotlin
applications that implement a different workaround to the above problem:

@Provides
Set<? extends Runnable> provideRunnables(Set<Runnable> set) {
  return set;
}

I don't expect backwards incompatibility with Java programs. In such programs
the behavior change will result in a fast failure (injector fails to create)
which is easy to diagnose and fix.

Closes: #1282

This is a simple linked binding to the original multibinder set. This is
safe because 'Set<? extends T>' is assignable from 'Set<T>'.

This is useful for projects integrating Guice and Kotlin. In Kotlin there
are different types for read-only vs. read-write Sets. When declaring a
read-only Set, the bytecode includes a wildcard.

In particular, this Kotlin:

    var setA: Set<Runnable>
    var setB: MutableSet<Runnable>

is equivalent to this Java:

    Set<? extends Runnable> setA;
    Set<Runnable> setB;

Using Kotlin with Guice multibindings is quite annoying because the binding
to `Set<Runnable>` is typically provided, but `Set<? extends Runnable>` is
needed.

The typical Kotlin workaround is also ugly:

    var setA: Set<@JvmSuppressWildcards Runnable>

With this fix, Guice multibindings and Guice should just work.

I expect this may be backwards-incompatible with a very small number of Kotlin
applications that implement a different workaround to the above problem:

    @provides
    Set<? extends Runnable> provideRunnables(Set<Runnable> set) {
      return set;
    }

I don't expect backwards incompatibility with Java programs. In such programs
the behavior change will result in a fast failure (injector fails to create)
which is easy to diagnose and fix.
@swankjesse swankjesse force-pushed the jwilson.0721.bind_set_of_extends branch from 1308029 to a5d2f01 Compare July 22, 2020 01:12
nick-someone pushed a commit that referenced this pull request Jul 30, 2020
Create a new multibinder alternate for Set<? extends T>

Closes #1366

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=323843272
nick-someone pushed a commit that referenced this pull request Jul 30, 2020
Create a new multibinder alternate for Set<? extends T>

Closes #1366

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=323843272
ShoOgino pushed a commit to ShoOgino/guiceFile that referenced this pull request Oct 14, 2020
Create a new multibinder alternate for Set<? extends T>

Closes #1366

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=323843272
ShoOgino pushed a commit to ShoOgino/guiceMethod that referenced this pull request Oct 14, 2020
Create a new multibinder alternate for Set<? extends T>

Closes #1366

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=323843272
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to ignore wildcards in keys
2 participants