From d2cbf941db07189ba952eba9fcaa143a64d16d70 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Fri, 7 May 2021 13:48:46 +0300 Subject: [PATCH] Collect all accessor checks in FirPropertyAccessorChecker --- .../declaration/FirMemberPropertiesChecker.kt | 34 ------------------- .../declaration/FirPropertyAccessorChecker.kt | 28 +++++++++++++-- 2 files changed, 25 insertions(+), 37 deletions(-) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt index f60499a3aed..b54f3ef8798 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt @@ -9,7 +9,6 @@ import org.jetbrains.kotlin.contracts.description.EventOccurrencesRange import org.jetbrains.kotlin.contracts.description.isDefinitelyVisited import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.fir.FirFakeSourceElementKind -import org.jetbrains.kotlin.fir.FirSourceElement import org.jetbrains.kotlin.fir.analysis.cfa.PropertyInitializationInfo import org.jetbrains.kotlin.fir.analysis.cfa.PropertyInitializationInfoCollector import org.jetbrains.kotlin.fir.analysis.checkers.contains @@ -21,11 +20,9 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn import org.jetbrains.kotlin.fir.analysis.diagnostics.withSuppressedDiagnostics import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertyAccessor -import org.jetbrains.kotlin.fir.expressions.FirExpression import org.jetbrains.kotlin.fir.resolve.dfa.cfg.ControlFlowGraph import org.jetbrains.kotlin.fir.resolve.dfa.cfg.NormalPath import org.jetbrains.kotlin.fir.resolve.dfa.controlFlowGraph -import org.jetbrains.kotlin.fir.symbols.impl.FirPropertyAccessorSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol import org.jetbrains.kotlin.lexer.KtTokens @@ -193,17 +190,6 @@ object FirMemberPropertiesChecker : FirRegularClassChecker() { property.delegate?.source?.let { reporter.reportOn(it, FirErrors.ABSTRACT_DELEGATED_PROPERTY, context) } - - checkAccessor(property.getter, property.delegate) { src, _, hasBody -> - if (hasBody) reporter.reportOn(src, FirErrors.ABSTRACT_PROPERTY_WITH_GETTER, context) - } - checkAccessor(property.setter, property.delegate) { src, symbol, hasBody -> - when { - symbol.fir.visibility == Visibilities.Private && property.visibility != Visibilities.Private -> - reporter.reportOn(src, FirErrors.PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY, context) - hasBody -> reporter.reportOn(src, FirErrors.ABSTRACT_PROPERTY_WITH_SETTER, context) - } - } } val hasOpenModifier = KtTokens.OPEN_KEYWORD in modifierList @@ -217,26 +203,6 @@ object FirMemberPropertiesChecker : FirRegularClassChecker() { reporter.reportOn(it, FirErrors.REDUNDANT_OPEN_IN_INTERFACE, context) } } - val isOpen = property.isOpen || hasOpenModifier - if (isOpen) { - checkAccessor(property.setter, property.delegate) { src, symbol, _ -> - if (symbol.fir.visibility == Visibilities.Private && property.visibility != Visibilities.Private) { - reporter.reportOn(src, FirErrors.PRIVATE_SETTER_FOR_OPEN_PROPERTY, context) - } - } - } - } - } - - private fun checkAccessor( - accessor: FirPropertyAccessor?, - delegate: FirExpression?, - report: (FirSourceElement, FirPropertyAccessorSymbol, hasBody: Boolean) -> Unit, - ) { - if (accessor != null && delegate == null) { - accessor.source?.let { - report.invoke(it, accessor.symbol, accessor.hasBody) - } } } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirPropertyAccessorChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirPropertyAccessorChecker.kt index 798a411f3c2..571a52fb463 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirPropertyAccessorChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirPropertyAccessorChecker.kt @@ -5,15 +5,15 @@ package org.jetbrains.kotlin.fir.analysis.checkers.declaration +import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.fir.FirFakeSourceElementKind import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.findClosestClassOrObject 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.analysis.diagnostics.withSuppressedDiagnostics -import org.jetbrains.kotlin.fir.declarations.FirProperty -import org.jetbrains.kotlin.fir.declarations.FirPropertyAccessor -import org.jetbrains.kotlin.fir.declarations.visibility +import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.types.ConeClassErrorType import org.jetbrains.kotlin.fir.types.coneType import org.jetbrains.kotlin.fir.types.isUnit @@ -33,6 +33,11 @@ object FirPropertyAccessorChecker : FirPropertyChecker() { if (getter.visibility != property.visibility) { reporter.reportOn(getter.source, FirErrors.GETTER_VISIBILITY_DIFFERS_FROM_PROPERTY_VISIBILITY, context) } + if (property.symbol.callableId.classId != null && getter.body != null && property.delegate == null) { + if (isLegallyAbstract(property, context)) { + reporter.reportOn(getter.source, FirErrors.ABSTRACT_PROPERTY_WITH_GETTER, context) + } + } val getterReturnTypeRef = getter.returnTypeRef if (getterReturnTypeRef.source?.kind is FirFakeSourceElementKind) { return @@ -63,6 +68,19 @@ object FirPropertyAccessorChecker : FirPropertyChecker() { if (visibilityCompareResult == null || visibilityCompareResult > 0) { reporter.reportOn(setter.source, FirErrors.SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY, context) } + if (property.symbol.callableId.classId != null && property.delegate == null) { + val isLegallyAbstract = isLegallyAbstract(property, context) + if (setter.visibility == Visibilities.Private && property.visibility != Visibilities.Private) { + if (isLegallyAbstract) { + reporter.reportOn(setter.source, FirErrors.PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY, context) + } else if (property.isOpen) { + reporter.reportOn(setter.source, FirErrors.PRIVATE_SETTER_FOR_OPEN_PROPERTY, context) + } + } + if (isLegallyAbstract && setter.body != null) { + reporter.reportOn(setter.source, FirErrors.ABSTRACT_PROPERTY_WITH_SETTER, context) + } + } val valueSetterParameter = setter.valueParameters.first() if (valueSetterParameter.isVararg) { @@ -105,4 +123,8 @@ object FirPropertyAccessorChecker : FirPropertyChecker() { reporter.reportOn(accessor.source, FirErrors.ACCESSOR_FOR_DELEGATED_PROPERTY, context) } } + + private fun isLegallyAbstract(property: FirProperty, context: CheckerContext): Boolean { + return property.isAbstract && context.findClosestClassOrObject().let { it is FirRegularClass && it.canHaveAbstractDeclaration } + } }