diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt index 7c7aef1b0b5..36faf79c062 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt @@ -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) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertyChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertyChecker.kt index db2bc0b6df7..cd3036ece87 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertyChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertyChecker.kt @@ -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?, diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertyChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertyChecker.kt new file mode 100644 index 00000000000..c1c5c4246e1 --- /dev/null +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertyChecker.kt @@ -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) + } +} 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 2cff65fc736..682cf11b5cd 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 @@ -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") diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt index e987ed91adf..3d934e6174d 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt @@ -168,12 +168,20 @@ object FirErrors { val PRIVATE_PROPERTY_IN_INTERFACE by error0(SourceElementPositioningStrategies.VISIBILITY_MODIFIER) val ABSTRACT_PROPERTY_WITH_INITIALIZER by error0() + // TODO: val MUST_BE_INITIALIZED by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) + // TODO: val MUST_BE_INITIALIZED_OR_BE_ABSTRACT by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) + // TODO: val EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) + + // TODO: val EXTENSION_PROPERTY_WITH_BACKING_FIELD by error0() + // TODO: val PROPERTY_INITIALIZER_NO_BACKING_FIELD by error0() + val PROPERTY_INITIALIZER_IN_INTERFACE by error0() val PROPERTY_WITH_NO_TYPE_NO_INITIALIZER by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) + // TODO: val BACKING_FIELD_IN_INTERFACE by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) val ABSTRACT_DELEGATED_PROPERTY by error0() val DELEGATED_PROPERTY_IN_INTERFACE by error0() - // TODO: val ACCESSOR_FOR_DELEGATED_PROPERTY by error1() + // TODO: val ACCESSOR_FOR_DELEGATED_PROPERTY by error1() val ABSTRACT_PROPERTY_WITH_GETTER by error0() val ABSTRACT_PROPERTY_WITH_SETTER by error0() @@ -182,6 +190,9 @@ object FirErrors { // Multi-platform projects val EXPECTED_DECLARATION_WITH_BODY by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) + val EXPECTED_PROPERTY_INITIALIZER by error0() + // TODO: need to cover `by` as well as delegate expression + val EXPECTED_DELEGATED_PROPERTY by error0() // Destructuring declaration val INITIALIZER_REQUIRED_FOR_DESTRUCTURING_DECLARATION by error0() diff --git a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt index 0c3fe1f985a..e02afe01990 100644 --- a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt +++ b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/checkers/CommonDeclarationCheckers.kt @@ -61,6 +61,7 @@ object CommonDeclarationCheckers : DeclarationCheckers() { override val fileCheckers: Set = setOf( FirTopLevelFunctionChecker, + FirTopLevelPropertyChecker, ) override val controlFlowAnalyserCheckers: Set = setOf( diff --git a/compiler/testData/diagnostics/tests/delegatedProperty/inTrait.fir.kt b/compiler/testData/diagnostics/tests/delegatedProperty/inTrait.fir.kt index cbffec8a11e..78224599fb2 100644 --- a/compiler/testData/diagnostics/tests/delegatedProperty/inTrait.fir.kt +++ b/compiler/testData/diagnostics/tests/delegatedProperty/inTrait.fir.kt @@ -3,7 +3,7 @@ import kotlin.reflect.KProperty interface T { - val a: Int by Delegate() + val a: Int by Delegate() } class Delegate { diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/headerClassWithFunctionBody.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/headerClassWithFunctionBody.fir.kt index 7f3380f618c..c05fa0c4ff5 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/headerClassWithFunctionBody.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/headerClassWithFunctionBody.fir.kt @@ -15,7 +15,7 @@ expect class Foo( constructor() : this("no") - val prop: String = "no" + val prop: String = "no" var getSet: String get() = "no" diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/memberPropertyKinds.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/memberPropertyKinds.fir.kt index cc6004b6ea3..84232b8f2c1 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/memberPropertyKinds.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/memberPropertyKinds.fir.kt @@ -14,8 +14,8 @@ expect class Foo { var varWithGetSet: String get set - val backingFieldVal: String = "no" - var backingFieldVar: String = "no" + val backingFieldVal: String = "no" + var backingFieldVar: String = "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 Delegate } object Delegate { operator fun getValue(x: Any?, y: Any?): String = "" } diff --git a/compiler/testData/diagnostics/tests/multiplatform/topLevelProperty/differentKindsOfProperties.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/topLevelProperty/differentKindsOfProperties.fir.kt index d4ec1318474..aa42447a43f 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/topLevelProperty/differentKindsOfProperties.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/topLevelProperty/differentKindsOfProperties.fir.kt @@ -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 = "no" +expect var backingFieldVar: String = "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 Delegate object Delegate { operator fun getValue(x: Any?, y: Any?): String = "" } fun test(): String { diff --git a/compiler/testData/diagnostics/tests/sourceCompatibility/noMultiplatformProjects.fir.kt b/compiler/testData/diagnostics/tests/sourceCompatibility/noMultiplatformProjects.fir.kt index 3032aaa5e43..54fa5bc124b 100644 --- a/compiler/testData/diagnostics/tests/sourceCompatibility/noMultiplatformProjects.fir.kt +++ b/compiler/testData/diagnostics/tests/sourceCompatibility/noMultiplatformProjects.fir.kt @@ -1,5 +1,5 @@ expect fun foo1() -expect val bar1 = 42 +expect val bar1 = 42 expect class Baz1 actual fun foo2() = 42