From d658e4c8b6d7f10f0562c72911519f8f4d86cac5 Mon Sep 17 00:00:00 2001 From: arpitagarwal1301 Date: Sun, 26 Jul 2026 15:34:15 +0530 Subject: [PATCH] Report illegal codegen parameter types at declaration --- CHANGELOG.md | 2 +- .../moshi/kotlin/codegen/ksp/TargetTypes.kt | 29 +++++-- .../ksp/JsonClassSymbolProcessorTest.kt | 83 +++++++++++++++++++ 3 files changed, 106 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fa227f7c..4f63ee8c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -* None yet. +* Report illegal `Unit`, `Nothing`, and `Void` codegen constructor parameters on the parameter. ## [2.0.0-alpha.1] - 2026-01-28 diff --git a/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/TargetTypes.kt b/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/TargetTypes.kt index 7880fb1aa..4cd31f2a7 100644 --- a/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/TargetTypes.kt +++ b/moshi-kotlin-codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/TargetTypes.kt @@ -38,9 +38,12 @@ import com.google.devtools.ksp.symbol.Origin import com.squareup.kotlinpoet.AnnotationSpec import com.squareup.kotlinpoet.ClassName import com.squareup.kotlinpoet.KModifier +import com.squareup.kotlinpoet.NOTHING import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy import com.squareup.kotlinpoet.PropertySpec import com.squareup.kotlinpoet.TypeName +import com.squareup.kotlinpoet.UNIT +import com.squareup.kotlinpoet.asTypeName import com.squareup.kotlinpoet.ksp.TypeParameterResolver import com.squareup.kotlinpoet.ksp.toClassName import com.squareup.kotlinpoet.ksp.toKModifier @@ -97,11 +100,7 @@ internal fun targetType( val appliedType = AppliedType(type) val constructor = - primaryConstructor(resolver, type, classTypeParamsResolver, logger) - ?: run { - logger.error("No primary constructor found on $type", type) - return null - } + primaryConstructor(resolver, type, classTypeParamsResolver, logger) ?: return null if (constructor.visibility != KModifier.INTERNAL && constructor.visibility != KModifier.PUBLIC) { logger.error( "@JsonClass can't be applied to $type: " + "primary constructor is not internal or public", @@ -184,16 +183,32 @@ internal fun primaryConstructor( typeParameterResolver: TypeParameterResolver, logger: KSPLogger, ): TargetConstructor? { - val primaryConstructor = targetType.primaryConstructor ?: return null + val primaryConstructor = + targetType.primaryConstructor + ?: run { + logger.error("No primary constructor found on $targetType", targetType) + return null + } val parameters = LinkedHashMap() for ((index, parameter) in primaryConstructor.parameters.withIndex()) { val name = parameter.name!!.getShortName() + val typeName = parameter.type.toTypeName(typeParameterResolver) + val normalizedTypeName = + typeName.unwrapTypeAlias().copy(nullable = false, annotations = emptyList()) + if ( + normalizedTypeName == UNIT || + normalizedTypeName == NOTHING || + normalizedTypeName == Void::class.asTypeName() + ) { + logger.error("Parameter $name with void, Unit, or Nothing type is illegal", parameter) + return null + } parameters[name] = TargetParameter( name = name, index = index, - type = parameter.type.toTypeName(typeParameterResolver), + type = typeName, hasDefault = parameter.hasDefault, qualifiers = parameter.qualifiers(resolver), jsonName = parameter.jsonName(), diff --git a/moshi-kotlin-codegen/src/test/java/com/squareup/moshi/kotlin/codegen/ksp/JsonClassSymbolProcessorTest.kt b/moshi-kotlin-codegen/src/test/java/com/squareup/moshi/kotlin/codegen/ksp/JsonClassSymbolProcessorTest.kt index bad93a333..05bd7ed36 100644 --- a/moshi-kotlin-codegen/src/test/java/com/squareup/moshi/kotlin/codegen/ksp/JsonClassSymbolProcessorTest.kt +++ b/moshi-kotlin-codegen/src/test/java/com/squareup/moshi/kotlin/codegen/ksp/JsonClassSymbolProcessorTest.kt @@ -570,6 +570,60 @@ class JsonClassSymbolProcessorTest { assertThat(result.messages).contains("Error preparing ElementEnvelope") } + @Test + fun nullableUnitConstructorParameter() { + assertIllegalConstructorParameter("unit", "kotlin.Unit?") + } + + @Test + fun nullableNothingConstructorParameter() { + assertIllegalConstructorParameter("nothing", "kotlin.Nothing?") + } + + @Test + fun nullableVoidConstructorParameter() { + assertIllegalConstructorParameter("void", "java.lang.Void?") + } + + @Test + fun nestedUnitConstructorParameter() { + val result = + compile( + kotlin( + "source.kt", + """ + package test + import com.squareup.moshi.JsonClass + + @JsonClass(generateAdapter = true) + data class NestedUnit(val units: List) + """, + ) + ) + assertThat(result.exitCode).isEqualTo(KotlinCompilation.ExitCode.OK) + } + + @Test + fun chainedUnitTypeAliasConstructorParameter() { + val result = + compile( + kotlin( + "source.kt", + """ + package test + import com.squareup.moshi.JsonClass + + typealias UnitAlias = kotlin.Unit? + typealias ChainedUnitAlias = UnitAlias + + @JsonClass(generateAdapter = true) + data class AliasedParameter(val aliasedUnit: ChainedUnitAlias) + """, + ) + ) + assertIllegalConstructorParameter(result, "aliasedUnit") + } + @Test fun inlineClassWithMultiplePropertiesFails() { val result = @@ -925,4 +979,33 @@ class JsonClassSymbolProcessorTest { private fun compile(vararg sourceFiles: SourceFile): JvmCompilationResult { return prepareCompilation(*sourceFiles).compile() } + + private fun assertIllegalConstructorParameter(parameterName: String, typeName: String) { + val result = + compile( + kotlin( + "source.kt", + """ + package test + import com.squareup.moshi.JsonClass + + @JsonClass(generateAdapter = true) + data class IllegalParameter(val $parameterName: $typeName) + """, + ) + ) + assertIllegalConstructorParameter(result, parameterName) + } + + private fun assertIllegalConstructorParameter( + result: JvmCompilationResult, + parameterName: String, + ) { + assertThat(result.exitCode).isEqualTo(KotlinCompilation.ExitCode.COMPILATION_ERROR) + assertThat(result.messages) + .contains("Parameter $parameterName with void, Unit, or Nothing type is illegal") + assertThat(result.messages).doesNotContain("Error preparing") + assertThat(result.messages).doesNotContain("IllegalStateException") + assertThat(result.messages).doesNotContain("No primary constructor found") + } }