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<Int>? but Class<*> was expected
          String::class.java, String::class.java, Int::class.javaPrimitiveType, String::class.java,
                                                  ^
```

```
error: type mismatch: inferred type is Class<Int>? but Class<*> was expected
          String::class.java, FooResponse::class.java, Int::class.javaPrimitiveType,
                                                       ^
```

```
error: type mismatch: inferred type is Class<CapturedType(*)>? 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.
This commit is contained in:
Chris Povirk
2023-08-30 11:15:25 -04:00
parent 3879185dfd
commit c9c9f9d838
2 changed files with 3 additions and 3 deletions

View File

@@ -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()

View File

@@ -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")