Provide more hints for error types.

Allow KSErrorType to receive a message as additional piece of information on why a valid type can't be provided. Cover `replace`/`asType` cases with a utility function `errorTypeOnInconsistentArguments` to get consistent error messages across implementations. Tweak KSTypeImpl.isError detection in AA, move TODO there. (cherry picked from commit 319ddff)
google · Jun 3, 2024 · 82871d8 · 82871d8
1 parent c77e170
commit 82871d8
Show file tree

Hide file tree

Showing 19 changed files with 207 additions and 190 deletions.
diff --git a/common-util/src/main/kotlin/com/google/devtools/ksp/common/ErrorTypeUtils.kt b/common-util/src/main/kotlin/com/google/devtools/ksp/common/ErrorTypeUtils.kt
@@ -0,0 +1,24 @@
+package com.google.devtools.ksp.common
+
+import com.google.devtools.ksp.symbol.KSType
+import com.google.devtools.ksp.symbol.KSTypeArgument
+
+inline fun <E> errorTypeOnInconsistentArguments(
+    arguments: List<KSTypeArgument>,
+    placeholdersProvider: () -> List<KSTypeArgument>,
+    withCorrectedArguments: (corrected: List<KSTypeArgument>) -> KSType,
+    errorType: (name: String, message: String) -> E,
+): E? {
+    if (arguments.isNotEmpty()) {
+        val placeholders = placeholdersProvider()
+        val diff = arguments.size - placeholders.size
+        if (diff > 0) {
+            val wouldBeType = withCorrectedArguments(arguments.dropLast(diff))
+            return errorType(wouldBeType.toString(), "Unexpected extra $diff type argument(s)")
+        } else if (diff < 0) {
+            val wouldBeType = withCorrectedArguments(arguments + placeholders.drop(arguments.size))
+            return errorType(wouldBeType.toString(), "Missing ${-diff} type argument(s)")
+        }
+    }
+    return null
+}
diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/processing/impl/ResolverImpl.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/processing/impl/ResolverImpl.kt
@@ -1248,7 +1248,7 @@ class ResolverImpl(
     }
 
     // Convert type arguments for Java wildcard, recursively.
