FIR checker: introduce top-level property checkers

Also, refactor property initializer checking so that those
newly added property checkers as well as member property checker
can share the same logic
This commit is contained in:
Jinseong Jeon
2021-01-20 00:10:23 -08:00
committed by Mikhail Glukhikh
parent f9378a3ab7
commit 57c8dd86a0
11 changed files with 151 additions and 47 deletions
@@ -10,10 +10,10 @@ import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.checkers.extended.report
import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
import org.jetbrains.kotlin.fir.declarations.FirRegularClass
import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction
import org.jetbrains.kotlin.fir.declarations.hasBody
import org.jetbrains.kotlin.fir.declarations.isExpect
import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertyAccessor
import org.jetbrains.kotlin.fir.types.FirImplicitTypeRef
import org.jetbrains.kotlin.lexer.KtTokens
// Note that the class that contains the currently visiting declaration will *not* be in the context's containing declarations *yet*.
internal fun isInsideExpectClass(containingDeclaration: FirRegularClass, context: CheckerContext): Boolean =
@@ -26,3 +26,89 @@ internal fun checkExpectFunction(function: FirSimpleFunction, reporter: Diagnost
reporter.report(source, FirErrors.EXPECTED_DECLARATION_WITH_BODY)
}
}
fun checkPropertyInitializer(
containingClass: FirRegularClass?,
property: FirProperty,
reporter: DiagnosticReporter
) {
val inInterface = containingClass?.isInterface == true
// If multiple (potentially conflicting) modality modifiers are specified, not all modifiers are recorded at `status`.
// So, our source of truth should be the full modifier list retrieved from the source.
val modifierList = with(FirModifierList) { property.source.getModifierList() }
val hasAbstractModifier = modifierList?.modifiers?.any { it.token == KtTokens.ABSTRACT_KEYWORD } == true
val isAbstract = property.isAbstract || hasAbstractModifier
if (isAbstract) {
if (property.initializer == null && property.delegate == null && property.returnTypeRef is FirImplicitTypeRef) {
property.source?.let {
reporter.report(it, FirErrors.PROPERTY_WITH_NO_TYPE_NO_INITIALIZER)
}
}
return
}
// TODO: not exactly...
val backingFieldRequired = property.hasBackingField
if (inInterface && backingFieldRequired && property.hasAccessorImplementation) {
property.source?.let {
// reporter.report(it, FirErrors.BACKING_FIELD_IN_INTERFACE)
}
}
val isExpect = property.isExpect || modifierList?.modifiers?.any { it.token == KtTokens.EXPECT_KEYWORD } == true
when {
property.initializer != null -> {
property.initializer?.source?.let {
when {
inInterface -> {
reporter.report(it, FirErrors.PROPERTY_INITIALIZER_IN_INTERFACE)
}
isExpect -> {
reporter.report(it, FirErrors.EXPECTED_PROPERTY_INITIALIZER)
}
!backingFieldRequired -> {
// reporter.report(it, FirErrors.PROPERTY_INITIALIZER_NO_BACKING_FIELD)
}
property.receiverTypeRef != null -> {
// reporter.report(it, FirErrors.EXTENSION_PROPERTY_WITH_BACKING_FIELD)
}
}
}
}
property.delegate != null -> {
property.delegate?.source?.let {
when {
inInterface -> {
reporter.report(it, FirErrors.DELEGATED_PROPERTY_IN_INTERFACE)
}
isExpect -> {
reporter.report(it, FirErrors.EXPECTED_DELEGATED_PROPERTY)
}
}
}
}
else -> {
val isExternal = property.isExternal || modifierList?.modifiers?.any { it.token == KtTokens.EXTERNAL_KEYWORD } == true
// TODO: need to analyze class anonymous initializer to see if the property is initialized there.
val isUninitialized = false
if (backingFieldRequired && !inInterface && !property.isLateInit && !isExpect && isUninitialized && !isExternal) {
property.source?.let {
if (property.receiverTypeRef != null && !property.hasAccessorImplementation) {
// reporter.report(it, FirErrors.EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT)
} else {
if (containingClass != null || property.hasAccessorImplementation) {
// reporter.report(it, FirErrors.MUST_BE_INITIALIZED)
} else {
// reporter.report(it, FirErrors.MUST_BE_INITIALIZED_OR_BE_ABSTRACT)
}
}
}
}
}
}
}
private val FirProperty.hasAccessorImplementation: Boolean
get() = (getter !is FirDefaultPropertyAccessor && getter?.hasBody == true) ||
(setter !is FirDefaultPropertyAccessor && setter?.hasBody == true)
@@ -6,6 +6,7 @@
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.FirSourceElement
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.checkers.extended.report
@@ -15,7 +16,6 @@ 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.symbols.impl.FirPropertyAccessorSymbol
import org.jetbrains.kotlin.fir.types.FirImplicitTypeRef
import org.jetbrains.kotlin.lexer.KtTokens
// See old FE's [DeclarationsChecker]
@@ -34,6 +34,8 @@ object FirMemberPropertyChecker : FirRegularClassChecker() {
context: CheckerContext,
reporter: DiagnosticReporter
) {
val source = property.source ?: return
if (source.kind is FirFakeSourceElementKind) return
// If multiple (potentially conflicting) modality modifiers are specified, not all modifiers are recorded at `status`.
// So, our source of truth should be the full modifier list retrieved from the source.
val modifierList = with(FirModifierList) { property.source.getModifierList() }
@@ -56,15 +58,11 @@ object FirMemberPropertyChecker : FirRegularClassChecker() {
return
}
}
if (property.delegate != null) {
property.delegate!!.source?.let {
if (containingDeclaration.isInterface) {
reporter.report(it, FirErrors.DELEGATED_PROPERTY_IN_INTERFACE)
} else {
reporter.report(it, FirErrors.ABSTRACT_DELEGATED_PROPERTY)
}
}
property.initializer?.source?.let {
reporter.report(it, FirErrors.ABSTRACT_PROPERTY_WITH_INITIALIZER)
}
property.delegate?.source?.let {
reporter.report(it, FirErrors.ABSTRACT_DELEGATED_PROPERTY)
}
checkAccessor(property.getter, property.delegate) { src, symbol ->
@@ -79,7 +77,7 @@ object FirMemberPropertyChecker : FirRegularClassChecker() {
}
}
checkPropertyInitializer(containingDeclaration, property, isAbstract, reporter)
checkPropertyInitializer(containingDeclaration, property, reporter)
val hasOpenModifier = modifierList?.modifiers?.any { it.token == KtTokens.OPEN_KEYWORD } == true
if (hasOpenModifier &&
@@ -102,28 +100,6 @@ object FirMemberPropertyChecker : FirRegularClassChecker() {
}
}
private fun checkPropertyInitializer(
containingDeclaration: FirRegularClass,
property: FirProperty,
propertyIsAbstract: Boolean,
reporter: DiagnosticReporter
) {
property.initializer?.source?.let {
if (propertyIsAbstract) {
reporter.report(it, FirErrors.ABSTRACT_PROPERTY_WITH_INITIALIZER)
} else if (containingDeclaration.isInterface) {
reporter.report(it, FirErrors.PROPERTY_INITIALIZER_IN_INTERFACE)
}
}
if (propertyIsAbstract) {
if (property.initializer == null && property.delegate == null && property.returnTypeRef is FirImplicitTypeRef) {
property.source?.let {
reporter.report(it, FirErrors.PROPERTY_WITH_NO_TYPE_NO_INITIALIZER)
}
}
}
}
private fun checkAccessor(
accessor: FirPropertyAccessor?,
delegate: FirExpression?,
@@ -0,0 +1,26 @@
/*
* 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.declarations.FirFile
import org.jetbrains.kotlin.fir.declarations.FirProperty
// See old FE's [DeclarationsChecker]
object FirTopLevelPropertyChecker : FirFileChecker() {
override fun check(declaration: FirFile, context: CheckerContext, reporter: DiagnosticReporter) {
for (topLevelDeclaration in declaration.declarations) {
if (topLevelDeclaration is FirProperty) {
checkProperty(topLevelDeclaration, reporter)
}
}
}
private fun checkProperty(property: FirProperty, reporter: DiagnosticReporter) {
checkPropertyInitializer(null, property, reporter)
}
}
@@ -51,6 +51,8 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EMPTY_RANGE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ENUM_AS_SUPERTYPE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ERROR_FROM_JAVA_RESOLUTION
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPECTED_DECLARATION_WITH_BODY
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPECTED_DELEGATED_PROPERTY
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPECTED_PROPERTY_INITIALIZER
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPLICIT_DELEGATION_CALL_REQUIRED
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPOSED_FUNCTION_RETURN_TYPE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPOSED_PARAMETER_TYPE
@@ -397,6 +399,8 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension {
// Multi-platform projects
map.put(EXPECTED_DECLARATION_WITH_BODY, "Expected declaration must not have a body")
map.put(EXPECTED_PROPERTY_INITIALIZER, "Expected property cannot have an initializer")
map.put(EXPECTED_DELEGATED_PROPERTY, "Expected property cannot be delegated")
// Destructuring declaration
map.put(INITIALIZER_REQUIRED_FOR_DESTRUCTURING_DECLARATION, "Initializer required for destructuring declaration")
@@ -168,12 +168,20 @@ object FirErrors {
val PRIVATE_PROPERTY_IN_INTERFACE by error0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
val ABSTRACT_PROPERTY_WITH_INITIALIZER by error0<FirSourceElement, KtExpression>()
// TODO: val MUST_BE_INITIALIZED by error0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
// TODO: val MUST_BE_INITIALIZED_OR_BE_ABSTRACT by error0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
// TODO: val EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT by error0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
// TODO: val EXTENSION_PROPERTY_WITH_BACKING_FIELD by error0<FirSourceElement, KtExpression>()
// TODO: val PROPERTY_INITIALIZER_NO_BACKING_FIELD by error0<FirSourceElement, KtExpression>()
val PROPERTY_INITIALIZER_IN_INTERFACE by error0<FirSourceElement, KtExpression>()
val PROPERTY_WITH_NO_TYPE_NO_INITIALIZER by error0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
// TODO: val BACKING_FIELD_IN_INTERFACE by error0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
val ABSTRACT_DELEGATED_PROPERTY by error0<FirSourceElement, KtPropertyDelegate>()
val DELEGATED_PROPERTY_IN_INTERFACE by error0<FirSourceElement, KtPropertyDelegate>()
// TODO: val ACCESSOR_FOR_DELEGATED_PROPERTY by error1<FirSourceElement, PsiElement, FirPropertyAccessorSymbol>()
// TODO: val ACCESSOR_FOR_DELEGATED_PROPERTY by error1<FirSourceElement, KtPropertyAccessor, FirPropertyAccessorSymbol>()
val ABSTRACT_PROPERTY_WITH_GETTER by error0<FirSourceElement, KtPropertyAccessor>()
val ABSTRACT_PROPERTY_WITH_SETTER by error0<FirSourceElement, KtPropertyAccessor>()
@@ -182,6 +190,9 @@ object FirErrors {
// Multi-platform projects
val EXPECTED_DECLARATION_WITH_BODY by error0<FirSourceElement, KtDeclaration>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
val EXPECTED_PROPERTY_INITIALIZER by error0<FirSourceElement, KtExpression>()
// TODO: need to cover `by` as well as delegate expression
val EXPECTED_DELEGATED_PROPERTY by error0<FirSourceElement, KtPropertyDelegate>()
// Destructuring declaration
val INITIALIZER_REQUIRED_FOR_DESTRUCTURING_DECLARATION by error0<FirSourceElement, KtDestructuringDeclaration>()
@@ -61,6 +61,7 @@ object CommonDeclarationCheckers : DeclarationCheckers() {
override val fileCheckers: Set<FirFileChecker> = setOf(
FirTopLevelFunctionChecker,
FirTopLevelPropertyChecker,
)
override val controlFlowAnalyserCheckers: Set<FirControlFlowChecker> = setOf(
@@ -3,7 +3,7 @@
import kotlin.reflect.KProperty
interface T {
val a: Int by Delegate()
val a: Int by <!DELEGATED_PROPERTY_IN_INTERFACE!>Delegate()<!>
}
class Delegate {
@@ -15,7 +15,7 @@ expect class Foo(
constructor() : this("no")
val prop: String = "no"
val prop: String = <!EXPECTED_PROPERTY_INITIALIZER!>"no"<!>
var getSet: String
get() = "no"
@@ -14,8 +14,8 @@ expect class Foo {
var varWithGetSet: String
get set
val backingFieldVal: String = "no"
var backingFieldVar: String = "no"
val backingFieldVal: String = <!EXPECTED_PROPERTY_INITIALIZER!>"no"<!>
var backingFieldVar: String = <!EXPECTED_PROPERTY_INITIALIZER!>"no"<!>
val customAccessorVal: String
get() = "no"
@@ -25,7 +25,7 @@ expect class Foo {
lateinit var lateinitVar: String
val delegated: String by Delegate
val delegated: String by <!EXPECTED_DELEGATED_PROPERTY!>Delegate<!>
}
object Delegate { operator fun getValue(x: Any?, y: Any?): String = "" }
@@ -17,8 +17,8 @@ expect var varWithPlatformGetSet: String
expect get
expect set
expect val backingFieldVal: String = "no"
expect var backingFieldVar: String = "no"
expect val backingFieldVal: String = <!EXPECTED_PROPERTY_INITIALIZER!>"no"<!>
expect var backingFieldVar: String = <!EXPECTED_PROPERTY_INITIALIZER!>"no"<!>
expect val customAccessorVal: String
get() = "no"
@@ -30,7 +30,7 @@ expect const val constVal: Int
expect lateinit var lateinitVar: String
expect val delegated: String by Delegate
expect val delegated: String by <!EXPECTED_DELEGATED_PROPERTY!>Delegate<!>
object Delegate { operator fun getValue(x: Any?, y: Any?): String = "" }
fun test(): String {
@@ -1,5 +1,5 @@
expect fun foo1()
expect val bar1 = 42
expect val bar1 = <!EXPECTED_PROPERTY_INITIALIZER!>42<!>
expect class Baz1
actual fun foo2() = 42