From 29dbaa4ae1c13007f28825472512bed290c6f230 Mon Sep 17 00:00:00 2001 From: Andrey Zinovyev Date: Wed, 21 Jul 2021 11:51:53 +0300 Subject: [PATCH] [FIR] Add OVERRIDE_BY_INLINE diagnostic --- .../diagnostics/FirDiagnosticsList.kt | 2 + .../fir/analysis/diagnostics/FirErrors.kt | 1 + .../FirInlineDeclarationChecker.kt | 70 +++++++++++-------- .../declaration/FirInlinePropertyChecker.kt | 14 +--- .../diagnostics/FirDefaultErrorMessages.kt | 2 + .../inline/defaultLambdaInlineDisable.fir.kt | 2 +- .../tests/inline/defaultLambdaInlining.fir.kt | 2 +- .../messagesForUnsupportedInInline.fir.kt | 4 +- .../tests/inline/overrideWithInline.fir.kt | 42 +++++------ .../inline/unsupportedConstruction.fir.kt | 2 +- .../diagnostics/KtFirDataClassConverters.kt | 6 ++ .../api/fir/diagnostics/KtFirDiagnostics.kt | 4 ++ .../fir/diagnostics/KtFirDiagnosticsImpl.kt | 7 ++ 13 files changed, 91 insertions(+), 67 deletions(-) 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 bf049c4310b..cf8832e73f1 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 @@ -1175,6 +1175,8 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") { } val DECLARATION_CANT_BE_INLINED by error(PositioningStrategy.DECLARATION_SIGNATURE) + + val OVERRIDE_BY_INLINE by warning(PositioningStrategy.DECLARATION_SIGNATURE) } val IMPORTS by object : DiagnosticGroup("Imports") { 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 bebc71dc01b..c61212001fd 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 @@ -609,6 +609,7 @@ object FirErrors { val PRIVATE_CLASS_MEMBER_FROM_INLINE by error2, FirBasedSymbol<*>>(SourceElementPositioningStrategies.REFERENCE_BY_QUALIFIED) val SUPER_CALL_FROM_PUBLIC_INLINE by warning1>(SourceElementPositioningStrategies.REFERENCE_BY_QUALIFIED) val DECLARATION_CANT_BE_INLINED by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) + val OVERRIDE_BY_INLINE by warning0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) // Imports val CANNOT_ALL_UNDER_IMPORT_FROM_SINGLETON by error1(SourceElementPositioningStrategies.IMPORT_LAST_NAME) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirInlineDeclarationChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirInlineDeclarationChecker.kt index 0bdd26ebfb8..abbd2baac83 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirInlineDeclarationChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirInlineDeclarationChecker.kt @@ -11,7 +11,6 @@ import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.fir.* import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.fir.analysis.checkers.context.PersistentCheckerContext -import org.jetbrains.kotlin.fir.analysis.checkers.context.findClosest import org.jetbrains.kotlin.fir.analysis.checkers.isInlineOnly import org.jetbrains.kotlin.fir.analysis.checkers.unsubstitutedScope import org.jetbrains.kotlin.fir.analysis.checkers.util.checkChildrenWithCustomVisitor @@ -49,9 +48,7 @@ object FirInlineDeclarationChecker : FirFunctionChecker() { val effectiveVisibility = declaration.effectiveVisibility checkInlineFunctionBody(declaration, effectiveVisibility, context, reporter) - checkParameters(declaration, context, reporter) - checkNothingToInline(declaration, context, reporter) - checkCanBeInlined(declaration, effectiveVisibility, context, reporter) + checkCallableDeclaration(declaration, context, reporter) } private fun checkInlineFunctionBody( @@ -337,9 +334,12 @@ object FirInlineDeclarationChecker : FirFunctionChecker() { } } - private fun checkParameters(function: FirFunction, context: CheckerContext, reporter: DiagnosticReporter) { - if (function !is FirSimpleFunction) return - + private fun checkParameters( + function: FirSimpleFunction, + overriddenSymbols: List>, + context: CheckerContext, + reporter: DiagnosticReporter + ) { for (param in function.valueParameters) { if (param.isNoinline) continue @@ -365,16 +365,7 @@ object FirInlineDeclarationChecker : FirFunctionChecker() { } //check for inherited default values - val classSymbol = function.containingClass()?.toSymbol(context.session) as? FirClassSymbol<*> ?: return - if (!function.isOverride) return - if (!function.valueParameters.any { it.defaultValue == null }) return - val scope = classSymbol.unsubstitutedScope(context) - - //this call is needed because AbstractFirUseSiteMemberScope collect overrides in it only, - //and not in processDirectOverriddenFunctionsWithBaseScope - scope.processFunctionsByName(function.name) { } - val overriddenMembers = scope.getDirectOverriddenMembers(function.symbol, true) - val paramsWithDefaults = overriddenMembers.flatMap { it -> + val paramsWithDefaults = overriddenSymbols.flatMap { if (it !is FirFunctionSymbol<*>) return@flatMap emptyList() it.valueParameterSymbols.mapIndexedNotNull { idx, param -> idx.takeIf { param.hasDefaultValue } @@ -392,8 +383,17 @@ object FirInlineDeclarationChecker : FirFunctionChecker() { } } - private fun checkNothingToInline(function: FirFunction, context: CheckerContext, reporter: DiagnosticReporter) { - if (function !is FirSimpleFunction) return + private fun FirCallableDeclaration.getOverriddenSymbols(context: CheckerContext): List> { + if (!this.isOverride) return emptyList() + val classSymbol = this.containingClass()?.toSymbol(context.session) as? FirClassSymbol<*> ?: return emptyList() + val scope = classSymbol.unsubstitutedScope(context) + //this call is needed because AbstractFirUseSiteMemberScope collect overrides in it only, + //and not in processDirectOverriddenFunctionsWithBaseScope + scope.processFunctionsByName(this.symbol.name) { } + return scope.getDirectOverriddenMembers(this.symbol, true) + } + + private fun checkNothingToInline(function: FirSimpleFunction, context: CheckerContext, reporter: DiagnosticReporter) { if (function.isExpect || function.isSuspend) return if (function.typeParameters.any { it.symbol.isReified }) return val hasInlinableParameters = @@ -409,20 +409,32 @@ object FirInlineDeclarationChecker : FirFunctionChecker() { } private fun checkCanBeInlined( - declaration: FirFunction, + declaration: FirCallableDeclaration, effectiveVisibility: EffectiveVisibility, context: CheckerContext, reporter: DiagnosticReporter - ) { - if (declaration !is FirSimpleFunction) return - if (declaration.containingClass() == null) return - if (effectiveVisibility == EffectiveVisibility.PrivateInClass) return - val isFinal = when (declaration) { - is FirPropertyAccessor -> context.findClosest()?.isFinal ?: declaration.isFinal - else -> declaration.isFinal - } - if (!isFinal) { + ): Boolean { + if (declaration.containingClass() == null) return true + if (effectiveVisibility == EffectiveVisibility.PrivateInClass) return true + + if (!declaration.isFinal) { reporter.reportOn(declaration.source, FirErrors.DECLARATION_CANT_BE_INLINED, context) + return false + } + return true + } + + internal fun checkCallableDeclaration(declaration: FirCallableDeclaration, context: CheckerContext, reporter: DiagnosticReporter) { + if (declaration is FirPropertyAccessor) return + val overriddenSymbols = declaration.getOverriddenSymbols(context) + if (declaration is FirSimpleFunction) { + checkParameters(declaration, overriddenSymbols, context, reporter) + checkNothingToInline(declaration, context, reporter) + } + val canBeInlined = checkCanBeInlined(declaration, declaration.effectiveVisibility, context, reporter) + + if (canBeInlined && overriddenSymbols.isNotEmpty()) { + reporter.reportOn(declaration.source, FirErrors.OVERRIDE_BY_INLINE, context) } } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirInlinePropertyChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirInlinePropertyChecker.kt index 1d96f66a2eb..95e729d67f2 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirInlinePropertyChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirInlinePropertyChecker.kt @@ -5,26 +5,16 @@ package org.jetbrains.kotlin.fir.analysis.checkers.declaration -import org.jetbrains.kotlin.descriptors.EffectiveVisibility 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.containingClass import org.jetbrains.kotlin.fir.declarations.FirProperty -import org.jetbrains.kotlin.fir.declarations.utils.effectiveVisibility -import org.jetbrains.kotlin.fir.declarations.utils.isFinal import org.jetbrains.kotlin.fir.declarations.utils.isInline object FirInlinePropertyChecker : FirPropertyChecker() { override fun check(declaration: FirProperty, context: CheckerContext, reporter: DiagnosticReporter) { - if (declaration.containingClass() == null) return - if (declaration.effectiveVisibility == EffectiveVisibility.PrivateInClass) return - - if (!declaration.isFinal && (declaration.getter?.isInline == true || declaration.setter?.isInline == true)) { - reporter.reportOn(declaration.source, FirErrors.DECLARATION_CANT_BE_INLINED, context) + if (declaration.getter?.isInline == true || declaration.setter?.isInline == true) { + FirInlineDeclarationChecker.checkCallableDeclaration(declaration, context, reporter) } - } } \ No newline at end of file diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt index b53ce5b50bd..be56bf21669 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt @@ -298,6 +298,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.OPERATOR_RENAMED_ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.OTHER_ERROR import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.OUTER_CLASS_ARGUMENTS_REQUIRED import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.OVERLOAD_RESOLUTION_AMBIGUITY +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.OVERRIDE_BY_INLINE import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.OVERRIDING_FINAL_MEMBER import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PACKAGE_CANNOT_BE_IMPORTED import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.PRIMARY_CONSTRUCTOR_DELEGATION_CALL_EXPECTED @@ -930,6 +931,7 @@ class FirDefaultErrorMessages { map.put(INLINE_CLASS_CANNOT_EXTEND_CLASSES, "Inline class cannot extend classes") map.put(INLINE_CLASS_CANNOT_BE_RECURSIVE, "Inline class cannot be recursive") map.put(DECLARATION_CANT_BE_INLINED, "'inline' modifier is not allowed on virtual members. Only private or final members can be inlined") + map.put(OVERRIDE_BY_INLINE, "Override by an inline function") map.put(RESERVED_MEMBER_INSIDE_INLINE_CLASS, "Member with the name ''{0}'' is reserved for future releases", TO_STRING) map.put( SECONDARY_CONSTRUCTOR_WITH_BODY_INSIDE_INLINE_CLASS, diff --git a/compiler/testData/diagnostics/tests/inline/defaultLambdaInlineDisable.fir.kt b/compiler/testData/diagnostics/tests/inline/defaultLambdaInlineDisable.fir.kt index 142e68f093b..31910368a43 100644 --- a/compiler/testData/diagnostics/tests/inline/defaultLambdaInlineDisable.fir.kt +++ b/compiler/testData/diagnostics/tests/inline/defaultLambdaInlineDisable.fir.kt @@ -40,7 +40,7 @@ abstract class Base { } class Derived : Base() { - override final inline fun foo(f: () -> Unit) { + override final inline fun foo(f: () -> Unit) { f() } } diff --git a/compiler/testData/diagnostics/tests/inline/defaultLambdaInlining.fir.kt b/compiler/testData/diagnostics/tests/inline/defaultLambdaInlining.fir.kt index 18ad100e6a7..f8b7bc24d99 100644 --- a/compiler/testData/diagnostics/tests/inline/defaultLambdaInlining.fir.kt +++ b/compiler/testData/diagnostics/tests/inline/defaultLambdaInlining.fir.kt @@ -39,7 +39,7 @@ abstract class Base { } class Derived : Base() { - override final inline fun foo(f: () -> Unit) { + override final inline fun foo(f: () -> Unit) { f() } } diff --git a/compiler/testData/diagnostics/tests/inline/messagesForUnsupportedInInline.fir.kt b/compiler/testData/diagnostics/tests/inline/messagesForUnsupportedInInline.fir.kt index 1e912c2ac51..e9a4333c540 100644 --- a/compiler/testData/diagnostics/tests/inline/messagesForUnsupportedInInline.fir.kt +++ b/compiler/testData/diagnostics/tests/inline/messagesForUnsupportedInInline.fir.kt @@ -12,7 +12,7 @@ abstract class Base { } class Derived : Base() { - override final inline fun withDefault( + override final inline fun withDefault( f: () -> Unit - ) {} + ) {} } diff --git a/compiler/testData/diagnostics/tests/inline/overrideWithInline.fir.kt b/compiler/testData/diagnostics/tests/inline/overrideWithInline.fir.kt index 3f1bcf0aabf..d1b83c2a22d 100644 --- a/compiler/testData/diagnostics/tests/inline/overrideWithInline.fir.kt +++ b/compiler/testData/diagnostics/tests/inline/overrideWithInline.fir.kt @@ -7,53 +7,53 @@ interface IBase { } class CDerived : IBase { - override inline final fun foo() {} - override inline fun bar() {} - override inline final fun qux(x: T) {} + override inline final fun foo() {} + override inline fun bar() {} + override inline final fun qux(x: T) {} class CNested : IBase { - override inline final fun foo() {} - override inline fun bar() {} - override inline final fun qux(x: T) {} + override inline final fun foo() {} + override inline fun bar() {} + override inline final fun qux(x: T) {} } val anObject = object : IBase { - override inline final fun foo() {} - override inline fun bar() {} - override inline final fun qux(x: T) {} + override inline final fun foo() {} + override inline fun bar() {} + override inline final fun qux(x: T) {} } fun aMethod() { class CLocal : IBase { - override inline final fun foo() {} - override inline fun bar() {} - override inline final fun qux(x: T) {} + override inline final fun foo() {} + override inline fun bar() {} + override inline final fun qux(x: T) {} } } } open class COpen : IBase { - override inline final fun foo() {} + override inline final fun foo() {} override inline fun bar() {} - override inline final fun qux(x: T) {} + override inline final fun qux(x: T) {} open class COpenNested : IBase { - override inline final fun foo() {} + override inline final fun foo() {} override inline fun bar() {} - override inline final fun qux(x: T) {} + override inline final fun qux(x: T) {} } val anObject = object : IBase { - override inline final fun foo() {} - override inline fun bar() {} - override inline final fun qux(x: T) {} + override inline final fun foo() {} + override inline fun bar() {} + override inline final fun qux(x: T) {} } fun aMethod() { open class COpenLocal : IBase { - override inline final fun foo() {} + override inline final fun foo() {} override inline fun bar() {} - override inline final fun qux(x: T) {} + override inline final fun qux(x: T) {} } } } diff --git a/compiler/testData/diagnostics/tests/inline/unsupportedConstruction.fir.kt b/compiler/testData/diagnostics/tests/inline/unsupportedConstruction.fir.kt index ca197313e3b..2f1c38d1f16 100644 --- a/compiler/testData/diagnostics/tests/inline/unsupportedConstruction.fir.kt +++ b/compiler/testData/diagnostics/tests/inline/unsupportedConstruction.fir.kt @@ -27,7 +27,7 @@ open class Base { } class Derived: Base() { - inline final override fun foo(a: Int) { + inline final override fun foo(a: 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 3ae75df17c1..6043e0cd7f3 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 @@ -3173,6 +3173,12 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirErrors.OVERRIDE_BY_INLINE) { firDiagnostic -> + OverrideByInlineImpl( + firDiagnostic as FirPsiDiagnostic, + token, + ) + } add(FirErrors.CANNOT_ALL_UNDER_IMPORT_FROM_SINGLETON) { firDiagnostic -> CannotAllUnderImportFromSingletonImpl( 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 53f0ae6f22c..ac0eabbefe3 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 @@ -2214,6 +2214,10 @@ sealed class KtFirDiagnostic : KtDiagnosticWithPsi { override val diagnosticClass get() = DeclarationCantBeInlined::class } + abstract class OverrideByInline : KtFirDiagnostic() { + override val diagnosticClass get() = OverrideByInline::class + } + abstract class CannotAllUnderImportFromSingleton : KtFirDiagnostic() { override val diagnosticClass get() = CannotAllUnderImportFromSingleton::class abstract val objectName: Name 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 190d8335c23..c42535b9769 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 @@ -3574,6 +3574,13 @@ internal class DeclarationCantBeInlinedImpl( override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) } +internal class OverrideByInlineImpl( + firDiagnostic: FirPsiDiagnostic, + override val token: ValidityToken, +) : KtFirDiagnostic.OverrideByInline(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic by weakRef(firDiagnostic) +} + internal class CannotAllUnderImportFromSingletonImpl( override val objectName: Name, firDiagnostic: FirPsiDiagnostic,