-    private fun KotlinType.toWildcard(mode: TypeMappingMode): KotlinType {
+    private fun KotlinType.toWildcard(mode: TypeMappingMode): Result<KotlinType> {
         val parameters = constructor.parameters
         val arguments = arguments
 
@@ -1258,10 +1258,11 @@ class ResolverImpl(
                 parameter.variance != org.jetbrains.kotlin.types.Variance.INVARIANT &&
                 argument.projectionKind != org.jetbrains.kotlin.types.Variance.INVARIANT
             ) {
-                // conflicting variances
-                throw IllegalArgumentException(
-                    "Conflicting variance: variance '${parameter.variance.label}' vs projection " +
-                        "'${argument.projectionKind.label}'"
+                return Result.failure(
+                    IllegalArgumentException(
+                        "Conflicting variance: variance '${parameter.variance.label}' vs projection " +
+                            "'${argument.projectionKind.label}'"
+                    )
                 )
             }
 
@@ -1270,10 +1271,10 @@ class ResolverImpl(
             val genericMode = argMode.toGenericArgumentMode(
                 getEffectiveVariance(parameter.variance, argument.projectionKind)
             )
-            TypeProjectionImpl(variance, argument.type.toWildcard(genericMode))
+            TypeProjectionImpl(variance, argument.type.toWildcard(genericMode).getOrElse { return Result.failure(it) })
         }
 
-        return replace(wildcardArguments)
+        return Result.success(replace(wildcardArguments))
     }
 
     private val JVM_SUPPRESS_WILDCARDS_NAME = KSNameImpl.getCached("kotlin.jvm.JvmSuppressWildcards")
@@ -1372,12 +1373,19 @@ class ResolverImpl(
             if (position == RefPosition.SUPER_TYPE &&
                 argument.projectionKind != org.jetbrains.kotlin.types.Variance.INVARIANT
             ) {
-                throw IllegalArgumentException("Type projection isn't allowed in immediate arguments to supertypes")
+                val errorType = KSErrorType(
+                    name = type.toString(),
+                    message = "Type projection isn't allowed in immediate arguments to supertypes"
+                )
+                return KSTypeReferenceSyntheticImpl.getCached(errorType, null)
             }
         }
 
         val wildcardType = kotlinType.toWildcard(typeMappingMode).let {
-            var candidate: KotlinType = it
+            var candidate: KotlinType = it.getOrElse { error ->
+                val errorType = KSErrorType(name = type.toString(), message = error.message)
+                return KSTypeReferenceSyntheticImpl.getCached(errorType, null)
+            }
             for (i in indexes.reversed()) {
                 candidate = candidate.arguments[i].type
             }

diff --git a/...ain/kotlin/com/google/devtools/ksp/symbol/impl/binary/KSClassDeclarationDescriptorImpl.kt b/...ain/kotlin/com/google/devtools/ksp/symbol/impl/binary/KSClassDeclarationDescriptorImpl.kt
@@ -151,9 +151,7 @@ class KSClassDeclarationDescriptorImpl private constructor(val descriptor: Class
     }
 
     override fun asType(typeArguments: List<KSTypeArgument>): KSType =
-        descriptor.defaultType.replaceTypeArguments(typeArguments)?.let {
-            getKSTypeCached(it, typeArguments)
-        } ?: KSErrorType()
+        descriptor.defaultType.replaceTypeArguments(typeArguments)
 
     override fun asStarProjectedType(): KSType {
         return getKSTypeCached(descriptor.defaultType.replaceArgumentsWithStarProjections())

diff --git a/...in/kotlin/com/google/devtools/ksp/symbol/impl/java/KSClassDeclarationJavaEnumEntryImpl.kt b/...in/kotlin/com/google/devtools/ksp/symbol/impl/java/KSClassDeclarationJavaEnumEntryImpl.kt
@@ -17,6 +17,7 @@
 
 package com.google.devtools.ksp.symbol.impl.java
 
+import com.google.devtools.ksp.common.errorTypeOnInconsistentArguments
 import com.google.devtools.ksp.common.impl.KSNameImpl
 import com.google.devtools.ksp.common.toKSModifiers
 import com.google.devtools.ksp.processing.impl.KSObjectCache
@@ -96,8 +97,10 @@ class KSClassDeclarationJavaEnumEntryImpl private constructor(val psi: PsiEnumCo
 
     // Enum can't have type parameters.
     override fun asType(typeArguments: List<KSTypeArgument>): KSType {
-        if (typeArguments.isNotEmpty())
-            return KSErrorType()
+        errorTypeOnInconsistentArguments(
+            arguments = typeArguments, placeholdersProvider = ::emptyList,
+            withCorrectedArguments = ::asType, errorType = ::KSErrorType,
+        )?.let { error -> return error }
         return asStarProjectedType()
     }
 

diff --git a/...in/src/main/kotlin/com/google/devtools/ksp/symbol/impl/java/KSClassDeclarationJavaImpl.kt b/...in/src/main/kotlin/com/google/devtools/ksp/symbol/impl/java/KSClassDeclarationJavaImpl.kt
@@ -153,17 +153,13 @@ class KSClassDeclarationJavaImpl private constructor(val psi: PsiClass) :
     }
 
     override fun asType(typeArguments: List<KSTypeArgument>): KSType {
-        return descriptor?.let {
-            it.defaultType.replaceTypeArguments(typeArguments)?.let {
-                getKSTypeCached(it, typeArguments)
-            }
-        } ?: KSErrorType()
+        return descriptor?.defaultType?.replaceTypeArguments(typeArguments) ?: KSErrorType(psi.qualifiedName)
     }
 
     override fun asStarProjectedType(): KSType {
         return descriptor?.let {
             getKSTypeCached(it.defaultType.replaceArgumentsWithStarProjections())
-        } ?: KSErrorType()
+        } ?: KSErrorType(psi.qualifiedName)
     }
 
     override fun <D, R> accept(visitor: KSVisitor<D, R>, data: D): R {

diff --git a/...ugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSClassDeclarationImpl.kt b/...ugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSClassDeclarationImpl.kt
@@ -122,9 +122,7 @@ class KSClassDeclarationImpl private constructor(val ktClassOrObject: KtClassOrO
     }
 
     override fun asType(typeArguments: List<KSTypeArgument>): KSType {
-        return descriptor.defaultType.replaceTypeArguments(typeArguments)?.let {
-            getKSTypeCached(it, typeArguments)
-        } ?: KSErrorType()
+        return descriptor.defaultType.replaceTypeArguments(typeArguments)
     }
 
     override fun asStarProjectedType(): KSType {

diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSErrorType.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSErrorType.kt
@@ -25,8 +25,12 @@ import org.jetbrains.kotlin.types.error.ErrorType
 import org.jetbrains.kotlin.types.error.ErrorTypeKind
 
 class KSErrorType(
-    val nameHint: String? = null,
+    val nameHint: String?,
 ) : KSType {
+    constructor(name: String, message: String?) : this(
+        nameHint = listOfNotNull(name, message).takeIf { it.isNotEmpty() }?.joinToString(" % ")
+    )
+
     override val annotations: Sequence<KSAnnotation>
         get() = emptySequence()
 

diff --git a/...n/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSPropertyDeclarationImpl.kt b/...n/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSPropertyDeclarationImpl.kt
@@ -97,7 +97,7 @@ class KSPropertyDeclarationImpl private constructor(val ktProperty: KtProperty)
             KSTypeReferenceDeferredImpl.getCached(this) {
                 val desc = propertyDescriptor as? VariableDescriptorWithAccessors
                 if (desc == null) {
-                    KSErrorType()
+                    KSErrorType(null /* no info available */)
                 } else {
                     getKSTypeCached(desc.type)
                 }

diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSTypeImpl.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSTypeImpl.kt
@@ -95,9 +95,7 @@ class KSTypeImpl private constructor(
     }
 
     override fun replace(arguments: List<KSTypeArgument>): KSType {
-        return kotlinType.replaceTypeArguments(arguments)?.let {
-            getKSTypeCached(it, arguments, annotations)
-        } ?: KSErrorType()
+        return kotlinType.replaceTypeArguments(arguments, annotations)
     }
 
     override fun starProjection(): KSType {

diff --git a/...plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSValueParameterImpl.kt b/...plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSValueParameterImpl.kt
@@ -112,7 +112,8 @@ class KSValueParameterImpl private constructor(val ktParameter: KtParameter) : K
 
     override val type: KSTypeReference by lazy {
         ktParameter.typeReference?.let { KSTypeReferenceImpl.getCached(it) }
-            ?: findPropertyForAccessor()?.type ?: KSTypeReferenceSyntheticImpl.getCached(KSErrorType(), this)
+            ?: findPropertyForAccessor()?.type
+            ?: KSTypeReferenceSyntheticImpl.getCached(KSErrorType(null /* no info available */), this)
     }
 
     override val hasDefault: Boolean = ktParameter.hasDefaultValue()

diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/utils.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/utils.kt
@@ -18,6 +18,7 @@ package com.google.devtools.ksp.symbol.impl
 
 import com.google.devtools.ksp.ExceptionMessage
 import com.google.devtools.ksp.KspExperimental
+import com.google.devtools.ksp.common.errorTypeOnInconsistentArguments
 import com.google.devtools.ksp.common.impl.KSNameImpl
 import com.google.devtools.ksp.processing.impl.ResolverImpl
 import com.google.devtools.ksp.symbol.*
@@ -136,10 +137,18 @@ fun org.jetbrains.kotlin.types.Variance.toKSVariance(): Variance {
 
 private fun KSTypeReference.toKotlinType() = (resolve() as? KSTypeImpl)?.kotlinType
 
-// returns null if error
-internal fun KotlinType.replaceTypeArguments(newArguments: List<KSTypeArgument>): KotlinType? {
-    if (newArguments.isNotEmpty() && this.arguments.size != newArguments.size)
-        return null
+// returns KSErrorType if error
+internal fun KotlinType.replaceTypeArguments(
+    newArguments: List<KSTypeArgument>,
+    annotations: Sequence<KSAnnotation> = emptySequence(),
+): KSType {
+    errorTypeOnInconsistentArguments(
+        arguments = newArguments,
+        placeholdersProvider = { arguments.map { KSTypeArgumentDescriptorImpl.getCached(it, Origin.SYNTHETIC, null) } },
+        withCorrectedArguments = ::replaceTypeArguments,
+        errorType = ::KSErrorType,
+    )?.let { error -> return error }
+
     return replace(
         newArguments.mapIndexed { index, ksTypeArgument ->
             val variance = when (ksTypeArgument.variance) {
@@ -155,11 +164,11 @@ internal fun KotlinType.replaceTypeArguments(newArguments: List<KSTypeArgument>)
                 else -> throw IllegalStateException(
                     "Unexpected psi for type argument: ${ksTypeArgument.javaClass}, $ExceptionMessage"
                 )
-            }.toKotlinType() ?: return null
+            }.let { it.toKotlinType() ?: return KSErrorType.fromReferenceBestEffort(it) }
 
             TypeProjectionImpl(variance, type)
         }
-    )
+    ).let { KSTypeImpl.getCached(it, newArguments, annotations) }
 }
 
 internal fun FunctionDescriptor.toKSDeclaration(): KSDeclaration {

diff --git a/...-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSClassDeclarationImpl.kt b/...-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSClassDeclarationImpl.kt
@@ -18,6 +18,7 @@
 package com.google.devtools.ksp.impl.symbol.kotlin
 
 import com.google.devtools.ksp.common.KSObjectCache
+import com.google.devtools.ksp.common.errorTypeOnInconsistentArguments
 import com.google.devtools.ksp.common.impl.KSNameImpl
 import com.google.devtools.ksp.common.impl.KSTypeReferenceSyntheticImpl
 import com.google.devtools.ksp.impl.ResolverAAImpl
@@ -124,9 +125,12 @@ class KSClassDeclarationImpl private constructor(internal val ktClassOrObjectSym
     }
 
     override fun asType(typeArguments: List<KSTypeArgument>): KSType {
-        if (typeArguments.isNotEmpty() && typeArguments.size != asStarProjectedType().arguments.size) {
-            return KSErrorType()
-        }
+        errorTypeOnInconsistentArguments(
+            arguments = typeArguments,
+            placeholdersProvider = { asStarProjectedType().arguments },
+            withCorrectedArguments = ::asType,
+            errorType = ::KSErrorType,
+        )?.let { error -> return error }
         return analyze {
             if (typeArguments.isEmpty()) {
                 typeParameters.map { buildTypeParameterType((it as KSTypeParameterImpl).ktTypeParameterSymbol) }

diff --git a/...in-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSErrorType.kt b/...in-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSErrorType.kt
@@ -19,8 +19,12 @@ package com.google.devtools.ksp.impl.symbol.kotlin
 import com.google.devtools.ksp.symbol.*
 
 class KSErrorType(
-    private val hint: String? = null,
+    private val hint: String?,
 ) : KSType {
+    constructor(name: String, message: String?) : this(
+        hint = listOfNotNull(name, message).takeIf { it.isNotEmpty() }?.joinToString(" % ")
+    )
+
     override val declaration: KSDeclaration
         get() = KSErrorTypeClassDeclaration(this)
 

diff --git a/...src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSErrorTypeClassDeclaration.kt b/...src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSErrorTypeClassDeclaration.kt
@@ -114,7 +114,7 @@ class KSErrorTypeClassDeclaration(
         return this === other || other is KSErrorTypeClassDeclaration && other.type == type
     }
 
-    override fun hashCode(): Int = type.hashCode()
+    override fun hashCode(): Int = type.hashCode() * 2
 
     override val docString
         get() = null

diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSTypeImpl.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSTypeImpl.kt
@@ -19,6 +19,7 @@ package com.google.devtools.ksp.impl.symbol.kotlin
 
 import com.google.devtools.ksp.common.IdKeyPair
 import com.google.devtools.ksp.common.KSObjectCache
+import com.google.devtools.ksp.common.errorTypeOnInconsistentArguments
 import com.google.devtools.ksp.impl.ResolverAAImpl
 import com.google.devtools.ksp.impl.recordLookupWithSupertypes
 import com.google.devtools.ksp.impl.symbol.kotlin.resolved.KSTypeArgumentResolvedImpl
@@ -110,12 +111,17 @@ class KSTypeImpl private constructor(internal val type: KtType) : KSType {
     }
 
     override fun replace(arguments: List<KSTypeArgument>): KSType {
-        return type.replace(arguments.map { it.toKtTypeProjection() })?.let { getCached(it) } ?: KSErrorType()
+        errorTypeOnInconsistentArguments(
+            arguments = arguments,
+            placeholdersProvider = { type.typeArguments().map { KSTypeArgumentResolvedImpl.getCached(it) } },
+            withCorrectedArguments = ::replace,
+            errorType = ::KSErrorType,
+        )?.let { error -> return error }
+        return getCached(type.replace(arguments.map { it.toKtTypeProjection() }))
     }
 
     override fun starProjection(): KSType {
-        return type.replace(List(type.typeArguments().size) { KtStarTypeProjection(type.token) })
-            ?.let { getCached(it) } ?: KSErrorType()
+        return getCached(type.replace(List(type.typeArguments().size) { KtStarTypeProjection(type.token) }))
     }
 
     override fun makeNullable(): KSType {
@@ -134,7 +140,8 @@ class KSTypeImpl private constructor(internal val type: KtType) : KSType {
         get() = type.nullability == KtTypeNullability.NULLABLE
 
     override val isError: Boolean
-        get() = type is KtErrorType
+        // TODO: non exist type returns KtNonErrorClassType, check upstream for KtClassErrorType usage.
+        get() = type is KtErrorType || type.classifierSymbol() == null
 
     override val isFunctionType: Boolean
         get() = type is KtFunctionalType && !type.isSuspend

diff --git a/...kotlin/com/google/devtools/ksp/impl/symbol/kotlin/resolved/KSTypeReferenceResolvedImpl.kt b/...kotlin/com/google/devtools/ksp/impl/symbol/kotlin/resolved/KSTypeReferenceResolvedImpl.kt
@@ -22,14 +22,11 @@ import com.google.devtools.ksp.common.KSObjectCache
 import com.google.devtools.ksp.impl.recordLookup
 import com.google.devtools.ksp.impl.symbol.kotlin.Deferrable
 import com.google.devtools.ksp.impl.symbol.kotlin.KSClassDeclarationImpl
-import com.google.devtools.ksp.impl.symbol.kotlin.KSErrorType
 import com.google.devtools.ksp.impl.symbol.kotlin.KSTypeImpl
 import com.google.devtools.ksp.impl.symbol.kotlin.KSTypeParameterImpl
 import com.google.devtools.ksp.impl.symbol.kotlin.Restorable
 import com.google.devtools.ksp.impl.symbol.kotlin.analyze
 import com.google.devtools.ksp.impl.symbol.kotlin.annotations
-import com.google.devtools.ksp.impl.symbol.kotlin.classifierSymbol
-import com.google.devtools.ksp.impl.symbol.kotlin.getNameHint
 import com.google.devtools.ksp.impl.symbol.kotlin.render
 import com.google.devtools.ksp.impl.symbol.kotlin.toClassifierReference
 import com.google.devtools.ksp.impl.symbol.kotlin.toLocation
@@ -61,22 +58,7 @@ class KSTypeReferenceResolvedImpl private constructor(
 
     override fun resolve(): KSType {
         analyze { recordLookup(ktType, parent) }
-        // TODO: non exist type returns KtNonErrorClassType, check upstream for KtClassErrorType usage.
-        return if (
-            analyze {
-                ktType is KtClassErrorType || (ktType.classifierSymbol() == null)
-            }
-        ) {
-            KSErrorType(
-                when (ktType) {
-                    is KtClassErrorType -> ktType.getNameHint()
-                    is KtTypeErrorType -> null // No info available
-                    else -> ktType.render()
-                }
-            )
-        } else {
-            KSTypeImpl.getCached(ktType)
-        }
+        return KSTypeImpl.getCached(ktType)
     }
 
     override val annotations: Sequence<KSAnnotation> by lazy {