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

JavaConfig bean resolution fails with generics + Kotlin declaration-site variance #22313

Open
sdeleuze opened this issue Jan 28, 2019 · 10 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support type: enhancement A general enhancement
Projects
Milestone

Comments

@sdeleuze
Copy link
Contributor

Declaration-site variance used in the very common kotlin.Pair class seems to impact bean resolution and currently prevents bean resolution to work as expected by the end user.

Based on an original report on Twitter, I have created a more focused repro project.

Following test works with class Tuple<A, B> but fails with class Tuple<out A, out B>, which is pretty hard to diagnose when the user is using kotlin.Pair which is provided in Kotlin standard library.

@RunWith(SpringRunner::class)
@ContextConfiguration(classes = [Config::class])
class TypeProjectionTest {

  @Inject
  private lateinit var ctx: Container<Tuple<String, String>>

  @Test
  fun testContext() {
  }

}

// Variant 1: works when not using declaration-site variance
//class Tuple<A, B>

// Variant 2: Fails with using declaration-site variance
class Tuple<out A, out B>

private interface Container<T>
private class ContainerTuple : Container<Tuple<String, String>>

@Configuration
private open class Config {

  @Bean
  open fun containerPair() : ContainerTuple {
    return ContainerTuple()
  }

}

@jhoeller Do you think there is something to refine in our bean resolution algorithm to support this kind of Kotlin declaration-site variance?

@sdeleuze sdeleuze added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 28, 2019
@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 11, 2019
@sbrannen sbrannen added this to To Triage in Tracking Feb 18, 2019
@encircled
Copy link
Contributor

In the end in java code it is translated to Tuple<String, String> and Tuple<? extends String, ? extends String>, which is not allowed for injection (and should stay so I believe).

The only solution is to have kotlinized version of org.springframework.core.ResolvableType, where generic type variance would be available and can be considered when looking for the injection candidates.

@sdeleuze sdeleuze added the theme: kotlin An issue related to Kotlin support label Jan 19, 2022
@sdeleuze sdeleuze self-assigned this Feb 8, 2023
@sdeleuze sdeleuze added the type: enhancement A general enhancement label Feb 9, 2023
@sdeleuze sdeleuze closed this as not planned Won't fix, can't repro, duplicate, stale Feb 24, 2023
@sdeleuze sdeleuze added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 24, 2023
@sdeleuze sdeleuze removed the status: duplicate A duplicate of another issue label Feb 24, 2023
@sdeleuze sdeleuze reopened this Feb 24, 2023
@sdeleuze sdeleuze added this to the 6.x Backlog milestone Feb 24, 2023
@sdeleuze
Copy link
Contributor Author

See also #20494 even more popular use case on collections.

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Mar 14, 2023

Let's try to tackle that in parallel of #24033 since there may be some links related to type with resolved bounds. The fix implementation should not be tied to Kotlin (but we will introduce Kotlin tests for that).

@sdeleuze sdeleuze modified the milestones: 6.1.0-M1, 6.1.0-M2 Jun 12, 2023
sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue Jun 30, 2023
This commit adds support for WildcardType bounds resolution,
commonly seen in Kotlin due to declaration-site variance,
but also possible in Java even if less common.

Closes spring-projectsgh-22313
@sdeleuze sdeleuze reopened this Jul 3, 2023
@sdeleuze
Copy link
Contributor Author

sdeleuze commented Jul 3, 2023

Reopening as I mixed-up issue numbers in a recent commit message.

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Jul 4, 2023

I have added the repro as a unit test here.

With declaration-site variance, there is a type mismatch in GenericTypeAwareAutowireCandidateResolver#checkGenericTypeMatch here because in ResolvableType#isAssignableFrom(ResolvableType, Map<Type, Type>):

  • The type of fun container() : TupleContainer is resolved to Tuple<String, String>
  • The type of lateinit var container: Container<Tuple<String, String>> is resolved to Tuple<? extends String, ? extends String>

Tuple<String, String> is not assignable from Tuple<? extends String, ? extends String> so the function returns false.

@sdeleuze
Copy link
Contributor Author

I am waiting a feedback from Kotlin team, so I will move this issue to M3.

@sdeleuze sdeleuze modified the milestones: 6.1.0-M2, 6.1.0-M3 Jul 10, 2023
@jhoeller jhoeller modified the milestones: 6.1.0-M3, 6.1.0-M4 Jul 17, 2023
@sdeleuze
Copy link
Contributor Author

sdeleuze commented Aug 7, 2023

Kotlin team is on PTO, so I have to move this issue to RC1 (tentatively).

@sdeleuze sdeleuze modified the milestones: 6.1.0-M4, 6.1.0-RC1 Aug 7, 2023
@sdeleuze
Copy link
Contributor Author

sdeleuze commented Aug 31, 2023

Kotlin team confirmed my proposal to resolve generic types with variance make sense, should be testable on Java via use-site variance. So I will move forward with that strategy and see how it goes.

@sdeleuze
Copy link
Contributor Author

sdeleuze commented Sep 5, 2023

After a deeper look, it looks like we are going to need to leverage Kotlin reflection in either GenericTypeAwareAutowireCandidateResolver or ResolvableType bounds resolution because the Java algorithm looks correct, but there is no declaration-site variance to use for proper comparison.

We need to compare the use-side variance (available on both Java and Kotlin) to declaration-site variance (available only via Kotlin reflection).

The Kotlin team has confirmed there is no way to get a Ktype from a JVM Type so this is going to need extra logic to find the information; potentially via the KClass.

@sdeleuze sdeleuze modified the milestones: 6.1.0-RC1, 6.1.x Sep 7, 2023
@sdeleuze
Copy link
Contributor Author

sdeleuze commented Sep 7, 2023

I move this issue to 6.1.x bucket in order to be able to move forward on CRaC and Coroutines refinements. Since this issue will likely be about Kotlin-specific refinements, it can probably come in a patch release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support type: enhancement A general enhancement
Projects
No open projects
Tracking
  
To Triage
Development

No branches or pull requests

4 participants