From b2230327df4bbe399ffa02cfab56c2186459149d Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Fri, 19 May 2023 13:46:47 -0500 Subject: [PATCH] [FIR] Refactor upper bound validation with better source element info Attach source information to each argument of the type before expanding to preserve information during validation. This allows errors to be reported on the original argument during nested type alias expansion. ^KT-50798 Fixed ^KT-50703 Fixed --- .../checkers/FirUpperBoundViolatedHelpers.kt | 124 ++++++++++++------ .../FirUpperBoundViolatedExpressionChecker.kt | 36 +++-- .../typealias/abbreviatedSupertypes.fir.kt | 20 +-- .../abbreviatedSupertypesErrors.fir.kt | 20 +-- ...oundViolationInTypeAliasConstructor.fir.kt | 8 +- ...dsViolationInDeepTypeAliasExpansion.fir.kt | 4 +- ...boundsViolationInTypeAliasExpansion.fir.kt | 8 +- .../boundsViolationInTypeAliasRHS.fir.kt | 8 +- .../tests/typealias/kt19601.fir.kt | 11 ++ .../diagnostics/tests/typealias/kt19601.kt | 1 - ...erOfArgumentsInTypeAliasConstructor.fir.kt | 4 +- .../java/checkEnhancedUpperBounds.fir.kt | 6 +- ...dUpperBoundsWithEnabledImprovements.fir.kt | 6 +- 13 files changed, 163 insertions(+), 93 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/typealias/kt19601.fir.kt diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirUpperBoundViolatedHelpers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirUpperBoundViolatedHelpers.kt index 9c29402d0cb..267a7e624cc 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirUpperBoundViolatedHelpers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirUpperBoundViolatedHelpers.kt @@ -32,13 +32,28 @@ fun checkUpperBoundViolated( reporter: DiagnosticReporter, isIgnoreTypeParameters: Boolean = false ) { - val notExpandedType = typeRef?.coneTypeSafe() ?: return + val type = typeRef?.coneTypeSafe() ?: return + checkUpperBoundViolated(typeRef, type, context, reporter, isIgnoreTypeParameters) +} - // Everything should be reported on the typealias expansion +private fun checkUpperBoundViolated( + typeRef: FirTypeRef?, + notExpandedType: ConeClassLikeType, + context: CheckerContext, + reporter: DiagnosticReporter, + isIgnoreTypeParameters: Boolean = false, +) { if (notExpandedType.typeArguments.isEmpty()) return - val type = notExpandedType.fullyExpandedType(context.session) - val isAbbreviatedType = notExpandedType !== type + // If we have FirTypeRef information, add KtSourceElement information to each argument of the type and fully expand. + val type = if (typeRef != null) { + notExpandedType.fullyExpandedTypeWithSource(typeRef, context.session) + // Add fallback source information to arguments of the expanded type. + ?.withArguments { it.withSource(FirTypeRefSource(null, typeRef.source)) } + ?: return + } else { + notExpandedType + } val prototypeClassSymbol = type.lookupTag.toSymbol(context.session) as? FirRegularClassSymbol ?: return @@ -51,29 +66,15 @@ fun checkUpperBoundViolated( val substitution = typeParameterSymbols.zip(type.typeArguments).toMap() val substitutor = FE10LikeConeSubstitutor(substitution, context.session) - val typeRefAndSourcesForArguments = extractArgumentsTypeRefAndSource(typeRef) ?: return - val typeArgumentsWithSourceInfo = type.typeArguments.withIndex().map { (index, projection) -> - val (argTypeRef, source) = - if (!isAbbreviatedType) { - typeRefAndSourcesForArguments.getOrNull(index) ?: return - } else { - // For abbreviated arguments we use the whole typeRef as a place to report - FirTypeRefSource(null, typeRef.source) - } - - TypeArgumentWithSourceInfo(projection, argTypeRef, source) - } - return checkUpperBoundViolated( - context, reporter, typeParameterSymbols, typeArgumentsWithSourceInfo, substitutor, - isAbbreviatedType, - isIgnoreTypeParameters, + context, reporter, typeParameterSymbols, type.typeArguments.toList(), substitutor, + isReportExpansionError = true, isIgnoreTypeParameters, ) } private class FE10LikeConeSubstitutor( private val substitution: Map, - private val useSiteSession: FirSession + useSiteSession: FirSession ) : AbstractConeSubstitutor(useSiteSession.typeContext) { override fun substituteType(type: ConeKotlinType): ConeKotlinType? { if (type !is ConeTypeParameterType) return null @@ -135,29 +136,24 @@ private class OriginalProjectionTypeAttribute(val data: ConeTypeProjection) : Co private val ConeAttributes.originalProjection: OriginalProjectionTypeAttribute? by ConeAttributes.attributeAccessor() -class TypeArgumentWithSourceInfo( - val coneTypeProjection: ConeTypeProjection, - val typeRef: FirTypeRef?, - val source: KtSourceElement?, -) - fun checkUpperBoundViolated( context: CheckerContext, reporter: DiagnosticReporter, typeParameters: List, - arguments: List, + typeArguments: List, substitutor: ConeSubstitutor, - isAbbreviatedType: Boolean = false, - isIgnoreTypeParameters: Boolean = false + isReportExpansionError: Boolean = false, + isIgnoreTypeParameters: Boolean = false, ) { - val count = minOf(typeParameters.size, arguments.size) + val count = minOf(typeParameters.size, typeArguments.size) val typeSystemContext = context.session.typeContext for (index in 0 until count) { - val argument = arguments.getOrNull(index) ?: continue - val argumentType: ConeKotlinType? = argument.coneTypeProjection.type - val argumentTypeRef = argument.typeRef - val argumentSource = argument.source + val argument = typeArguments[index] + val argumentType = argument.type + val sourceAttribute = argumentType?.attributes?.sourceAttribute + val argumentTypeRef = sourceAttribute?.typeRef + val argumentSource = sourceAttribute?.source if (argumentType != null && argumentSource != null) { if (!isIgnoreTypeParameters || (argumentType.typeArguments.isEmpty() && argumentType !is ConeTypeParameterType)) { @@ -172,17 +168,63 @@ fun checkUpperBoundViolated( stubTypesEqualToAnything = true ) ) { - val factory = - if (isAbbreviatedType) FirErrors.UPPER_BOUND_VIOLATED_IN_TYPEALIAS_EXPANSION else FirErrors.UPPER_BOUND_VIOLATED - reporter.reportOn(argumentSource, factory, upperBound, argumentType.type, context) - if (isAbbreviatedType) { - return + val factory = when { + isReportExpansionError && argumentTypeRef == null -> FirErrors.UPPER_BOUND_VIOLATED_IN_TYPEALIAS_EXPANSION + else -> FirErrors.UPPER_BOUND_VIOLATED } + reporter.reportOn(argumentSource, factory, upperBound, argumentType.type, context) } } } - checkUpperBoundViolated(argumentTypeRef, context, reporter, isIgnoreTypeParameters = isIgnoreTypeParameters) + if (argumentType is ConeClassLikeType) { + checkUpperBoundViolated(argumentTypeRef, argumentType, context, reporter, isIgnoreTypeParameters) + } + } + } +} + +fun ConeClassLikeType.fullyExpandedTypeWithSource( + typeRef: FirTypeRef, + useSiteSession: FirSession, +): ConeClassLikeType? { + val typeRefAndSourcesForArguments = extractArgumentsTypeRefAndSource(typeRef) ?: return null + // Avoid issues with nested type aliases and context receivers on function types as source information isn't returned. + if (typeRefAndSourcesForArguments.size != typeArguments.size) return null + + // Add source information to arguments of non-expanded type, which is preserved during expansion. + val typeArguments = + typeArguments.zip(typeRefAndSourcesForArguments) { projection, source -> projection.withSource(source) } + .toTypedArray() + + return withArguments(typeArguments).fullyExpandedType(useSiteSession) +} + +private class SourceAttribute(private val data: FirTypeRefSource) : ConeAttribute() { + val source: KtSourceElement? get() = data.source + val typeRef: FirTypeRef? get() = data.typeRef + + override fun union(other: SourceAttribute?): SourceAttribute = other ?: this + override fun intersect(other: SourceAttribute?): SourceAttribute = other ?: this + override fun add(other: SourceAttribute?): SourceAttribute = other ?: this + + override fun isSubtypeOf(other: SourceAttribute?): Boolean = true + + override fun toString() = "SourceAttribute: $data" + + override val key: KClass + get() = SourceAttribute::class +} + +private val ConeAttributes.sourceAttribute: SourceAttribute? by ConeAttributes.attributeAccessor() + +fun ConeTypeProjection.withSource(source: FirTypeRefSource?): ConeTypeProjection { + return when { + source == null || this !is ConeKotlinTypeProjection -> this + else -> { + // Prefer existing source information. + val attributes = ConeAttributes.create(listOf(SourceAttribute(source))).add(type.attributes) + replaceType(type.withAttributes(attributes)) } } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirUpperBoundViolatedExpressionChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirUpperBoundViolatedExpressionChecker.kt index 0357a91e84f..31a7b2ed5d6 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirUpperBoundViolatedExpressionChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirUpperBoundViolatedExpressionChecker.kt @@ -6,9 +6,10 @@ package org.jetbrains.kotlin.fir.analysis.checkers.expression import org.jetbrains.kotlin.diagnostics.DiagnosticReporter -import org.jetbrains.kotlin.fir.analysis.checkers.TypeArgumentWithSourceInfo +import org.jetbrains.kotlin.fir.analysis.checkers.FirTypeRefSource import org.jetbrains.kotlin.fir.analysis.checkers.checkUpperBoundViolated import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.withSource import org.jetbrains.kotlin.fir.expressions.FirQualifiedAccessExpression import org.jetbrains.kotlin.fir.references.toResolvedCallableSymbol import org.jetbrains.kotlin.fir.references.FirResolvedErrorReference @@ -39,22 +40,23 @@ object FirUpperBoundViolatedExpressionChecker : FirQualifiedAccessExpressionChec else -> null } - val typeArguments: List + val typeArguments: List val typeParameters: List if (calleeSymbol is FirConstructorSymbol && calleeSymbol.isTypeAliasedConstructor) { val constructedType = expression.typeRef.coneType.fullyExpandedType(context.session) + // Updating arguments with source information after expanding the type seems extremely brittle as it relies on identity equality + // of the expression type arguments and the expanded type arguments. This cannot be applied before expanding the type because it + // seems like the type is already expended. typeArguments = constructedType.typeArguments.map { - TypeArgumentWithSourceInfo(it, typeRef = null, expression.source) + it.withSourceRecursive(expression) } typeParameters = (constructedType.toSymbol(context.session) as? FirRegularClassSymbol)?.typeParameterSymbols ?: return } else { typeArguments = expression.typeArguments.map { firTypeProjection -> - TypeArgumentWithSourceInfo( - firTypeProjection.toConeTypeProjection(), - (firTypeProjection as? FirTypeProjectionWithVariance)?.typeRef, - firTypeProjection.source + firTypeProjection.toConeTypeProjection().withSource( + FirTypeRefSource((firTypeProjection as? FirTypeProjectionWithVariance)?.typeRef, firTypeProjection.source) ) } typeParameters = calleeSymbol?.typeParameterSymbols ?: return @@ -62,14 +64,14 @@ object FirUpperBoundViolatedExpressionChecker : FirQualifiedAccessExpressionChec // Neither common calls nor type alias constructor calls may contain projections // That should be checked somewhere else - if (typeArguments.any { it.coneTypeProjection !is ConeKotlinType }) { + if (typeArguments.any { it !is ConeKotlinType }) { return } if (typeArguments.size != typeParameters.size) return val substitutor = substitutorByMap( - typeParameters.withIndex().associate { Pair(it.value, typeArguments[it.index].coneTypeProjection as ConeKotlinType) }, + typeParameters.withIndex().associate { Pair(it.value, typeArguments[it.index] as ConeKotlinType) }, context.session, ) @@ -81,4 +83,20 @@ object FirUpperBoundViolatedExpressionChecker : FirQualifiedAccessExpressionChec substitutor, ) } + + private fun ConeTypeProjection.withSourceRecursive(expression: FirQualifiedAccessExpression): ConeTypeProjection { + // Recursively apply source to any arguments of this type. + val type = when { + this is ConeClassLikeType && typeArguments.isNotEmpty() -> withArguments { it.withSourceRecursive(expression) } + else -> this + } + + // Try to match the expanded type arguments back to the original expression type arguments. + return when (val argument = expression.typeArguments.find { it.toConeTypeProjection() === this }) { + // Unable to find a matching argument, fall back to marking the entire expression. + null -> type.withSource(FirTypeRefSource(null, expression.source)) + // Found the original argument! + else -> type.withSource(FirTypeRefSource((argument as? FirTypeProjectionWithVariance)?.typeRef, argument.source)) + } + } } diff --git a/compiler/testData/diagnostics/tests/typealias/abbreviatedSupertypes.fir.kt b/compiler/testData/diagnostics/tests/typealias/abbreviatedSupertypes.fir.kt index e0be30b9a98..50f09db1e87 100644 --- a/compiler/testData/diagnostics/tests/typealias/abbreviatedSupertypes.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/abbreviatedSupertypes.fir.kt @@ -9,19 +9,19 @@ typealias OneList = List> typealias Both = TK typealias BothList = List> -object O1 : One() // compiler error expected -object O2 : Both() +object O1 : One<Any>() // compiler error expected +object O2 : Both<Any, Any>() -class A1One> -class A2One> +class A1Any>> +class A2Any, Any>> -interface IO1 : OneList {} -interface IO2 : BothList {} +interface IO1 : OneList<Any> {} +interface IO2 : BothList<Any, Any> {} -fun foo1(x: One) {} -fun foo2(x: Both) {} +fun foo1(x: One<Any>) {} +fun foo2(x: Both<Any, Any>) {} fun main() { - One() - Both() + One<Any>() + Both<Any, Any>() } diff --git a/compiler/testData/diagnostics/tests/typealias/abbreviatedSupertypesErrors.fir.kt b/compiler/testData/diagnostics/tests/typealias/abbreviatedSupertypesErrors.fir.kt index bfc63fb9aab..1e99963b329 100644 --- a/compiler/testData/diagnostics/tests/typealias/abbreviatedSupertypesErrors.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/abbreviatedSupertypesErrors.fir.kt @@ -9,19 +9,19 @@ typealias OneList = List> typealias Both = TK typealias BothList = List> -object O1 : One() // compiler error expected -object O2 : Both() +object O1 : One<Any>() // compiler error expected +object O2 : Both<Any, Any>() -class A1One> -class A2One> +class A1Any>> +class A2Any, Any>> -interface IO1 : OneList {} -interface IO2 : BothList {} +interface IO1 : OneList<Any> {} +interface IO2 : BothList<Any, Any> {} -fun foo1(x: One) {} -fun foo2(x: Both) {} +fun foo1(x: One<Any>) {} +fun foo2(x: Both<Any, Any>) {} fun main() { - One() - Both() + One<Any>() + Both<Any, Any>() } diff --git a/compiler/testData/diagnostics/tests/typealias/boundViolationInTypeAliasConstructor.fir.kt b/compiler/testData/diagnostics/tests/typealias/boundViolationInTypeAliasConstructor.fir.kt index a9cdc8a745e..bd83204af87 100644 --- a/compiler/testData/diagnostics/tests/typealias/boundViolationInTypeAliasConstructor.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/boundViolationInTypeAliasConstructor.fir.kt @@ -4,8 +4,8 @@ typealias N = Num typealias N2 = N val x1 = Num<String>("") -val x2 = N("") -val x3 = N2("") +val x2 = N<String>("") +val x3 = N2<String>("") class TColl> @@ -13,5 +13,5 @@ typealias TC = TColl typealias TC2 = TC val y1 = TCollAny>() -val y2 = TC() -val y3 = TC2() +val y2 = TCAny>() +val y3 = TC2Any>() diff --git a/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.fir.kt b/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.fir.kt index ca91b3862f6..1bc616dd766 100644 --- a/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.fir.kt @@ -8,9 +8,9 @@ typealias TC2 = TC fun test1(x: TC2>) {} fun test2(x: TC2>) {} fun test3(x: TC2>) {} -fun test4(x: TC2>) {} +fun test4(x: TC2List>) {} val test5 = TC2>() val test6 = TC2>() val test7 = TC2>() -val test8 = TC2>() +val test8 = TC2List>() diff --git a/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasExpansion.fir.kt b/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasExpansion.fir.kt index 1a133bfcc22..896c1643037 100644 --- a/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasExpansion.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasExpansion.fir.kt @@ -10,12 +10,12 @@ typealias MMMM = NL typealias TC = TColl fun test1(x: NA) {} -fun test2(x: NA) {} +fun test2(x: NA<Any>) {} fun test3(x: NL) {} fun test4(x: NL) {} val test5 = NA() -val test6 = NA() +val test6 = NA<Any>() val test7 = NL() val test8 = MMMM() val test9dwd = NL() @@ -23,9 +23,9 @@ val test9dwd = NL() fun test9(x: TC>) {} fun test10(x: TC>) {} fun test11(x: TC>) {} -fun test12(x: TC>) {} +fun test12(x: TCList>) {} val test13 = TC>() val test14 = TC>() val test15 = TC>() -val test16 = TC>() +val test16 = TCList>() diff --git a/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasRHS.fir.kt b/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasRHS.fir.kt index 09485f45348..0e2ab9e610b 100644 --- a/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasRHS.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasRHS.fir.kt @@ -5,16 +5,16 @@ class TC> typealias TCAlias = TC typealias TCAliasT = TCAny> typealias TCAliasC = TC -typealias TCAliasT1 = TCAlias +typealias TCAliasT1 = TCAliasAny> typealias TCAliasC1 = TCAlias typealias Test1 = TCAny> typealias Test2 = TC> -typealias Test3 = TCAlias +typealias Test3 = TCAliasAny> typealias Test4 = TCAlias> typealias Test5 = TCAliasT -typealias Test6 = TCAliasC +typealias Test6 = TCAliasC<Any> typealias Test7 = TCAliasC> typealias Test8 = TCAliasT1 -typealias Test9 = TCAliasC1 +typealias Test9 = TCAliasC1<Any> typealias Test10 = TCAliasC1> diff --git a/compiler/testData/diagnostics/tests/typealias/kt19601.fir.kt b/compiler/testData/diagnostics/tests/typealias/kt19601.fir.kt new file mode 100644 index 00000000000..640bc53e7a1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/typealias/kt19601.fir.kt @@ -0,0 +1,11 @@ +interface Order + +typealias Ord = Order + +class Test1> + +interface Num + +typealias N = Num + +class Test2String>> diff --git a/compiler/testData/diagnostics/tests/typealias/kt19601.kt b/compiler/testData/diagnostics/tests/typealias/kt19601.kt index 559b11a851b..7d01bb10612 100644 --- a/compiler/testData/diagnostics/tests/typealias/kt19601.kt +++ b/compiler/testData/diagnostics/tests/typealias/kt19601.kt @@ -1,4 +1,3 @@ -// FIR_IDENTICAL interface Order typealias Ord = Order diff --git a/compiler/testData/diagnostics/tests/typealias/wrongNumberOfArgumentsInTypeAliasConstructor.fir.kt b/compiler/testData/diagnostics/tests/typealias/wrongNumberOfArgumentsInTypeAliasConstructor.fir.kt index 70be9cf1a44..9681f60f27c 100644 --- a/compiler/testData/diagnostics/tests/typealias/wrongNumberOfArgumentsInTypeAliasConstructor.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/wrongNumberOfArgumentsInTypeAliasConstructor.fir.kt @@ -30,7 +30,7 @@ typealias N = Num val testN0 = N("") val testN1 = N(1) -val testN1a = N("") +val testN1a = N<String>("") val testN2 = N(1) class MyPair(val string: T1, val number: T2) @@ -38,4 +38,4 @@ typealias MP = MyPair val testMP0 = MP("", 1) val testMP1 = MP(1, "") -val testMP2 = MP("", "") +val testMP2 = MP<String>("", "") diff --git a/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBounds.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBounds.fir.kt index 30f23b4c488..e7638e4f5cc 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBounds.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBounds.fir.kt @@ -27,11 +27,11 @@ typealias A = MapLike typealias A2 = Foo> typealias A3 = ListLike> -fun main1(x: A) {} -fun main2(x: A2) {} +fun main1(x: A<Int?>) {} +fun main2(x: A2<Int?>) {} fun main3(x: A3) {} fun main3() { val x = A3() // TODO: support reporting errors on typealias constructor calls - val x2 = A() // TODO: support reporting errors on typealias constructor calls + val x2 = A<Int?>() // TODO: support reporting errors on typealias constructor calls val y: A3 = A3() } diff --git a/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBoundsWithEnabledImprovements.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBoundsWithEnabledImprovements.fir.kt index b5f17242f8f..a3823c735e8 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBoundsWithEnabledImprovements.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBoundsWithEnabledImprovements.fir.kt @@ -27,11 +27,11 @@ typealias A = MapLike typealias A2 = Foo> typealias A3 = ListLike> -fun main1(x: A) {} -fun main2(x: A2) {} +fun main1(x: A<Int?>) {} +fun main2(x: A2<Int?>) {} fun main3(x: A3) {} fun main3() { val x = A3() - val x2 = A() + val x2 = A<Int?>() val y: A3 = A3() }