From 51d23b5b330c1ff797c1417f8b3ec58ededa493d Mon Sep 17 00:00:00 2001 From: Eric Cochran Date: Tue, 24 Apr 2018 13:04:10 -0700 Subject: [PATCH] Fix error message for assigning to non-null properties. instead of falling down to "Required property 'a' missing at $" --- .../com/squareup/moshi/AdapterGenerator.kt | 22 ++++++++++++--- .../squareup/moshi/GeneratedAdaptersTest.kt | 28 +++++++++++++++++++ .../moshi/kotlin/KotlinJsonAdapterTest.kt | 28 +++++++++++++++++++ 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/kotlin-codegen/compiler/src/main/java/com/squareup/moshi/AdapterGenerator.kt b/kotlin-codegen/compiler/src/main/java/com/squareup/moshi/AdapterGenerator.kt index 2e408ac..4899d4f 100644 --- a/kotlin-codegen/compiler/src/main/java/com/squareup/moshi/AdapterGenerator.kt +++ b/kotlin-codegen/compiler/src/main/java/com/squareup/moshi/AdapterGenerator.kt @@ -181,13 +181,27 @@ internal class AdapterGenerator( propertyList.forEachIndexed { index, property -> if (property.differentiateAbsentFromNull) { result.beginControlFlow("%L -> ", index) - result.addStatement("%N = %N.fromJson(%N)", - property.localName, nameAllocator.get(property.delegateKey), readerParam) + if (property.delegateKey.nullable) { + result.addStatement("%N = %N.fromJson(%N)", + property.localName, nameAllocator.get(property.delegateKey), readerParam) + } else { + result.addStatement("%N = %N.fromJson(%N)" + + " ?: throw %T(\"Non-null value '%N' was null at \${%N.path}\")", + property.localName, nameAllocator.get(property.delegateKey), readerParam, + JsonDataException::class, property.localName, readerParam) + } result.addStatement("%N = true", property.localIsPresentName) result.endControlFlow() } else { - result.addStatement("%L -> %N = %N.fromJson(%N)", - index, property.localName, nameAllocator.get(property.delegateKey), readerParam) + if (property.delegateKey.nullable) { + result.addStatement("%L -> %N = %N.fromJson(%N)", + index, property.localName, nameAllocator.get(property.delegateKey), readerParam) + } else { + result.addStatement("%L -> %N = %N.fromJson(%N)" + + " ?: throw %T(\"Non-null value '%N' was null at \${%N.path}\")", + index, property.localName, nameAllocator.get(property.delegateKey), readerParam, + JsonDataException::class, property.localName, readerParam) + } } } diff --git a/kotlin-codegen/integration-test/src/test/kotlin/com/squareup/moshi/GeneratedAdaptersTest.kt b/kotlin-codegen/integration-test/src/test/kotlin/com/squareup/moshi/GeneratedAdaptersTest.kt index cc879e9..60bc77c 100644 --- a/kotlin-codegen/integration-test/src/test/kotlin/com/squareup/moshi/GeneratedAdaptersTest.kt +++ b/kotlin-codegen/integration-test/src/test/kotlin/com/squareup/moshi/GeneratedAdaptersTest.kt @@ -383,6 +383,20 @@ class GeneratedAdaptersTest { } } + @Test fun nonNullConstructorParameterCalledWithNullFromAdapterFailsWithJsonDataException() { + val moshi = Moshi.Builder().add(object { + @FromJson fun fromJson(string: String): String? = null + }).build() + val jsonAdapter = moshi.adapter(HasNonNullConstructorParameter::class.java) + + try { + jsonAdapter.fromJson("{\"a\":\"hello\"}") + fail() + } catch (expected: JsonDataException) { + assertThat(expected).hasMessage("Non-null value 'a' was null at \$.a") + } + } + @JsonClass(generateAdapter = true) class HasNonNullConstructorParameter(val a: String) @@ -539,6 +553,20 @@ class GeneratedAdaptersTest { } } + @Test fun nonNullPropertySetToNullFromAdapterFailsWithJsonDataException() { + val moshi = Moshi.Builder().add(object { + @FromJson fun fromJson(string: String): String? = null + }).build() + val jsonAdapter = moshi.adapter(HasNonNullProperty::class.java) + + try { + jsonAdapter.fromJson("{\"a\":\"hello\"}") + fail() + } catch (expected: JsonDataException) { + assertThat(expected).hasMessage("Non-null value 'a' was null at \$.a") + } + } + @JsonClass(generateAdapter = true) class HasNonNullProperty { var a: String = "" diff --git a/kotlin/src/test/java/com/squareup/moshi/kotlin/KotlinJsonAdapterTest.kt b/kotlin/src/test/java/com/squareup/moshi/kotlin/KotlinJsonAdapterTest.kt index e1111ae..d9b0519 100644 --- a/kotlin/src/test/java/com/squareup/moshi/kotlin/KotlinJsonAdapterTest.kt +++ b/kotlin/src/test/java/com/squareup/moshi/kotlin/KotlinJsonAdapterTest.kt @@ -152,6 +152,20 @@ class KotlinJsonAdapterTest { } } + @Test fun nonNullConstructorParameterCalledWithNullFromAdapterFailsWithJsonDataException() { + val moshi = Moshi.Builder().add(object { + @FromJson fun fromJson(string: String): String? = null + }).add(KotlinJsonAdapterFactory()).build() + val jsonAdapter = moshi.adapter(HasNonNullConstructorParameter::class.java) + + try { + jsonAdapter.fromJson("{\"a\":\"hello\"}") + fail() + } catch (expected: JsonDataException) { + assertThat(expected).hasMessage("Non-null value 'a' was null at \$") + } + } + class HasNonNullConstructorParameter(val a: String) @Test fun nonNullPropertySetToNullFailsWithJsonDataException() { @@ -166,6 +180,20 @@ class KotlinJsonAdapterTest { } } + @Test fun nonNullPropertySetToNullFromAdapterFailsWithJsonDataException() { + val moshi = Moshi.Builder().add(object { + @FromJson fun fromJson(string: String): String? = null + }).add(KotlinJsonAdapterFactory()).build() + val jsonAdapter = moshi.adapter(HasNonNullProperty::class.java) + + try { + jsonAdapter.fromJson("{\"a\":\"hello\"}") + fail() + } catch (expected: JsonDataException) { + assertThat(expected).hasMessage("Non-null value 'a' was null at \$") + } + } + class HasNonNullProperty { var a: String = "" }