Use of TypeMirror#toString in SpecElementTypeDeterminator is fragile
- Dominant language
- Kotlin
- Stars
- 7.8k
- Forks
- 768
- PR merge metrics
- No merged PRs in 30d
Description
`SpecElementTypeDeterminator` uses `TypeMirror#toString` to test types:
https://github.com/facebook/litho/blob/55e28e58930a53133620fd32406ec8c3a9116c0c/litho-processor/src/main/java/com/facebook/litho/specmodels/processor/SpecElementTypeDeterminator.java#L75
This is fragile because `TypeMirror#toString` will return any type annotations that are present on the type, which breaks the comparison.
I think this isn't currently causing problems because that logic is used to test fields in Kotlin classes, and when using kapt it creates stubs with fully qualified class names, and there's a javac bug that causes the type annotations to not be correctly handled in that case ([JDK-8381925](https://bugs.openjdk.org/browse/JDK-8381925)).
For example with
```
package com.facebook.litho;
public static class FakeKotlinSingleton {
@Nullable public static final com.facebook.litho.FakeKotlinSingleton INSTANCE = null;
}
```
The type should be printed as `com.facebook.litho.@Nullable FakeKotlinSingleton`.
The tests for SpecElementTypeDeterminator don't cover this because they use `javax.annotation.Nullable` instead of `org.jetbrains.annotations.NotNull`, and the javax annotation isn't a type annotation.
https://github.com/facebook/litho/blob/55e28e58930a53133620fd32406ec8c3a9116c0c/litho-it/src/test/com/facebook/litho/specmodels/processor/SpecElementTypeDeterminatorTest.java#L41
Contributor guide
Assessment
This issue has not been assessed yet.