From c9c9f9d8380166e3543141980e8bf017eff3a9aa Mon Sep 17 00:00:00 2001 From: Chris Povirk Date: Wed, 30 Aug 2023 11:15:25 -0400 Subject: [PATCH] Insert `!!` to make some types in generated code non-nullable. Hi, Square folks. The background here is that we're working to put nullness annotations on more "JDK classes," such as `java.lang.Class`, at least in Google's build system. That means that calls to methods like `clazz.getMethod(...)` and `clazz.getDeclaredConstructor` must pass non-nullable `Class` instances. If they don't, we see errors: ``` error: type mismatch: inferred type is Class? but Class<*> was expected String::class.java, String::class.java, Int::class.javaPrimitiveType, String::class.java, ^ ``` ``` error: type mismatch: inferred type is Class? but Class<*> was expected String::class.java, FooResponse::class.java, Int::class.javaPrimitiveType, ^ ``` ``` error: type mismatch: inferred type is Class? but Class<*> was expected Util.DEFAULT_CONSTRUCTOR_MARKER).also { this.constructorRef = it } ^ ``` Expressions like `Int::class.javaPrimitiveType` are always non-nullable, so we can use `!!` (or `checkNotNull` or whatever you might prefer) there without risk. (Note that it would be possible to avoiding using `!!` for `INT_TYPE_BLOCK`: We could do so by passing `Integer.TYPE`. The more general case of `TypeName.asTypeBlock` could theoretically be handled similarly but would likely require maintaining a mapping from Kotlin type name to Java primitive wrapper class.) As for `DEFAULT_CONSTRUCTOR_MARKER`, I haven't looked at the generated code enough to prove that it's non-nullable here. But if I'm reading the bytecode right, the property value is being passed to `getDeclaredConstructor`, so I'd expect NPE there if it were null. Let me know what you think. I could check with our internal Moshi owners about maintaining this as a Google-local patch if that's better, but I'm hoping that this is a fairly harmless change, and our research so far suggests that we _probably_ won't be back soon to ask about other changes for this nullness work. --- .../com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt | 4 ++-- .../java/com/squareup/moshi/kotlin/codegen/api/kotlintypes.kt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt b/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt index 5bba552..6ea3494 100644 --- a/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt +++ b/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt @@ -58,9 +58,9 @@ public class AdapterGenerator( ) { private companion object { - private val INT_TYPE_BLOCK = CodeBlock.of("%T::class.javaPrimitiveType", INT) + private val INT_TYPE_BLOCK = CodeBlock.of("%T::class.javaPrimitiveType!!", INT) private val DEFAULT_CONSTRUCTOR_MARKER_TYPE_BLOCK = CodeBlock.of( - "%M", + "%M!!", MemberName(MOSHI_UTIL_PACKAGE, "DEFAULT_CONSTRUCTOR_MARKER"), ) private val CN_MOSHI = Moshi::class.asClassName() diff --git a/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/kotlintypes.kt b/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/kotlintypes.kt index 6d22ebf..9e84a95 100644 --- a/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/kotlintypes.kt +++ b/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/kotlintypes.kt @@ -118,7 +118,7 @@ internal fun TypeName.asTypeBlock(): CodeBlock { // Remove nullable but keep the java object type CodeBlock.of("%T::class.javaObjectType", copy(nullable = false)) } else { - CodeBlock.of("%T::class.javaPrimitiveType", this) + CodeBlock.of("%T::class.javaPrimitiveType!!", this) } } UNIT, Void::class.asTypeName(), NOTHING -> throw IllegalStateException("Parameter with void, Unit, or Nothing type is illegal")