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:
committed by
Mikhail Glukhikh
parent
f9378a3ab7
commit
57c8dd86a0
+90
-4
@@ -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)
|
||||
|
||||
+9
-33
@@ -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?,
|
||||
|
||||
+26
@@ -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)
|
||||
}
|
||||
}
|
||||
+4
@@ -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")
|
||||
|
||||
+12
-1
@@ -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>()
|
||||
|
||||
+1
@@ -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 {
|
||||
|
||||
Vendored
+1
-1
@@ -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"
|
||||
|
||||
+3
-3
@@ -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 = "" }
|
||||
|
||||
compiler/testData/diagnostics/tests/multiplatform/topLevelProperty/differentKindsOfProperties.fir.kt
Vendored
+3
-3
@@ -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
-1
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user