From b193e708d3230b786c0f9254e444e6b413d1a787 Mon Sep 17 00:00:00 2001 From: "Denis.Zharkov" Date: Mon, 20 Dec 2021 12:55:28 +0300 Subject: [PATCH] FIR: Refactor checkUpperBoundViolated There just should be a different facades for cases A and foo() Test data has changed for type alias constructors since previously, it was working by mistake because of assumption that type alias arguments are linearly mapped to the type parameters of the corresponding constructors ^KT-50703 Open --- .../resolve/diagnostics/upperBoundViolated.kt | 2 +- .../checkers/FirUpperBoundViolatedHelpers.kt | 157 +++++++----------- .../FirUpperBoundViolatedExpressionChecker.kt | 52 ++++-- ...oundViolationInTypeAliasConstructor.fir.kt | 8 +- ...dsViolationInDeepTypeAliasExpansion.fir.kt | 16 ++ ...boundsViolationInDeepTypeAliasExpansion.kt | 1 - ...boundsViolationInTypeAliasExpansion.fir.kt | 6 +- ...erOfArgumentsInTypeAliasConstructor.fir.kt | 4 +- .../java/checkEnhancedUpperBounds.fir.kt | 6 +- ...dUpperBoundsWithEnabledImprovements.fir.kt | 6 +- 10 files changed, 131 insertions(+), 127 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.fir.kt diff --git a/compiler/fir/analysis-tests/testData/resolve/diagnostics/upperBoundViolated.kt b/compiler/fir/analysis-tests/testData/resolve/diagnostics/upperBoundViolated.kt index e855ef956fd..1b0dd99fb2c 100644 --- a/compiler/fir/analysis-tests/testData/resolve/diagnostics/upperBoundViolated.kt +++ b/compiler/fir/analysis-tests/testData/resolve/diagnostics/upperBoundViolated.kt @@ -45,7 +45,7 @@ fun rest() { class NumColl> typealias NL = NumColl> val test7 = NL()NumberPhile -val test8 = NL<String>() +val test8 = NL() class NumberPhile(x: T) val np1 = NumberPhile(10) 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 aa428d41087..bc4f46ce212 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 @@ -6,11 +6,12 @@ package org.jetbrains.kotlin.fir.analysis.checkers import org.jetbrains.kotlin.KtSourceElement -import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.diagnostics.DiagnosticReporter -import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.diagnostics.reportOn +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.fir.resolve.fullyExpandedType +import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor import org.jetbrains.kotlin.fir.resolve.substitution.substitutorByMap import org.jetbrains.kotlin.fir.resolve.toSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol @@ -20,142 +21,112 @@ import org.jetbrains.kotlin.types.AbstractTypeChecker /** * Recursively analyzes type parameters and reports the diagnostic on the given source calculated using typeRef - * Returns true if an error occurred */ fun checkUpperBoundViolated( typeRef: FirTypeRef?, context: CheckerContext, reporter: DiagnosticReporter, - typeParameters: List? = null, - typeArguments: List? = null, - typeArgumentRefsAndSources: List? = null, - isTypeAlias: Boolean = false, isIgnoreTypeParameters: Boolean = false ) { - require(typeArguments == null || typeArgumentRefsAndSources == null) { - "Only one of those arguments can be not null" - } + val notExpandedType = typeRef?.coneTypeSafe() ?: return - val type = typeRef?.coneTypeSafe() ?: return + // Everything should be reported on the typealias expansion + if (notExpandedType.typeArguments.isEmpty()) return - val typeArgumentsCount = typeArguments?.size ?: type.typeArguments.size - if (typeArgumentsCount == 0) { - return - } + val type = notExpandedType.fullyExpandedType(context.session) + val isAbbreviatedType = notExpandedType !== type - val typeParameterSymbols = typeParameters - ?: if (type is ConeClassLikeType) { - val fullyExpandedType = type.fullyExpandedType(context.session) - val prototypeClassSymbol = fullyExpandedType.lookupTag.toSymbol(context.session) as? FirRegularClassSymbol ?: return - if (type != fullyExpandedType) { - // special check for type aliases - checkUpperBoundViolated( - typeRef, - context, - reporter, - prototypeClassSymbol.typeParameterSymbols, - fullyExpandedType.typeArguments.toList(), - null, - isTypeAlias = true, - isIgnoreTypeParameters = isIgnoreTypeParameters - ) - return - } + val prototypeClassSymbol = type.lookupTag.toSymbol(context.session) as? FirRegularClassSymbol ?: return - prototypeClassSymbol.typeParameterSymbols - } else { - listOf() - } + val typeParameterSymbols = prototypeClassSymbol.typeParameterSymbols if (typeParameterSymbols.isEmpty()) { return } - val count = minOf(typeParameterSymbols.size, typeArgumentsCount) + val count = minOf(typeParameterSymbols.size, type.typeArguments.size) val substitution = mutableMapOf() for (index in 0 until count) { - val typeArgument = typeArguments?.elementAt(index) ?: type.typeArguments[index] + val typeArgument = type.typeArguments[index] val typeParameterSymbol = typeParameterSymbols[index] - if (typeArgument is FirTypeProjectionWithVariance) { - substitution[typeParameterSymbol] = typeArgument.typeRef.coneType - } else if (typeArgument is ConeKotlinType) { - substitution[typeParameterSymbol] = typeArgument.type - } else if (typeArgument is ConeTypeProjection) { - val typeArgumentType = typeArgument.type - if (typeArgumentType != null) { - substitution[typeParameterSymbol] = typeArgumentType - } else { - substitution[typeParameterSymbol] = - ConeStubTypeForTypeVariableInSubtyping(ConeTypeVariable("", typeParameterSymbol.toLookupTag()), ConeNullability.NOT_NULL) - } + val typeArgumentType = typeArgument.type + if (typeArgumentType != null) { + substitution[typeParameterSymbol] = typeArgumentType + } else { + substitution[typeParameterSymbol] = + ConeStubTypeForTypeVariableInSubtyping(ConeTypeVariable("", typeParameterSymbol.toLookupTag()), ConeNullability.NOT_NULL) } } val substitutor = substitutorByMap(substitution, context.session) + val typeArgumentsWithSourceInfo = type.typeArguments.withIndex().map { (index, projection) -> + val (argTypeRef, source) = + if (!isAbbreviatedType) + extractArgumentTypeRefAndSource(typeRef, 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, + ) +} + +class TypeArgumentWithSourceInfo( + val coneTypeProjection: ConeTypeProjection, + val typeRef: FirTypeRef?, + val source: KtSourceElement?, +) + +fun checkUpperBoundViolated( + context: CheckerContext, + reporter: DiagnosticReporter, + typeParameters: List, + arguments: List, + substitutor: ConeSubstitutor, + isAbbreviatedType: Boolean = false, + isIgnoreTypeParameters: Boolean = false +) { + val count = minOf(typeParameters.size, arguments.size) val typeSystemContext = context.session.typeContext for (index in 0 until count) { - var typeArgument: ConeKotlinType? = null - var typeArgumentTypeRef: FirTypeRef? = null - var typeArgumentSource: KtSourceElement? = null + val argument = arguments.getOrNull(index) ?: continue + val argumentType: ConeKotlinType? = argument.coneTypeProjection.type + val argumentTypeRef = argument.typeRef + val argumentSource = argument.source - if (typeArguments != null) { - val localTypeArgument = typeArguments[index] - if (localTypeArgument is FirTypeProjectionWithVariance) { - typeArgumentTypeRef = localTypeArgument.typeRef - typeArgument = typeArgumentTypeRef.coneType - typeArgumentSource = localTypeArgument.source - } else if (localTypeArgument is ConeKotlinType) { - // Typealias case - typeArgument = localTypeArgument - typeArgumentSource = typeRef.source - } - } else { - val localTypeArgument = type.typeArguments[index] - if (localTypeArgument is ConeKotlinType) { - typeArgument = localTypeArgument - } else if (localTypeArgument is ConeKotlinTypeProjection) { - typeArgument = localTypeArgument.type - } - val typeArgumentRefAndSource = typeArgumentRefsAndSources?.elementAtOrNull(index) - if (typeArgumentRefAndSource != null) { - typeArgumentTypeRef = typeArgumentRefAndSource.typeRef - typeArgumentSource = typeArgumentRefAndSource.source - } else { - val extractedTypeArgumentRefAndSource = extractArgumentTypeRefAndSource(typeRef, index) - if (extractedTypeArgumentRefAndSource != null) { - typeArgumentTypeRef = extractedTypeArgumentRefAndSource.typeRef - typeArgumentSource = extractedTypeArgumentRefAndSource.source - } - } - } - - if (typeArgument != null && typeArgumentSource != null) { - if (!isIgnoreTypeParameters || (typeArgument.typeArguments.isEmpty() && typeArgument !is ConeTypeParameterType)) { + if (argumentType != null && argumentSource != null) { + if (!isIgnoreTypeParameters || (argumentType.typeArguments.isEmpty() && argumentType !is ConeTypeParameterType)) { val intersection = - typeSystemContext.intersectTypes(typeParameterSymbols[index].resolvedBounds.map { it.coneType }) as? ConeKotlinType + typeSystemContext.intersectTypes(typeParameters[index].resolvedBounds.map { it.coneType }) as? ConeKotlinType if (intersection != null) { val upperBound = substitutor.substituteOrSelf(intersection) if (!AbstractTypeChecker.isSubtypeOf( typeSystemContext, - typeArgument.type, + argumentType, upperBound, stubTypesEqualToAnything = true ) ) { val factory = - if (isTypeAlias) FirErrors.UPPER_BOUND_VIOLATED_IN_TYPEALIAS_EXPANSION else FirErrors.UPPER_BOUND_VIOLATED - reporter.reportOn(typeArgumentSource, factory, upperBound, typeArgument.type, context) - if (isTypeAlias) { + 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 } } } } - checkUpperBoundViolated(typeArgumentTypeRef, context, reporter, isIgnoreTypeParameters = isIgnoreTypeParameters) + checkUpperBoundViolated(argumentTypeRef, context, reporter, isIgnoreTypeParameters = isIgnoreTypeParameters) } } } 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 8c92ee64672..d47f4a7267b 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 @@ -5,22 +5,23 @@ package org.jetbrains.kotlin.fir.analysis.checkers.expression -import org.jetbrains.kotlin.fir.analysis.checkers.FirTypeRefSource +import org.jetbrains.kotlin.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.fir.analysis.checkers.TypeArgumentWithSourceInfo import org.jetbrains.kotlin.fir.analysis.checkers.checkUpperBoundViolated import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext -import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.expressions.FirQualifiedAccessExpression import org.jetbrains.kotlin.fir.expressions.toResolvedCallableSymbol import org.jetbrains.kotlin.fir.references.FirErrorNamedReference import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeInapplicableWrongReceiver +import org.jetbrains.kotlin.fir.resolve.fullyExpandedType import org.jetbrains.kotlin.fir.resolve.isTypeAliasedConstructor +import org.jetbrains.kotlin.fir.resolve.substitution.substitutorByMap import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirConstructorSymbol +import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirTypeParameterSymbol -import org.jetbrains.kotlin.fir.types.FirTypeProjection -import org.jetbrains.kotlin.fir.types.FirTypeProjectionWithVariance -import org.jetbrains.kotlin.fir.types.toSymbol +import org.jetbrains.kotlin.fir.types.* object FirUpperBoundViolatedExpressionChecker : FirQualifiedAccessExpressionChecker() { override fun check(expression: FirQualifiedAccessExpression, context: CheckerContext, reporter: DiagnosticReporter) { @@ -39,29 +40,46 @@ object FirUpperBoundViolatedExpressionChecker : FirQualifiedAccessExpressionChec calleeSymbol = calleReference.candidateSymbol as? FirCallableSymbol<*> } - val typeArguments: List? - val typeArgumentRefsAndSources: List? - val typeParameters: List? + val typeArguments: List + val typeParameters: List if (calleeSymbol is FirConstructorSymbol && calleeSymbol.isTypeAliasedConstructor) { - typeArguments = null - typeArgumentRefsAndSources = expression.typeArguments.map { - FirTypeRefSource((it as? FirTypeProjectionWithVariance)?.typeRef, it.source) + val constructedType = expression.typeRef.coneType.fullyExpandedType(context.session) + typeArguments = constructedType.typeArguments.map { + TypeArgumentWithSourceInfo(it, typeRef = null, expression.source) } - typeParameters = null + + typeParameters = (constructedType.toSymbol(context.session) as? FirRegularClassSymbol)?.typeParameterSymbols ?: return } else { - typeArguments = expression.typeArguments - typeArgumentRefsAndSources = null - typeParameters = calleeSymbol?.typeParameterSymbols + typeArguments = expression.typeArguments.map { firTypeProjection -> + TypeArgumentWithSourceInfo( + firTypeProjection.toConeTypeProjection(), + (firTypeProjection as? FirTypeProjectionWithVariance)?.typeRef, + firTypeProjection.source + ) + } + typeParameters = calleeSymbol?.typeParameterSymbols ?: return } + // Neither common calls nor type alias constructor calls may contain projections + // That should be checked somewhere else + if (typeArguments.any { it.coneTypeProjection !is ConeKotlinType }) { + return + } + + if (typeArguments.size != typeParameters.size) return + + val substitutor = substitutorByMap( + typeParameters.withIndex().associate { Pair(it.value, typeArguments[it.index].coneTypeProjection as ConeKotlinType) }, + context.session, + ) + checkUpperBoundViolated( - expression.typeRef, context, reporter, typeParameters, typeArguments, - typeArgumentRefsAndSources + substitutor, ) } } diff --git a/compiler/testData/diagnostics/tests/typealias/boundViolationInTypeAliasConstructor.fir.kt b/compiler/testData/diagnostics/tests/typealias/boundViolationInTypeAliasConstructor.fir.kt index bd83204af87..a9cdc8a745e 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<String>("") -val x3 = N2<String>("") +val x2 = N("") +val x3 = N2("") class TColl> @@ -13,5 +13,5 @@ typealias TC = TColl typealias TC2 = TC val y1 = TCollAny>() -val y2 = TCAny>() -val y3 = TC2Any>() +val y2 = TC() +val y3 = TC2() diff --git a/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.fir.kt b/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.fir.kt new file mode 100644 index 00000000000..ca91b3862f6 --- /dev/null +++ b/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.fir.kt @@ -0,0 +1,16 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER + +class TColl> + +typealias TC = TColl +typealias TC2 = TC + +fun test1(x: TC2>) {} +fun test2(x: TC2>) {} +fun test3(x: TC2>) {} +fun test4(x: TC2>) {} + +val test5 = TC2>() +val test6 = TC2>() +val test7 = TC2>() +val test8 = TC2>() diff --git a/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.kt b/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.kt index c28152b7ac1..ad38c5b85db 100644 --- a/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.kt +++ b/compiler/testData/diagnostics/tests/typealias/boundsViolationInDeepTypeAliasExpansion.kt @@ -1,4 +1,3 @@ -// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER class TColl> diff --git a/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasExpansion.fir.kt b/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasExpansion.fir.kt index e2b8b801c57..1a133bfcc22 100644 --- a/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasExpansion.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/boundsViolationInTypeAliasExpansion.fir.kt @@ -15,10 +15,10 @@ fun test3(x: NL) {} fun test4(x: NL) {} val test5 = NA() -val test6 = NA<Any>() +val test6 = NA() val test7 = NL() val test8 = MMMM() -val test9dwd = NL<Any>() +val test9dwd = NL() fun test9(x: TC>) {} fun test10(x: TC>) {} @@ -28,4 +28,4 @@ fun test12(x: TC>() val test14 = TC>() val test15 = TC>() -val test16 = TCList>() +val test16 = TC>() diff --git a/compiler/testData/diagnostics/tests/typealias/wrongNumberOfArgumentsInTypeAliasConstructor.fir.kt b/compiler/testData/diagnostics/tests/typealias/wrongNumberOfArgumentsInTypeAliasConstructor.fir.kt index ace851d2438..f1c7c3defdb 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<String>("") +val testN1a = N("") 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("", "") diff --git a/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBounds.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBounds.fir.kt index 42d2ba2730c..30f23b4c488 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBounds.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBounds.fir.kt @@ -31,7 +31,7 @@ fun main1(x: A) {} fun main2(x: A2) {} fun main3(x: A3) {} fun main3() { - val x = A3<Int?>() // TODO: support reporting errors on typealias constructor calls - val x2 = A<Int?>() // TODO: support reporting errors on typealias constructor calls - val y: A3 = A3<Int?>() + val x = A3() // TODO: support reporting errors on typealias constructor calls + val x2 = A() // 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 ba79ff60f9d..4fa544d2149 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBoundsWithEnabledImprovements.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/java/checkEnhancedUpperBoundsWithEnabledImprovements.fir.kt @@ -31,7 +31,7 @@ fun main1(x: A) {} fun main2(x: A2) {} fun main3(x: A3) {} fun main3() { - val x = A3<Int?>() // TODO: support reporting errors on typealias constructor calls - val x2 = A<Int?>() - val y: A3 = A3<Int?>() + val x = A3() // TODO: support reporting errors on typealias constructor calls + val x2 = A() + val y: A3 = A3() }