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

Document Kotlin declaration site variance subtleties #31370

Closed
wilkinsona opened this issue Oct 5, 2023 · 4 comments
Closed

Document Kotlin declaration site variance subtleties #31370

wilkinsona opened this issue Oct 5, 2023 · 4 comments
Assignees
Labels
theme: kotlin An issue related to Kotlin support type: documentation A documentation task
Milestone

Comments

@wilkinsona
Copy link
Member

Affects: 5.3.x

For background, this problem was identified while looking at https://stackoverflow.com/questions/77233335/how-to-register-new-collection-factories-in-spring-boot.

With a Converter implemented in Kotlin for a List to Guava's ImmutableList conversion:

class ListToImmutableListConverter : Converter<List<Any>, ImmutableList<Any>> {
    override fun convert(source: List<Any>): ImmutableList<Any> = ImmutableList.copyOf(source)
}

ResolvableType resolves the first Any to ? and the second to Object:

val type: ResolvableType = ResolvableType.forClass(ListToImmutableListConverter::class.java).`as`(Converter::class.java)
println(type)
org.springframework.core.convert.converter.Converter<java.util.List<?>, com.google.common.collect.ImmutableList<java.lang.Object>>

Contrastingly, using Java and ? results in both being resolved to ?:

static class ListToImmutableList implements Converter<List<?>, ImmutableList<?>>  {

	@Override
	public ImmutableList<?> convert(List<?> source) {
		return ImmutableList.copyOf(source);
	}
		
}
ResolvableType type = ResolvableType.forClass(ListToImmutableList.class).as(Converter.class);
System.out.println(type);
org.springframework.core.convert.converter.Converter<java.util.List<?>, com.google.common.collect.ImmutableList<?>>

Resolving the target type to ImmutableList<Object> rather than ImmutableList<?> is problematic as it prevents the converter from matching a List<String> to ImmutableList<String> conversion.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 5, 2023
@sdeleuze sdeleuze self-assigned this Oct 5, 2023
@sdeleuze sdeleuze added the theme: kotlin An issue related to Kotlin support label Oct 5, 2023
@sdeleuze
Copy link
Contributor

sdeleuze commented Oct 5, 2023

I suspect this kind of behavior is caused by Kotlin declaration site variance. When you declare List<Any> in Kotlin, List is not java.util.List but kotlin.collections.List which is declared as List<out E> so conceptually the equivalent of java.util.List<? extends E>, so to get compatible generics I would suggest to try to use ImmutableList<out Any> (usage site variance as the type is a Java type that we just use) in order to have the assignability to Kotlin List<E>:

class ListToImmutableListConverter : Converter<List<Any>, ImmutableList<out Any>> {
    override fun convert(source: List<Any>): ImmutableList<out Any> = ImmutableList.copyOf(source)
}

I know this is pretty subtle but this is how Kotlin deals with lists and when mixing with Java list, there is a need to be aware of the declaration type variance to get type comparison working as expected. I am not sure there is something to fix on Spring Framework side for this specific use case, but please let me know what you think about it.

@wilkinsona
Copy link
Member Author

Thanks very much, @sdeleuze. That fixes the problem.

If there are other areas in Framework where type comparison may not work when using Any, I wonder if this could be documented within https://docs.spring.io/spring-framework/reference/languages/kotlin.html? It's rather low-level but it would have been a long time, if ever, before I figured it out without your guidance.

@sdeleuze sdeleuze added this to the 6.0.13 milestone Oct 6, 2023
@sdeleuze sdeleuze added type: documentation A documentation task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 6, 2023
@sdeleuze sdeleuze changed the title ResolvableType produces inconsistent results with Kotlin's Any Document Kotlin declaration site variance subtleties Oct 6, 2023
@sdeleuze
Copy link
Contributor

sdeleuze commented Oct 6, 2023

Sure, that's a good idea, and that will give me an opportunity to document some guidance for injection related to #22313. As a consequence, I turn this issue into a documentation one.

@wilkinsona
Copy link
Member Author

wilkinsona commented Oct 7, 2023

An update from the question on Stack Overflow: using *, for example ImmutableList<*>, also works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: kotlin An issue related to Kotlin support type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

3 participants