From 14fe570a0085b84f411ec68471a5201903664fc1 Mon Sep 17 00:00:00 2001 From: Andrey Zinovyev Date: Sun, 11 Apr 2021 19:33:12 +0300 Subject: [PATCH] [FIR] Add more type params checks VARIANCE_ON_TYPE_PARAMETER_NOT_ALLOWED BOUND_ON_TYPE_ALIAS_PARAMETER_NOT_ALLOWED --- .../diagnostics/FirDiagnosticsList.kt | 4 ++ .../fir/analysis/diagnostics/FirErrors.kt | 2 + .../FirConflictingProjectionChecker.kt | 8 +-- .../FirTypeParameterBoundsChecker.kt | 57 ++++++++++++++----- .../FirTypeParameterVarianceChecker.kt | 30 ++++++++++ .../collectors/AbstractDiagnosticCollector.kt | 6 ++ .../fir/checkers/CommonDeclarationCheckers.kt | 1 + ...OnFunctionAndPropertyTypeParameters.fir.kt | 17 ------ ...anceOnFunctionAndPropertyTypeParameters.kt | 3 +- .../incompatibleVarianceModifiers.fir.kt | 2 +- .../typealias/boundsViolationRecursive.fir.kt | 2 +- .../typealias/parameterRestrictions.fir.kt | 4 +- .../diagnostics/KtFirDataClassConverters.kt | 12 ++++ .../api/fir/diagnostics/KtFirDiagnostics.kt | 8 +++ .../fir/diagnostics/KtFirDiagnosticsImpl.kt | 14 +++++ 15 files changed, 128 insertions(+), 42 deletions(-) create mode 100644 compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTypeParameterVarianceChecker.kt delete mode 100644 compiler/testData/diagnostics/tests/declarationChecks/VarianceOnFunctionAndPropertyTypeParameters.fir.kt diff --git a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt index dddff64f8a0..8061fa16ab8 100644 --- a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt +++ b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt @@ -318,6 +318,10 @@ object DIAGNOSTICS_LIST : DiagnosticList() { parameter("typeParametersOwner") } + val BOUND_ON_TYPE_ALIAS_PARAMETER_NOT_ALLOWED by error() + + val REIFIED_TYPE_PARAMETER_NO_INLINE by error() + val RETURN_TYPE_MISMATCH by error(PositioningStrategy.WHOLE_ELEMENT) { parameter("expected") parameter("actual") diff --git a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt index 5cc33fb4b19..12f975a2449 100644 --- a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt +++ b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt @@ -235,6 +235,8 @@ object FirErrors { val REPEATED_BOUND by error0() val CONFLICTING_UPPER_BOUNDS by error1() val NAME_IN_CONSTRAINT_IS_NOT_A_TYPE_PARAMETER by error2() + val BOUND_ON_TYPE_ALIAS_PARAMETER_NOT_ALLOWED by error0() + val REIFIED_TYPE_PARAMETER_NO_INLINE by error0() val RETURN_TYPE_MISMATCH by error2(SourceElementPositioningStrategies.WHOLE_ELEMENT) // Reflection diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirConflictingProjectionChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirConflictingProjectionChecker.kt index 92c716e42ed..297aefb5738 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirConflictingProjectionChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirConflictingProjectionChecker.kt @@ -36,14 +36,8 @@ object FirConflictingProjectionChecker : FirBasicDeclarationChecker() { checkTypeRef(it, context, reporter) } } - is FirTypeAlias -> { - for (it in declaration.typeParameters) { - if (it.variance != Variance.INVARIANT) { - reporter.reportOn(it.source, FirErrors.VARIANCE_ON_TYPE_PARAMETER_NOT_ALLOWED, context) - } - } + is FirTypeAlias -> checkTypeRef(declaration.expandedTypeRef, context, reporter) - } } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTypeParameterBoundsChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTypeParameterBoundsChecker.kt index b489be2f633..5637946990d 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTypeParameterBoundsChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTypeParameterBoundsChecker.kt @@ -7,6 +7,7 @@ package org.jetbrains.kotlin.fir.analysis.checkers.declaration import org.jetbrains.kotlin.KtNodeTypes import org.jetbrains.kotlin.descriptors.ClassKind +import org.jetbrains.kotlin.fir.FirRealSourceElementKind import org.jetbrains.kotlin.fir.analysis.checkers.* import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter @@ -28,19 +29,11 @@ object FirTypeParameterBoundsChecker : FirTypeParameterChecker() { ) override fun check(declaration: FirTypeParameter, context: CheckerContext, reporter: DiagnosticReporter) { - val containingDeclaration = context.containingDeclarations.lastOrNull() + val containingDeclaration = context.containingDeclarations.lastOrNull() ?: return if (containingDeclaration is FirConstructor) return - if (containingDeclaration is FirSimpleFunction && containingDeclaration.isOverride) return - if (containingDeclaration is FirProperty && containingDeclaration.isOverride) return - declaration.symbol.fir.bounds.forEach { bound -> - if (!bound.coneType.canHaveSubtypes(context.session)) { - reporter.reportOn(bound.source, FirErrors.FINAL_UPPER_BOUND, bound.coneType, context) - } - if (bound.isExtensionFunctionType(context.session)) { - reporter.reportOn(bound.source, FirErrors.UPPER_BOUND_IS_EXTENSION_FUNCTION_TYPE, context) - } - } + checkFinalUpperBounds(declaration, containingDeclaration, context, reporter) + checkExtensionFunctionTypeBound(declaration, context, reporter) if (containingDeclaration.safeAs()?.isInlineOnly() != true) { checkOnlyOneTypeParameterBound(declaration, context, reporter) @@ -48,6 +41,44 @@ object FirTypeParameterBoundsChecker : FirTypeParameterChecker() { checkBoundUniqueness(declaration, context, reporter) checkConflictingBounds(declaration, context, reporter) + checkTypeAliasBound(declaration, containingDeclaration, context, reporter) + } + + private fun checkFinalUpperBounds( + declaration: FirTypeParameter, + containingDeclaration: FirDeclaration, + context: CheckerContext, + reporter: DiagnosticReporter + ) { + if (containingDeclaration is FirSimpleFunction && containingDeclaration.isOverride) return + if (containingDeclaration is FirProperty && containingDeclaration.isOverride) return + + declaration.symbol.fir.bounds.forEach { bound -> + if (!bound.coneType.canHaveSubtypes(context.session)) { + reporter.reportOn(bound.source, FirErrors.FINAL_UPPER_BOUND, bound.coneType, context) + } + } + } + + private fun checkExtensionFunctionTypeBound(declaration: FirTypeParameter, context: CheckerContext, reporter: DiagnosticReporter) { + declaration.symbol.fir.bounds.forEach { bound -> + if (bound.isExtensionFunctionType(context.session)) { + reporter.reportOn(bound.source, FirErrors.UPPER_BOUND_IS_EXTENSION_FUNCTION_TYPE, context) + } + } + } + + private fun checkTypeAliasBound( + declaration: FirTypeParameter, + containingDeclaration: FirDeclaration, + context: CheckerContext, + reporter: DiagnosticReporter + ) { + if (containingDeclaration is FirTypeAlias) { + declaration.bounds.filter { it.source?.kind == FirRealSourceElementKind }.forEach { bound -> + reporter.reportOn(bound.source, FirErrors.BOUND_ON_TYPE_ALIAS_PARAMETER_NOT_ALLOWED, context) + } + } } private fun checkOnlyOneTypeParameterBound(declaration: FirTypeParameter, context: CheckerContext, reporter: DiagnosticReporter) { @@ -81,7 +112,7 @@ object FirTypeParameterBoundsChecker : FirTypeParameterChecker() { val seenClasses = mutableSetOf() val allNonErrorBounds = declaration.bounds.filter { it !is FirErrorTypeRef } val uniqueBounds = allNonErrorBounds.distinctBy { it.coneType.classId ?: it.coneType } - + uniqueBounds.forEach { bound -> bound.coneType.toRegularClass(context.session)?.let { clazz -> if (classKinds.contains(clazz.classKind) && seenClasses.add(clazz) && seenClasses.size > 1) { @@ -102,7 +133,7 @@ object FirTypeParameterBoundsChecker : FirTypeParameterChecker() { types.forEach { type -> if (!type.canHaveSubtypes(context.session)) { types.forEach { otherType -> - if (type != otherType && !type.isRelated(context.session.typeContext, otherType)){ + if (type != otherType && !type.isRelated(context.session.typeContext, otherType)) { return true } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTypeParameterVarianceChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTypeParameterVarianceChecker.kt new file mode 100644 index 00000000000..6ce53da0168 --- /dev/null +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTypeParameterVarianceChecker.kt @@ -0,0 +1,30 @@ +/* + * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.fir.analysis.checkers.declaration + +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors +import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn +import org.jetbrains.kotlin.fir.declarations.FirProperty +import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction +import org.jetbrains.kotlin.fir.declarations.FirTypeAlias +import org.jetbrains.kotlin.fir.declarations.FirTypeParameter +import org.jetbrains.kotlin.types.Variance + +object FirTypeParameterVarianceChecker : FirTypeParameterChecker() { + + override fun check(declaration: FirTypeParameter, context: CheckerContext, reporter: DiagnosticReporter) { + val containingDeclaration = context.containingDeclarations.lastOrNull() + if (declaration.variance != Variance.INVARIANT && + (containingDeclaration is FirSimpleFunction || + containingDeclaration is FirTypeAlias || + containingDeclaration is FirProperty) + ) { + reporter.reportOn(declaration.source, FirErrors.VARIANCE_ON_TYPE_PARAMETER_NOT_ALLOWED, context) + } + } +} diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/AbstractDiagnosticCollector.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/AbstractDiagnosticCollector.kt index a41137baad7..2d26cb9f1ce 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/AbstractDiagnosticCollector.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/AbstractDiagnosticCollector.kt @@ -92,6 +92,12 @@ abstract class AbstractDiagnosticCollector( } } + override fun visitTypeAlias(typeAlias: FirTypeAlias, data: Nothing?) { + withSuppressedDiagnostics(typeAlias) { + visitWithDeclaration(typeAlias) + } + } + private fun visitJump(loopJump: FirLoopJump) { withSuppressedDiagnostics(loopJump) { loopJump.runComponents() diff --git a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt index ff763d26c1f..395452accda 100644 --- a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt +++ b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt @@ -93,5 +93,6 @@ object CommonDeclarationCheckers : DeclarationCheckers() { override val typeParameterCheckers: Set = setOf( FirTypeParameterBoundsChecker, + FirTypeParameterVarianceChecker, ) } diff --git a/compiler/testData/diagnostics/tests/declarationChecks/VarianceOnFunctionAndPropertyTypeParameters.fir.kt b/compiler/testData/diagnostics/tests/declarationChecks/VarianceOnFunctionAndPropertyTypeParameters.fir.kt deleted file mode 100644 index 504ca188548..00000000000 --- a/compiler/testData/diagnostics/tests/declarationChecks/VarianceOnFunctionAndPropertyTypeParameters.fir.kt +++ /dev/null @@ -1,17 +0,0 @@ -fun f() { - -} - -fun g() { - -} - -fun h() { - -} - -val T.x: Int - get() = 1 - -val T.y: Int - get() = 1 \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/declarationChecks/VarianceOnFunctionAndPropertyTypeParameters.kt b/compiler/testData/diagnostics/tests/declarationChecks/VarianceOnFunctionAndPropertyTypeParameters.kt index aab935656fc..a3dfb02d39a 100644 --- a/compiler/testData/diagnostics/tests/declarationChecks/VarianceOnFunctionAndPropertyTypeParameters.kt +++ b/compiler/testData/diagnostics/tests/declarationChecks/VarianceOnFunctionAndPropertyTypeParameters.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL fun <in T> f() { } @@ -14,4 +15,4 @@ val <out T> T.x: Int get() = 1 val <in T> T.y: Int - get() = 1 \ No newline at end of file + get() = 1 diff --git a/compiler/testData/diagnostics/tests/modifiers/incompatibleVarianceModifiers.fir.kt b/compiler/testData/diagnostics/tests/modifiers/incompatibleVarianceModifiers.fir.kt index f28ff1dc4f4..c2cd96f8f12 100644 --- a/compiler/testData/diagnostics/tests/modifiers/incompatibleVarianceModifiers.fir.kt +++ b/compiler/testData/diagnostics/tests/modifiers/incompatibleVarianceModifiers.fir.kt @@ -14,7 +14,7 @@ fun test3() { } class A { - fun out T> bar() { + fun <out out T> bar() { } } diff --git a/compiler/testData/diagnostics/tests/typealias/boundsViolationRecursive.fir.kt b/compiler/testData/diagnostics/tests/typealias/boundsViolationRecursive.fir.kt index c2efd713d78..a7862a63f48 100644 --- a/compiler/testData/diagnostics/tests/typealias/boundsViolationRecursive.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/boundsViolationRecursive.fir.kt @@ -1,3 +1,3 @@ // !DIAGNOSTICS: -UNUSED_VARIABLE -UNUSED_PARAMETER -typealias R> = List +typealias RList> = List diff --git a/compiler/testData/diagnostics/tests/typealias/parameterRestrictions.fir.kt b/compiler/testData/diagnostics/tests/typealias/parameterRestrictions.fir.kt index 09ef7d29798..433da70e5ae 100644 --- a/compiler/testData/diagnostics/tests/typealias/parameterRestrictions.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/parameterRestrictions.fir.kt @@ -1,8 +1,8 @@ // !DIAGNOSTICS: -UNUSED_TYPEALIAS_PARAMETER typealias WithVariance<in X, out Y> = Int -typealias WithBounds1 = Int -typealias WithBounds2 = Int +typealias WithBounds1T> = Int +typealias WithBounds2Y, Y : X> = Int typealias WithBounds3 where X : Any = Int diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt index c3aa379c620..21fd72c76da 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt @@ -986,6 +986,18 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirErrors.BOUND_ON_TYPE_ALIAS_PARAMETER_NOT_ALLOWED) { firDiagnostic -> + BoundOnTypeAliasParameterNotAllowedImpl( + firDiagnostic as FirPsiDiagnostic<*>, + token, + ) + } + add(FirErrors.REIFIED_TYPE_PARAMETER_NO_INLINE) { firDiagnostic -> + ReifiedTypeParameterNoInlineImpl( + firDiagnostic as FirPsiDiagnostic<*>, + token, + ) + } add(FirErrors.RETURN_TYPE_MISMATCH) { firDiagnostic -> ReturnTypeMismatchImpl( firSymbolBuilder.typeBuilder.buildKtType(firDiagnostic.a), diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt index 18381637feb..28c7b0f8779 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt @@ -701,6 +701,14 @@ sealed class KtFirDiagnostic : KtDiagnosticWithPsi { abstract val typeParametersOwner: KtSymbol } + abstract class BoundOnTypeAliasParameterNotAllowed : KtFirDiagnostic() { + override val diagnosticClass get() = BoundOnTypeAliasParameterNotAllowed::class + } + + abstract class ReifiedTypeParameterNoInline : KtFirDiagnostic() { + override val diagnosticClass get() = ReifiedTypeParameterNoInline::class + } + abstract class ReturnTypeMismatch : KtFirDiagnostic() { override val diagnosticClass get() = ReturnTypeMismatch::class abstract val expected: KtType diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt index 06a45e19801..514d7059c1c 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt @@ -1131,6 +1131,20 @@ internal class NameInConstraintIsNotATypeParameterImpl( override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic) } +internal class BoundOnTypeAliasParameterNotAllowedImpl( + firDiagnostic: FirPsiDiagnostic<*>, + override val token: ValidityToken, +) : KtFirDiagnostic.BoundOnTypeAliasParameterNotAllowed(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic) +} + +internal class ReifiedTypeParameterNoInlineImpl( + firDiagnostic: FirPsiDiagnostic<*>, + override val token: ValidityToken, +) : KtFirDiagnostic.ReifiedTypeParameterNoInline(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic) +} + internal class ReturnTypeMismatchImpl( override val expected: KtType, override val actual: KtType,