Collect all accessor checks in FirPropertyAccessorChecker

This commit is contained in:
Mikhail Glukhikh
2021-05-07 13:48:46 +03:00
parent 243114ef29
commit d2cbf941db
2 changed files with 25 additions and 37 deletions
@@ -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)
}
}
}
}
@@ -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 }
}
}