Merge pull request #531 from square/jwilson.0513.track_test_cases

Finish migrating tests from the reflective adapter

kotlin-codegen/compiler/src/main/java/com/squareup/moshi/AdapterGenerator.kt

@@ -319,7 +319,6 @@ internal class AdapterGenerator(
         .returns(jsonAdapterTypeName)
         .addParameter(moshiParam)
 
-    // TODO make this configurable. Right now it just matches the source model
     if (visibility == Visibility.INTERNAL) {
       result.addModifiers(KModifier.INTERNAL)
     }

kotlin-codegen/compiler/src/main/java/com/squareup/moshi/DelegateKey.kt

@@ -53,12 +53,14 @@ internal data class DelegateKey(
       val annotationElement = MoreTypes.asTypeElement(annotationType)
       annotationElement.getAnnotation(java.lang.annotation.Retention::class.java)?.let {
         if (it.value != RetentionPolicy.RUNTIME) {
-          messager.printMessage(ERROR, "JsonQualifier @${MoreTypes.asTypeElement(annotationType).simpleName} must have RUNTIME retention")
+          messager.printMessage(ERROR, "JsonQualifier " +
+              "@${MoreTypes.asTypeElement(annotationType).simpleName} must have RUNTIME retention")
         }
       }
       annotationElement.getAnnotation(java.lang.annotation.Target::class.java)?.let {
         if (ElementType.FIELD !in it.value) {
-          messager.printMessage(ERROR, "JsonQualifier @${MoreTypes.asTypeElement(annotationType).simpleName} must support FIELD target")
+          messager.printMessage(ERROR, "JsonQualifier " +
+              "@${MoreTypes.asTypeElement(annotationType).simpleName} must support FIELD target")
         }
       }
       return this
@@ -84,9 +86,8 @@ internal data class DelegateKey(
     val (initializerString, args) = when {
       qualifiers.isEmpty() -> "" to emptyArray()
       else -> {
-        ", %${standardArgsSize}T.getFieldJsonQualifierAnnotations(javaClass, %${standardArgsSize + 1}S)" to arrayOf(
+        ", %${standardArgsSize}T.getFieldJsonQualifierAnnotations(javaClass, " +
-            Types::class.asTypeName(),
+            "%${standardArgsSize + 1}S)" to arrayOf(Types::class.asTypeName(), adapterName)
-            adapterName)
       }
     }
     val finalArgs = arrayOf(*standardArgs, *args)
@@ -94,7 +95,9 @@ internal data class DelegateKey(
     val nullModifier = if (nullable) ".nullSafe()" else ".nonNull()"
 
     return PropertySpec.builder(adapterName, adapterTypeName, KModifier.PRIVATE)
-        .addAnnotations(qualifiers.map { AnnotationSpec.get(it).toBuilder().useSiteTarget(FIELD).build() })
+        .addAnnotations(qualifiers.map {
+          AnnotationSpec.get(it).toBuilder().useSiteTarget(FIELD).build()
+        })
         .initializer("%1N.adapter%2L(%3L$initializerString)$nullModifier", *finalArgs)
         .build()
   }
@@ -104,9 +107,7 @@ internal data class DelegateKey(
  * Returns a suggested variable name derived from a list of type names. This just concatenates,
  * yielding types like MapOfStringLong.
  */
-private fun List<TypeName>.toVariableNames(): String {
+private fun List<TypeName>.toVariableNames() = joinToString("") { it.toVariableName() }
-  return joinToString("") { it.toVariableName() }
-}
 
 /** Returns a suggested variable name derived from a type name, like nullableListOfString. */
 private fun TypeName.toVariableName(): String {

kotlin-codegen/compiler/src/main/java/com/squareup/moshi/JsonClassCodeGenProcessor.kt

@@ -73,8 +73,8 @@ class JsonClassCodeGenProcessor : KotlinAbstractProcessor(), KotlinMetadataUtils
     super.init(processingEnv)
     generatedType = processingEnv.options[OPTION_GENERATED]?.let {
       if (it !in POSSIBLE_GENERATED_NAMES) {
-        throw IllegalArgumentException(
+        throw IllegalArgumentException("Invalid option value for $OPTION_GENERATED. Found $it, " +
-            "Invalid option value for $OPTION_GENERATED. Found $it, allowable values are $POSSIBLE_GENERATED_NAMES.")
+            "allowable values are $POSSIBLE_GENERATED_NAMES.")
       }
       processingEnv.elementUtils.getTypeElement(it)
     }

kotlin-codegen/compiler/src/main/java/com/squareup/moshi/TargetType.kt

@@ -31,7 +31,9 @@ import me.eugeniomarletti.kotlin.metadata.modality
 import me.eugeniomarletti.kotlin.metadata.shadow.metadata.ProtoBuf.Class
 import me.eugeniomarletti.kotlin.metadata.shadow.metadata.ProtoBuf.Modality.ABSTRACT
 import me.eugeniomarletti.kotlin.metadata.shadow.metadata.ProtoBuf.TypeParameter
+import me.eugeniomarletti.kotlin.metadata.shadow.metadata.ProtoBuf.Visibility.INTERNAL
 import me.eugeniomarletti.kotlin.metadata.shadow.metadata.ProtoBuf.Visibility.LOCAL
+import me.eugeniomarletti.kotlin.metadata.shadow.metadata.ProtoBuf.Visibility.PUBLIC
 import me.eugeniomarletti.kotlin.metadata.shadow.metadata.deserialization.NameResolver
 import me.eugeniomarletti.kotlin.metadata.shadow.util.capitalizeDecapitalize.decapitalizeAsciiOnly
 import me.eugeniomarletti.kotlin.metadata.visibility
@@ -70,6 +72,11 @@ internal data class TargetType(
 
       val proto = typeMetadata.data.classProto
       when {
+        proto.classKind == Class.Kind.ENUM_CLASS -> {
+          messager.printMessage(
+              ERROR, "@JsonClass can't be applied to $element: must not be an enum class", element)
+          return null
+        }
         proto.classKind != Class.Kind.CLASS -> {
           messager.printMessage(
               ERROR, "@JsonClass can't be applied to $element: must be a Kotlin class", element)
@@ -96,6 +103,12 @@ internal data class TargetType(
       val appliedType = AppliedType.get(element)
 
       val constructor = TargetConstructor.primary(typeMetadata, elements)
+      if (constructor.proto.visibility != INTERNAL && constructor.proto.visibility != PUBLIC) {
+        messager.printMessage(ERROR, "@JsonClass can't be applied to $element: " +
+            "primary constructor is not internal or public", element)
+        return null
+      }
+
       val properties = mutableMapOf<String, TargetProperty>()
       for (supertype in appliedType.supertypes(types)) {
         if (supertype.element.asClassName() == OBJECT_CLASS) {

kotlin-codegen/compiler/src/test/java/com/squareup/moshi/CompilerTest.kt

@@ -27,7 +27,29 @@ import javax.annotation.processing.Processor
 class CompilerTest {
   @Rule @JvmField var temporaryFolder: TemporaryFolder = TemporaryFolder()
 
-  @Test fun privateProperty() {
+  @Test fun privateConstructor() {
+    val call = KotlinCompilerCall(temporaryFolder.root)
+    call.inheritClasspath = true
+    call.addService(Processor::class, JsonClassCodeGenProcessor::class)
+    call.addKt("source.kt", """
+        |import com.squareup.moshi.JsonClass
+        |
+        |@JsonClass(generateAdapter = true)
+        |class PrivateConstructor private constructor(var a: Int, var b: Int) {
+        |  fun a() = a
+        |  fun b() = b
+        |  companion object {
+        |    fun newInstance(a: Int, b: Int) = PrivateConstructor(a, b)
+        |  }
+        |}
+        |""".trimMargin())
+
+    val result = call.execute()
+    assertThat(result.exitCode).isEqualTo(ExitCode.COMPILATION_ERROR)
+    assertThat(result.systemErr).contains("constructor is not internal or public")
+  }
+
+  @Test fun privateConstructorParameter() {
     val call = KotlinCompilerCall(temporaryFolder.root)
     call.inheritClasspath = true
     call.addService(Processor::class, JsonClassCodeGenProcessor::class)
@@ -43,6 +65,25 @@ class CompilerTest {
     assertThat(result.systemErr).contains("property a is not visible")
   }
 
+  @Test fun privateProperties() {
+    val call = KotlinCompilerCall(temporaryFolder.root)
+    call.inheritClasspath = true
+    call.addService(Processor::class, JsonClassCodeGenProcessor::class)
+    call.addKt("source.kt", """
+        |import com.squareup.moshi.JsonClass
+        |
+        |@JsonClass(generateAdapter = true)
+        |class PrivateProperties {
+        |  private var a: Int = -1
+        |  private var b: Int = -1
+        |}
+        |""".trimMargin())
+
+    val result = call.execute()
+    assertThat(result.exitCode).isEqualTo(ExitCode.COMPILATION_ERROR)
+    assertThat(result.systemErr).contains("property a is not visible")
+  }
+
   @Test fun interfacesNotSupported() {
     val call = KotlinCompilerCall(temporaryFolder.root)
     call.inheritClasspath = true
@@ -96,6 +137,25 @@ class CompilerTest {
         "error: @JsonClass can't be applied to Outer.InnerClass: must not be an inner class")
   }
 
+  @Test fun enumClassesNotSupported() {
+    val call = KotlinCompilerCall(temporaryFolder.root)
+    call.inheritClasspath = true
+    call.addService(Processor::class, JsonClassCodeGenProcessor::class)
+    call.addKt("source.kt", """
+        |import com.squareup.moshi.JsonClass
+        |
+        |@JsonClass(generateAdapter = true)
+        |enum class KotlinEnum {
+        |  A, B
+        |}
+        |""".trimMargin())
+
+    val result = call.execute()
+    assertThat(result.exitCode).isEqualTo(ExitCode.COMPILATION_ERROR)
+    assertThat(result.systemErr).contains(
+        "error: @JsonClass can't be applied to KotlinEnum: must not be an enum class")
+  }
+
   // Annotation processors don't get called for local classes, so we don't have the opportunity to
   // print an error message. Instead local classes will fail at runtime.
   @Ignore
@@ -245,6 +305,24 @@ class CompilerTest {
     assertThat(result.systemErr).contains("supertype java.util.Date is not a Kotlin type")
   }
 
+  @Test fun extendJavaType() {
+    val call = KotlinCompilerCall(temporaryFolder.root)
+    call.inheritClasspath = true
+    call.addService(Processor::class, JsonClassCodeGenProcessor::class)
+    call.addKt("source.kt", """
+        |import com.squareup.moshi.JsonClass
+        |import com.squareup.moshi.JavaSuperclass
+        |
+        |@JsonClass(generateAdapter = true)
+        |class ExtendsJavaType(var b: Int) : JavaSuperclass()
+        |""".trimMargin())
+
+    val result = call.execute()
+    assertThat(result.exitCode).isEqualTo(ExitCode.COMPILATION_ERROR)
+    assertThat(result.systemErr)
+        .contains("supertype com.squareup.moshi.JavaSuperclass is not a Kotlin type")
+  }
+
   @Test
   fun nonFieldApplicableQualifier() {
     val call = KotlinCompilerCall(temporaryFolder.root)

kotlin-codegen/compiler/src/test/java/com/squareup/moshi/JavaSuperclass.java

@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2018 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.squareup.moshi;
+
+/** For {@link CompilerTest#extendJavaType}. */
+public class JavaSuperclass {
+  public int a = 1;
+}

kotlin-codegen/compiler/src/test/java/com/squareup/moshi/TypeResolverTest.kt

@@ -1,3 +1,18 @@
+/*
+ * Copyright (C) 2018 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package com.squareup.moshi
 
 import com.google.common.truth.Truth.assertThat
@@ -7,7 +22,6 @@ import com.squareup.kotlinpoet.asClassName
 import org.junit.Test
 
 class TypeResolverTest {
-
   private val resolver = TypeResolver()
 
   @Test
@@ -32,5 +46,4 @@ class TypeResolverTest {
 
     assertThat(resolver.resolve(nullableTypeName).nullable).isTrue()
   }
-
 }

kotlin-codegen/integration-test/src/test/kotlin/com/squareup/moshi/GeneratedAdaptersTest.kt

@@ -18,6 +18,7 @@ package com.squareup.moshi
 import org.assertj.core.api.Assertions.assertThat
 import org.intellij.lang.annotations.Language
 import org.junit.Assert.fail
+import org.junit.Ignore
 import org.junit.Test
 import java.util.Locale
 
@@ -764,6 +765,22 @@ class GeneratedAdaptersTest {
     var b: Int = -1
   }
 
+  /** Generated adapters don't track enough state to detect duplicated values. */
+  @Ignore @Test fun duplicatedValue() {
+    val moshi = Moshi.Builder().build()
+    val jsonAdapter = moshi.adapter(DuplicateValue::class.java)
+
+    try {
+      jsonAdapter.fromJson("""{"a":4,"a":4}""")
+      fail()
+    } catch(expected: JsonDataException) {
+      assertThat(expected).hasMessage("Multiple values for a at $.a")
+    }
+  }
+
+  @JsonClass(generateAdapter = true)
+  class DuplicateValue(var a: Int = -1, var b: Int = -2)
+
   @JsonQualifier
   annotation class Uppercase(val inFrench: Boolean, val onSundays: Boolean = false)
 

kotlin-codegen/integration-test/src/test/kotlin/com/squareup/moshi/KotlinCodeGenTest.kt

@@ -1,200 +0,0 @@
-/*
- * Copyright (C) 2017 Square, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package com.squareup.moshi
-
-import org.assertj.core.api.Assertions.assertThat
-import org.junit.Assert.fail
-import org.junit.Ignore
-import org.junit.Test
-import java.io.ByteArrayOutputStream
-
-class KotlinCodeGenTest {
-  @Ignore @Test fun duplicatedValue() {
-    val moshi = Moshi.Builder().build()
-    val jsonAdapter = moshi.adapter(DuplicateValue::class.java)
-
-    try {
-      jsonAdapter.fromJson("""{"a":4,"a":4}""")
-      fail()
-    } catch(expected: JsonDataException) {
-      assertThat(expected).hasMessage("Multiple values for a at $.a")
-    }
-  }
-
-  class DuplicateValue(var a: Int = -1, var b: Int = -2)
-
-  @Ignore @Test fun repeatedValue() {
-    val moshi = Moshi.Builder().build()
-    val jsonAdapter = moshi.adapter(RepeatedValue::class.java)
-
-    try {
-      jsonAdapter.fromJson("""{"a":4,"b":null,"b":6}""")
-      fail()
-    } catch(expected: JsonDataException) {
-      assertThat(expected).hasMessage("Multiple values for b at $.b")
-    }
-  }
-
-  class RepeatedValue(var a: Int, var b: Int?)
-
-  @Ignore @Test fun supertypeConstructorParameters() {
-    val moshi = Moshi.Builder().build()
-    val jsonAdapter = moshi.adapter(SubtypeConstructorParameters::class.java)
-
-    val encoded = SubtypeConstructorParameters(3, 5)
-    assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3,"b":5}""")
-
-    val decoded = jsonAdapter.fromJson("""{"a":4,"b":6}""")!!
-    assertThat(decoded.a).isEqualTo(4)
-    assertThat(decoded.b).isEqualTo(6)
-  }
-
-  open class SupertypeConstructorParameters(var a: Int)
-
-  class SubtypeConstructorParameters(a: Int, var b: Int) : SupertypeConstructorParameters(a)
-
-  @Ignore @Test fun supertypeProperties() {
-    val moshi = Moshi.Builder().build()
-    val jsonAdapter = moshi.adapter(SubtypeProperties::class.java)
-
-    val encoded = SubtypeProperties()
-    encoded.a = 3
-    encoded.b = 5
-    assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"b":5,"a":3}""")
-
-    val decoded = jsonAdapter.fromJson("""{"a":4,"b":6}""")!!
-    assertThat(decoded.a).isEqualTo(4)
-    assertThat(decoded.b).isEqualTo(6)
-  }
-
-  open class SupertypeProperties {
-    var a: Int = -1
-  }
-
-  class SubtypeProperties : SupertypeProperties() {
-    var b: Int = -1
-  }
-
-  @Ignore @Test fun extendsPlatformClassWithProtectedField() {
-    val moshi = Moshi.Builder().build()
-    val jsonAdapter = moshi.adapter(ExtendsPlatformClassWithProtectedField::class.java)
-
-    val encoded = ExtendsPlatformClassWithProtectedField(3)
-    assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3,"buf":[0,0],"count":0}""")
-
-    val decoded = jsonAdapter.fromJson("""{"a":4,"buf":[0,0],"size":0}""")!!
-    assertThat(decoded.a).isEqualTo(4)
-    assertThat(decoded.buf()).isEqualTo(ByteArray(2, { 0 }))
-    assertThat(decoded.count()).isEqualTo(0)
-  }
-
-  internal class ExtendsPlatformClassWithProtectedField(var a: Int) : ByteArrayOutputStream(2) {
-    fun buf() = buf
-    fun count() = count
-  }
-
-  @Ignore @Test fun platformTypeThrows() {
-    val moshi = Moshi.Builder().build()
-    try {
-      moshi.adapter(Triple::class.java)
-      fail()
-    } catch (e: IllegalArgumentException) {
-      assertThat(e).hasMessage("Platform class kotlin.Triple (with no annotations) "
-          + "requires explicit JsonAdapter to be registered")
-    }
-  }
-
-  @Ignore @Test fun privateConstructorParameters() {
-    val moshi = Moshi.Builder().build()
-    val jsonAdapter = moshi.adapter(PrivateConstructorParameters::class.java)
-
-    val encoded = PrivateConstructorParameters(3, 5)
-    assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3,"b":5}""")
-
-    val decoded = jsonAdapter.fromJson("""{"a":4,"b":6}""")!!
-    assertThat(decoded.a()).isEqualTo(4)
-    assertThat(decoded.b()).isEqualTo(6)
-  }
-
-  class PrivateConstructorParameters(private var a: Int, private var b: Int) {
-    fun a() = a
-    fun b() = b
-  }
-
-  @Ignore @Test fun privateConstructor() {
-    val moshi = Moshi.Builder().build()
-    val jsonAdapter = moshi.adapter(PrivateConstructor::class.java)
-
-    val encoded = PrivateConstructor.newInstance(3, 5)
-    assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3,"b":5}""")
-
-    val decoded = jsonAdapter.fromJson("""{"a":4,"b":6}""")!!
-    assertThat(decoded.a()).isEqualTo(4)
-    assertThat(decoded.b()).isEqualTo(6)
-  }
-
-  class PrivateConstructor private constructor(var a: Int, var b: Int) {
-    fun a() = a
-    fun b() = b
-    companion object {
-      fun newInstance(a: Int, b: Int) = PrivateConstructor(a, b)
-    }
-  }
-
-  @Ignore @Test fun privateProperties() {
-    val moshi = Moshi.Builder().build()
-    val jsonAdapter = moshi.adapter(PrivateProperties::class.java)
-
-    val encoded = PrivateProperties()
-    encoded.a(3)
-    encoded.b(5)
-    assertThat(jsonAdapter.toJson(encoded)).isEqualTo("""{"a":3,"b":5}""")
-
-    val decoded = jsonAdapter.fromJson("""{"a":4,"b":6}""")!!
-    assertThat(decoded.a()).isEqualTo(4)
-    assertThat(decoded.b()).isEqualTo(6)
-  }
-
-  class PrivateProperties {
-    var a: Int = -1
-    var b: Int = -1
-
-    fun a() = a
-
-    fun a(a: Int) {
-      this.a = a
-    }
-
-    fun b() = b
-
-    fun b(b: Int) {
-      this.b = b
-    }
-  }
-
-  @Ignore @Test fun kotlinEnumsAreNotCovered() {
-    val moshi = Moshi.Builder().build()
-    val adapter = moshi.adapter(UsingEnum::class.java)
-
-    assertThat(adapter.fromJson("""{"e": "A"}""")).isEqualTo(UsingEnum(KotlinEnum.A))
-  }
-
-  data class UsingEnum(val e: KotlinEnum)
-
-  enum class KotlinEnum {
-    A, B
-  }
-}

kotlin/src/test/java/com/squareup/moshi/kotlin/KotlinJsonAdapterTest.kt

@@ -523,8 +523,8 @@ class KotlinJsonAdapterTest {
   }
 
   class PrivateProperties {
-    var a: Int = -1
+    private var a: Int = -1
-    var b: Int = -1
+    private var b: Int = -1
 
     fun a() = a