From 7d3368da5841c8ce7215bc5df02486af5e681c4e Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Tue, 12 Jul 2022 15:59:23 +0200 Subject: [PATCH] K2: report errors related to SubClassOptInRequired annotation Related to KT-41886 --- .../analysis/checkers/FirAnnotationHelpers.kt | 10 ++- .../expression/FirOptInUsageBaseChecker.kt | 87 +++++++++++++------ .../type/FirOptInUsageTypeRefChecker.kt | 9 +- .../experimental/subClassOptInRequired.fir.kt | 35 -------- .../experimental/subClassOptInRequired.kt | 1 + 5 files changed, 77 insertions(+), 65 deletions(-) delete mode 100644 compiler/testData/diagnostics/testsWithStdLib/experimental/subClassOptInRequired.fir.kt diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirAnnotationHelpers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirAnnotationHelpers.kt index 6eacf8ca7b7..78a8215a83b 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirAnnotationHelpers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirAnnotationHelpers.kt @@ -92,12 +92,16 @@ fun FirClassLikeSymbol<*>.getTargetAnnotation(): FirAnnotation? { fun FirExpression.extractClassesFromArgument(): List { return unfoldArrayOrVararg().mapNotNull { - if (it !is FirGetClassCall) return@mapNotNull null - val qualifier = it.argument as? FirResolvedQualifier ?: return@mapNotNull null - qualifier.symbol as? FirRegularClassSymbol + it.extractClassFromArgument() } } +fun FirExpression.extractClassFromArgument(): FirRegularClassSymbol? { + if (this !is FirGetClassCall) return null + val qualifier = argument as? FirResolvedQualifier ?: return null + return qualifier.symbol as? FirRegularClassSymbol +} + fun checkRepeatedAnnotation( useSiteTarget: AnnotationUseSiteTarget?, existingTargetsForAnnotation: MutableList, diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirOptInUsageBaseChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirOptInUsageBaseChecker.kt index fe7a021ce95..aa47c9c5d2f 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirOptInUsageBaseChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirOptInUsageBaseChecker.kt @@ -25,10 +25,7 @@ import org.jetbrains.kotlin.fir.scopes.processDirectlyOverriddenProperties import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol import org.jetbrains.kotlin.fir.symbols.SymbolInternals import org.jetbrains.kotlin.fir.symbols.lazyResolveToPhase -import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirNamedFunctionSymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol +import org.jetbrains.kotlin.fir.symbols.impl.* import org.jetbrains.kotlin.fir.types.* import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.Name @@ -41,7 +38,8 @@ object FirOptInUsageBaseChecker { val annotationClassId: ClassId, val severity: Severity, val message: String?, - val supertypeName: String? = null + val supertypeName: String? = null, + val fromSupertype: Boolean = false ) { enum class Severity { WARNING, ERROR } companion object { @@ -77,25 +75,34 @@ object FirOptInUsageBaseChecker { fun FirBasedSymbol<*>.loadExperimentalitiesFromAnnotationTo(session: FirSession, result: MutableCollection) { lazyResolveToPhase(FirResolvePhase.STATUS) @OptIn(SymbolInternals::class) - fir.loadExperimentalitiesFromAnnotationTo(session, result) + fir.loadExperimentalitiesFromAnnotationTo(session, result, fromSupertype = false) } private fun FirDeclaration.loadExperimentalitiesFromAnnotationTo( session: FirSession, - result: MutableCollection + result: MutableCollection, + fromSupertype: Boolean ) { for (annotation in annotations) { - val annotationType = annotation.annotationTypeRef.coneTypeSafe() + val annotationType = annotation.annotationTypeRef.coneTypeSafe() ?: continue val className = when (this) { is FirRegularClass -> name.asString() is FirCallableDeclaration -> symbol.callableId.className?.shortName()?.asString() else -> null } result.addIfNotNull( - annotationType?.lookupTag?.toFirRegularClassSymbol( + annotationType.lookupTag.toFirRegularClassSymbol( session )?.loadExperimentalityForMarkerAnnotation(className) ) + if (fromSupertype) { + if (annotationType.lookupTag.classId == OptInNames.SUBCLASS_OPT_IN_REQUIRED_CLASS_ID) { + val annotationClass = annotation.findArgumentByName(OptInNames.USE_EXPERIMENTAL_ANNOTATION_CLASS) ?: continue + result.addIfNotNull( + annotationClass.extractClassFromArgument()?.loadExperimentalityForMarkerAnnotation()?.copy(fromSupertype = true) + ) + } + } } } @@ -122,7 +129,12 @@ object FirOptInUsageBaseChecker { fun FirBasedSymbol<*>.loadExperimentalities( context: CheckerContext, fromSetter: Boolean, dispatchReceiverType: ConeKotlinType? ): Set = loadExperimentalities( - context, knownExperimentalities = null, visited = mutableSetOf(), fromSetter, dispatchReceiverType + context, knownExperimentalities = null, visited = mutableSetOf(), fromSetter, dispatchReceiverType, fromSupertype = false + ) + + fun FirClassLikeSymbol<*>.loadExperimentalitiesFromSupertype(context: CheckerContext): Set = loadExperimentalities( + context, knownExperimentalities = null, visited = mutableSetOf(), + fromSetter = false, dispatchReceiverType = null, fromSupertype = true ) @OptIn(SymbolInternals::class) @@ -132,6 +144,7 @@ object FirOptInUsageBaseChecker { visited: MutableSet, fromSetter: Boolean, dispatchReceiverType: ConeKotlinType?, + fromSupertype: Boolean, ): Set { lazyResolveToPhase(FirResolvePhase.STATUS) val fir = this.fir @@ -145,12 +158,14 @@ object FirOptInUsageBaseChecker { val parentClassScope = parentClassSymbol?.unsubstitutedScope(context) if (this is FirNamedFunctionSymbol) { parentClassScope?.processDirectlyOverriddenFunctions(this) { - it.loadExperimentalities(context, result, visited, fromSetter = false, dispatchReceiverType = null) + it.loadExperimentalities( + context, result, visited, fromSetter = false, dispatchReceiverType = null, fromSupertype = false + ) ProcessorAction.NEXT } } else if (this is FirPropertySymbol) { parentClassScope?.processDirectlyOverriddenProperties(this) { - it.loadExperimentalities(context, result, visited, fromSetter, dispatchReceiverType = null) + it.loadExperimentalities(context, result, visited, fromSetter, dispatchReceiverType = null, fromSupertype = false) ProcessorAction.NEXT } } @@ -166,19 +181,25 @@ object FirOptInUsageBaseChecker { } } if (dispatchReceiverType == null) { - parentClassSymbol?.loadExperimentalities(context, result, visited, fromSetter = false, dispatchReceiverType = null) + parentClassSymbol?.loadExperimentalities( + context, result, visited, fromSetter = false, dispatchReceiverType = null, fromSupertype = false + ) } else { dispatchReceiverType.addExperimentalities(context, result, visited) } if (fromSetter && this is FirPropertySymbol) { - setterSymbol?.loadExperimentalities(context, result, visited, fromSetter = false, dispatchReceiverType) + setterSymbol?.loadExperimentalities( + context, result, visited, fromSetter = false, dispatchReceiverType, fromSupertype = false + ) } } else if (this is FirRegularClassSymbol && fir is FirRegularClass && !fir.isLocal) { val parentClassSymbol = outerClassSymbol(context) - parentClassSymbol?.loadExperimentalities(context, result, visited, fromSetter = false, dispatchReceiverType = null) + parentClassSymbol?.loadExperimentalities( + context, result, visited, fromSetter = false, dispatchReceiverType = null, fromSupertype = false + ) } - fir.loadExperimentalitiesFromAnnotationTo(session, result) + fir.loadExperimentalitiesFromAnnotationTo(session, result, fromSupertype) if (fir is FirTypeAlias) { fir.expandedTypeRef.coneType.addExperimentalities(context, result, visited) @@ -205,7 +226,7 @@ object FirOptInUsageBaseChecker { ) { if (this !is ConeClassLikeType) return lookupTag.toSymbol(context.session)?.loadExperimentalities( - context, result, visited, fromSetter = false, dispatchReceiverType = null + context, result, visited, fromSetter = false, dispatchReceiverType = null, fromSupertype = false ) fullyExpandedType(context.session).typeArguments.forEach { if (!it.isStarProjection) it.type?.addExperimentalities(context, result, visited) @@ -230,8 +251,8 @@ object FirOptInUsageBaseChecker { context: CheckerContext, reporter: DiagnosticReporter ) { - for ((annotationClassId, severity, message) in experimentalities) { - if (!isExperimentalityAcceptableInContext(annotationClassId, context)) { + for ((annotationClassId, severity, message, _, fromSupertype) in experimentalities) { + if (!isExperimentalityAcceptableInContext(annotationClassId, context, fromSupertype)) { val (diagnostic, verb) = when (severity) { Experimentality.Severity.WARNING -> FirErrors.OPT_IN_USAGE to "should" Experimentality.Severity.ERROR -> FirErrors.OPT_IN_USAGE_ERROR to "must" @@ -252,8 +273,8 @@ object FirOptInUsageBaseChecker { reporter: DiagnosticReporter ) { for ((annotationClassId, severity, markerMessage, supertypeName) in experimentalities) { - if (!symbol.fir.isExperimentalityAcceptable(annotationClassId) && - !isExperimentalityAcceptableInContext(annotationClassId, context) + if (!symbol.fir.isExperimentalityAcceptable(annotationClassId, fromSupertype = false) && + !isExperimentalityAcceptableInContext(annotationClassId, context, fromSupertype = false) ) { val (diagnostic, verb) = when (severity) { Experimentality.Severity.WARNING -> FirErrors.OPT_IN_OVERRIDE to "should" @@ -272,7 +293,8 @@ object FirOptInUsageBaseChecker { private fun isExperimentalityAcceptableInContext( annotationClassId: ClassId, - context: CheckerContext + context: CheckerContext, + fromSupertype: Boolean ): Boolean { val languageVersionSettings = context.session.languageVersionSettings val fqNameAsString = annotationClassId.asFqNameString() @@ -280,15 +302,16 @@ object FirOptInUsageBaseChecker { return true } for (annotationContainer in context.annotationContainers) { - if (annotationContainer.isExperimentalityAcceptable(annotationClassId)) { + if (annotationContainer.isExperimentalityAcceptable(annotationClassId, fromSupertype)) { return true } } return false } - private fun FirAnnotationContainer.isExperimentalityAcceptable(annotationClassId: ClassId): Boolean { - return getAnnotationByClassId(annotationClassId) != null || isAnnotatedWithOptIn(annotationClassId) + private fun FirAnnotationContainer.isExperimentalityAcceptable(annotationClassId: ClassId, fromSupertype: Boolean): Boolean { + return getAnnotationByClassId(annotationClassId) != null || isAnnotatedWithOptIn(annotationClassId) || + fromSupertype && isAnnotatedWithSubclassOptInRequired(annotationClassId) } private fun FirAnnotationContainer.isAnnotatedWithOptIn(annotationClassId: ClassId): Boolean { @@ -305,6 +328,20 @@ object FirOptInUsageBaseChecker { return false } + private fun FirAnnotationContainer.isAnnotatedWithSubclassOptInRequired(annotationClassId: ClassId): Boolean { + for (annotation in annotations) { + val coneType = annotation.annotationTypeRef.coneType as? ConeClassLikeType + if (coneType?.lookupTag?.classId != OptInNames.SUBCLASS_OPT_IN_REQUIRED_CLASS_ID) { + continue + } + val annotationClass = annotation.findArgumentByName(OptInNames.USE_EXPERIMENTAL_ANNOTATION_CLASS) ?: continue + if (annotationClass.extractClassFromArgument()?.classId == annotationClassId) { + return true + } + } + return false + } + private val LEVEL = Name.identifier("level") private val MESSAGE = Name.identifier("message") diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/type/FirOptInUsageTypeRefChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/type/FirOptInUsageTypeRefChecker.kt index 7ea2de2a4ee..da29e70f77d 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/type/FirOptInUsageTypeRefChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/type/FirOptInUsageTypeRefChecker.kt @@ -15,6 +15,7 @@ import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.OPT_IN_CAN_ONLY_BE_USED_AS_ANNOTATION import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.OPT_IN_MARKER_CAN_ONLY_BE_USED_AS_ANNOTATION_OR_ARGUMENT_IN_OPT_IN import org.jetbrains.kotlin.diagnostics.reportOn +import org.jetbrains.kotlin.fir.declarations.FirClass import org.jetbrains.kotlin.fir.declarations.FirResolvePhase import org.jetbrains.kotlin.fir.declarations.getAnnotationByClassId import org.jetbrains.kotlin.fir.expressions.FirAnnotation @@ -55,9 +56,13 @@ object FirOptInUsageTypeRefChecker : FirTypeRefChecker() { } } + val isSupertypeRef = typeRef in (context.containingDeclarations.lastOrNull() as? FirClass)?.superTypeRefs.orEmpty() with(FirOptInUsageBaseChecker) { - val experimentalities = symbol.loadExperimentalities(context, fromSetter = false, dispatchReceiverType = null) + - loadExperimentalitiesFromConeArguments(context, coneType.typeArguments.toList()) + val classifierExperimentalities = + if (isSupertypeRef) symbol.loadExperimentalitiesFromSupertype(context) + else symbol.loadExperimentalities(context, fromSetter = false, dispatchReceiverType = null) + val experimentalities = + classifierExperimentalities + loadExperimentalitiesFromConeArguments(context, coneType.typeArguments.toList()) reportNotAcceptedExperimentalities(experimentalities, typeRef, context, reporter) } } diff --git a/compiler/testData/diagnostics/testsWithStdLib/experimental/subClassOptInRequired.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/experimental/subClassOptInRequired.fir.kt deleted file mode 100644 index 507d0664f39..00000000000 --- a/compiler/testData/diagnostics/testsWithStdLib/experimental/subClassOptInRequired.fir.kt +++ /dev/null @@ -1,35 +0,0 @@ -@RequiresOptIn -annotation class Marker - -@SubclassOptInRequired(Marker::class) -open class Base - -@Marker -class DerivedFirst : Base() - -@OptIn(Marker::class) -class DerivedSecond : Base() - -@SubclassOptInRequired(Marker::class) -open class DerivedThird : Base() - -open class DerivedFourth : Base() - -class GrandDerivedThird : DerivedThird() - -// Question: should we have an error also here? -class GrandDerivedFourth : DerivedFourth() - -@Marker -open class Marked - -@SubclassOptInRequired(Marker::class) -class DerivedMarked : Marked() - -fun test() { - val b = Base() - val d1 = DerivedFirst() - val d2 = DerivedSecond() - val d3 = DerivedThird() - val d4 = DerivedFourth() -} diff --git a/compiler/testData/diagnostics/testsWithStdLib/experimental/subClassOptInRequired.kt b/compiler/testData/diagnostics/testsWithStdLib/experimental/subClassOptInRequired.kt index 50856ce85e6..4d4638b3acf 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/experimental/subClassOptInRequired.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/experimental/subClassOptInRequired.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL @RequiresOptIn annotation class Marker