From 38319c55a81bcf8b68b94197bf3e0acd1735fca3 Mon Sep 17 00:00:00 2001 From: Nikita Bobko Date: Mon, 15 May 2023 12:28:56 +0200 Subject: [PATCH] [FE] Replace some MUST_BE_INITIALIZED messages with MUST_BE_INITIALIZED_OR_BE_FINAL From user point of view it's an improvement in compilation message. From technical point of view it's an introduction of new compilation diagnostic. Review: https://jetbrains.team/p/kt/reviews/9967 I'm going to deprecate `open val` case in the next few commits KT-57553. But it is always possible to suggest using `final` for `open val` case. --- .../diagnostics/KtFirDataClassConverters.kt | 12 +++++ .../api/fir/diagnostics/KtFirDiagnostics.kt | 8 +++ .../fir/diagnostics/KtFirDiagnosticsImpl.kt | 10 ++++ .../diagnostics/FirDiagnosticsList.kt | 3 ++ .../fir/analysis/diagnostics/FirErrors.kt | 2 + .../FirTopLevelPropertiesChecker.kt | 30 +++++++++-- .../diagnostics/FirErrorsDefaultMessages.kt | 5 ++ .../cfg/ControlFlowInformationProviderImpl.kt | 12 ++--- .../jetbrains/kotlin/diagnostics/Errors.java | 2 + .../rendering/DefaultErrorMessages.java | 2 + .../kotlin/resolve/BindingContext.java | 1 + .../kotlin/resolve/DeclarationsChecker.kt | 50 ++++++++++++++----- .../LateinitModifierApplicabilityChecker.kt | 5 +- .../tests/backingField/InitOpenSetter.fir.kt | 2 +- .../tests/backingField/InitOpenSetter.kt | 2 +- .../MustBeInitializedEffectivelyFinalOff.kt | 2 +- .../ValDeferredInitInOpenClass.fir.kt | 6 +-- .../VarDeferredInitInOpenClass.kt | 8 +-- 18 files changed, 124 insertions(+), 38 deletions(-) diff --git a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDataClassConverters.kt b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDataClassConverters.kt index 2f219c4fb69..1f7bab43c9e 100644 --- a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDataClassConverters.kt +++ b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDataClassConverters.kt @@ -2851,12 +2851,24 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirErrors.MUST_BE_INITIALIZED_OR_BE_FINAL) { firDiagnostic -> + MustBeInitializedOrBeFinalImpl( + firDiagnostic as KtPsiDiagnostic, + token, + ) + } add(FirErrors.MUST_BE_INITIALIZED_OR_BE_ABSTRACT) { firDiagnostic -> MustBeInitializedOrBeAbstractImpl( firDiagnostic as KtPsiDiagnostic, token, ) } + add(FirErrors.MUST_BE_INITIALIZED_OR_FINAL_OR_ABSTRACT) { firDiagnostic -> + MustBeInitializedOrFinalOrAbstractImpl( + firDiagnostic as KtPsiDiagnostic, + token, + ) + } add(FirErrors.EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT) { firDiagnostic -> ExtensionPropertyMustHaveAccessorsOrBeAbstractImpl( firDiagnostic as KtPsiDiagnostic, diff --git a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnostics.kt b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnostics.kt index 2cdf0a41ab6..8fdba4f51b0 100644 --- a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnostics.kt +++ b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnostics.kt @@ -2019,10 +2019,18 @@ sealed class KtFirDiagnostic : KtDiagnosticWithPsi { override val diagnosticClass get() = MustBeInitialized::class } + abstract class MustBeInitializedOrBeFinal : KtFirDiagnostic() { + override val diagnosticClass get() = MustBeInitializedOrBeFinal::class + } + abstract class MustBeInitializedOrBeAbstract : KtFirDiagnostic() { override val diagnosticClass get() = MustBeInitializedOrBeAbstract::class } + abstract class MustBeInitializedOrFinalOrAbstract : KtFirDiagnostic() { + override val diagnosticClass get() = MustBeInitializedOrFinalOrAbstract::class + } + abstract class ExtensionPropertyMustHaveAccessorsOrBeAbstract : KtFirDiagnostic() { override val diagnosticClass get() = ExtensionPropertyMustHaveAccessorsOrBeAbstract::class } diff --git a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnosticsImpl.kt b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnosticsImpl.kt index fd8a2e59e9f..1f21f598f05 100644 --- a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnosticsImpl.kt +++ b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/diagnostics/KtFirDiagnosticsImpl.kt @@ -2425,11 +2425,21 @@ internal class MustBeInitializedImpl( override val token: KtLifetimeToken, ) : KtFirDiagnostic.MustBeInitialized(), KtAbstractFirDiagnostic +internal class MustBeInitializedOrBeFinalImpl( + override val firDiagnostic: KtPsiDiagnostic, + override val token: KtLifetimeToken, +) : KtFirDiagnostic.MustBeInitializedOrBeFinal(), KtAbstractFirDiagnostic + internal class MustBeInitializedOrBeAbstractImpl( override val firDiagnostic: KtPsiDiagnostic, override val token: KtLifetimeToken, ) : KtFirDiagnostic.MustBeInitializedOrBeAbstract(), KtAbstractFirDiagnostic +internal class MustBeInitializedOrFinalOrAbstractImpl( + override val firDiagnostic: KtPsiDiagnostic, + override val token: KtLifetimeToken, +) : KtFirDiagnostic.MustBeInitializedOrFinalOrAbstract(), KtAbstractFirDiagnostic + internal class ExtensionPropertyMustHaveAccessorsOrBeAbstractImpl( override val firDiagnostic: KtPsiDiagnostic, override val token: KtLifetimeToken, diff --git a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt index d7f0a92af60..fdc2e34bd30 100644 --- a/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt +++ b/compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt @@ -1009,7 +1009,10 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") { val PROPERTY_WITH_NO_TYPE_NO_INITIALIZER by error(PositioningStrategy.DECLARATION_SIGNATURE) val MUST_BE_INITIALIZED by error(PositioningStrategy.DECLARATION_SIGNATURE) + val MUST_BE_INITIALIZED_OR_BE_FINAL by error(PositioningStrategy.DECLARATION_SIGNATURE) val MUST_BE_INITIALIZED_OR_BE_ABSTRACT by error(PositioningStrategy.DECLARATION_SIGNATURE) + val MUST_BE_INITIALIZED_OR_FINAL_OR_ABSTRACT by error(PositioningStrategy.DECLARATION_SIGNATURE) + val EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT by error(PositioningStrategy.DECLARATION_SIGNATURE) val UNNECESSARY_LATEINIT by warning(PositioningStrategy.LATEINIT_MODIFIER) diff --git a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt index 68b1385fdef..a89a17066e1 100644 --- a/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt +++ b/compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt @@ -533,7 +533,9 @@ object FirErrors { val PROPERTY_INITIALIZER_IN_INTERFACE by error0() val PROPERTY_WITH_NO_TYPE_NO_INITIALIZER by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) val MUST_BE_INITIALIZED by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) + val MUST_BE_INITIALIZED_OR_BE_FINAL by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) val MUST_BE_INITIALIZED_OR_BE_ABSTRACT by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) + val MUST_BE_INITIALIZED_OR_FINAL_OR_ABSTRACT by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) val EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) val UNNECESSARY_LATEINIT by warning0(SourceElementPositioningStrategies.LATEINIT_MODIFIER) val BACKING_FIELD_IN_INTERFACE by error0(SourceElementPositioningStrategies.DECLARATION_SIGNATURE) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt index 681f31a109c..c19649ab718 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt @@ -153,11 +153,7 @@ internal fun checkPropertyInitializer( if (property.receiverParameter != null && !property.hasAnyAccessorImplementation) { reporter.reportOn(propertySource, FirErrors.EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT, context) } else if (reachable) { // TODO: can be suppressed not to report diagnostics about no body - if (containingClass == null || property.hasAnyAccessorImplementation) { - reporter.reportOn(propertySource, FirErrors.MUST_BE_INITIALIZED, context) - } else { - reporter.reportOn(propertySource, FirErrors.MUST_BE_INITIALIZED_OR_BE_ABSTRACT, context) - } + reportMustBeInitialized(property, isDefinitelyAssignedInConstructor, containingClass, propertySource, reporter, context) } } if (property.isLateInit) { @@ -175,6 +171,30 @@ internal fun checkPropertyInitializer( } } +private fun reportMustBeInitialized( + property: FirProperty, + isDefinitelyAssignedInConstructor: Boolean, + containingClass: FirClass?, + propertySource: KtSourceElement, + reporter: DiagnosticReporter, + context: CheckerContext, +) { + check(!property.isAbstract) { "${::reportMustBeInitialized.name} isn't called for abstract properties" } + val suggestMakingItFinal = containingClass != null && + !property.hasSetterAccessorImplementation && + property.getEffectiveModality(containingClass, context.languageVersionSettings) != Modality.FINAL && + isDefinitelyAssignedInConstructor + val suggestMakingItAbstract = containingClass != null && !property.hasAnyAccessorImplementation + + val factory = when { + suggestMakingItFinal && suggestMakingItAbstract -> FirErrors.MUST_BE_INITIALIZED_OR_FINAL_OR_ABSTRACT + suggestMakingItFinal -> FirErrors.MUST_BE_INITIALIZED_OR_BE_FINAL + suggestMakingItAbstract -> FirErrors.MUST_BE_INITIALIZED_OR_BE_ABSTRACT + else -> FirErrors.MUST_BE_INITIALIZED + } + reporter.reportOn(propertySource, factory, context) +} + private val FirPropertyAccessor?.hasImplementation: Boolean get() = (this !is FirDefaultPropertyAccessor && this?.hasBody == true) private val FirProperty.hasSetterAccessorImplementation: Boolean diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt index d4a6e234116..a701521ad1f 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrorsDefaultMessages.kt @@ -337,6 +337,8 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.MULTIPLE_VARARG_P import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.MULTI_FIELD_VALUE_CLASS_PRIMARY_CONSTRUCTOR_DEFAULT_PARAMETER import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.MUST_BE_INITIALIZED import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.MUST_BE_INITIALIZED_OR_BE_ABSTRACT +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.MUST_BE_INITIALIZED_OR_BE_FINAL +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.MUST_BE_INITIALIZED_OR_FINAL_OR_ABSTRACT import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NAMED_ARGUMENTS_NOT_ALLOWED import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NAMED_PARAMETER_NOT_FOUND import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NAME_FOR_AMBIGUOUS_PARAMETER @@ -1629,7 +1631,10 @@ object FirErrorsDefaultMessages : BaseDiagnosticRendererFactory() { map.put(TYPECHECKER_HAS_RUN_INTO_RECURSIVE_PROBLEM, "Type checking has run into a recursive problem. Easiest workaround: specify types of your declarations explicitly") map.put(MUST_BE_INITIALIZED, "Property must be initialized") + map.put(MUST_BE_INITIALIZED_OR_BE_FINAL, "Property must be initialized or be final") map.put(MUST_BE_INITIALIZED_OR_BE_ABSTRACT, "Property must be initialized or be abstract") + map.put(MUST_BE_INITIALIZED_OR_FINAL_OR_ABSTRACT, "Property must be initialized, be final, or be abstract"); + map.put(EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT, "Extension property must have accessors or be abstract") map.put(UNNECESSARY_LATEINIT, "Lateinit is unnecessary: definitely initialized in constructors") diff --git a/compiler/frontend/cfg/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProviderImpl.kt b/compiler/frontend/cfg/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProviderImpl.kt index a5e68778d11..b12113e34ca 100644 --- a/compiler/frontend/cfg/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProviderImpl.kt +++ b/compiler/frontend/cfg/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProviderImpl.kt @@ -653,8 +653,13 @@ class ControlFlowInformationProviderImpl private constructor( val initializers = initializersMap[pseudocode.exitInstruction] ?: return val declaredVariables = pseudocodeVariablesData.getDeclaredVariables(pseudocode, false) for (variable in declaredVariables) { + // - If we have a primary constructor and several secondary constructors then the `if` below is called only once for the primary + // constructor/init block + // - If we have several secondary constructors without a primary constructor then the `if` below is called each time for every + // secondary constructor. (init block is considered as part of each secondary constructor in that case) if (variable is PropertyDescriptor) { if (initializers.incoming.getOrNull(variable)?.definitelyInitialized() == true) continue + trace.record(IS_DEFINITELY_NOT_ASSIGNED_IN_CONSTRUCTOR, variable) trace.record(IS_UNINITIALIZED, variable) } } @@ -1472,10 +1477,3 @@ class ControlFlowInformationProviderImpl private constructor( return result } } - -private fun PropertyDescriptor.getEffectiveModality(languageVersionSettings: LanguageVersionSettings): Modality = - when (languageVersionSettings.supportsFeature(LanguageFeature.TakeIntoAccountEffectivelyFinalInMustBeInitializedCheck) && - modality == Modality.OPEN && (containingDeclaration as? ClassDescriptor)?.modality == Modality.FINAL) { - true -> Modality.FINAL - false -> modality - } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index f864472a8da..90eae3bf497 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -673,7 +673,9 @@ public interface Errors { DiagnosticFactory0 PROPERTY_WITH_NO_TYPE_NO_INITIALIZER = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE); DiagnosticFactory0 MUST_BE_INITIALIZED = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE); + DiagnosticFactory0 MUST_BE_INITIALIZED_OR_BE_FINAL = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE); DiagnosticFactory0 MUST_BE_INITIALIZED_OR_BE_ABSTRACT = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE); + DiagnosticFactory0 MUST_BE_INITIALIZED_OR_FINAL_OR_ABSTRACT = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE); DiagnosticFactory0 EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE); DiagnosticFactory0 UNNECESSARY_LATEINIT = DiagnosticFactory0.create(WARNING, LATEINIT_MODIFIER); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index 905f8e6ad3b..b01d26b79d1 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -306,7 +306,9 @@ public class DefaultErrorMessages { MAP.put(PRIVATE_SETTER_FOR_OPEN_PROPERTY, "Private setters are not allowed for open properties"); MAP.put(BACKING_FIELD_IN_INTERFACE, "Property in an interface cannot have a backing field"); MAP.put(MUST_BE_INITIALIZED, "Property must be initialized"); + MAP.put(MUST_BE_INITIALIZED_OR_BE_FINAL, "Property must be initialized or be final"); MAP.put(MUST_BE_INITIALIZED_OR_BE_ABSTRACT, "Property must be initialized or be abstract"); + MAP.put(MUST_BE_INITIALIZED_OR_FINAL_OR_ABSTRACT, "Property must be initialized, be final, or be abstract"); MAP.put(EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT, "Extension property must have accessors or be abstract"); MAP.put(UNNECESSARY_LATEINIT, "Lateinit is unnecessary: definitely initialized in constructors"); MAP.put(PROPERTY_INITIALIZER_IN_INTERFACE, "Property initializers are not allowed in interfaces"); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingContext.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingContext.java index 771ae57e053..5f8882732a6 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingContext.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingContext.java @@ -224,6 +224,7 @@ public interface BindingContext { } }; WritableSlice IS_UNINITIALIZED = Slices.createSimpleSetSlice(); + WritableSlice IS_DEFINITELY_NOT_ASSIGNED_IN_CONSTRUCTOR = Slices.createSimpleSetSlice(); WritableSlice MUST_BE_LATEINIT = Slices.createSimpleSetSlice(); WritableSlice LAMBDA_INVOCATIONS = Slices.createSimpleSlice(); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt index bb0deb67ad4..d403eee3186 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt @@ -694,7 +694,7 @@ class DeclarationsChecker( } private fun checkPropertyInitializer(property: KtProperty, propertyDescriptor: PropertyDescriptor) { - val hasAccessorImplementation = propertyDescriptor.hasAccessorImplementation() + val hasAnyAccessorImplementation = propertyDescriptor.hasAnyAccessorImplementation() val containingDeclaration = propertyDescriptor.containingDeclaration val inInterface = DescriptorUtils.isInterface(containingDeclaration) @@ -709,7 +709,7 @@ class DeclarationsChecker( } val backingFieldRequired = trace.bindingContext.get(BACKING_FIELD_REQUIRED, propertyDescriptor) ?: false - if (inInterface && backingFieldRequired && hasAccessorImplementation) { + if (inInterface && backingFieldRequired && hasAnyAccessorImplementation) { trace.report(BACKING_FIELD_IN_INTERFACE.on(property)) } @@ -772,14 +772,10 @@ class DeclarationsChecker( val isUninitialized = trace.bindingContext.get(IS_UNINITIALIZED, propertyDescriptor) ?: false val isExternal = propertyDescriptor.isEffectivelyExternal() if (backingFieldRequired && !inInterface && !propertyDescriptor.isLateInit && !isExpect && isUninitialized && !isExternal) { - if (propertyDescriptor.extensionReceiverParameter != null && !hasAccessorImplementation) { + if (propertyDescriptor.extensionReceiverParameter != null && !hasAnyAccessorImplementation) { trace.report(EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT.on(property)) } else if (diagnosticSuppressor.shouldReportNoBody(propertyDescriptor)) { - if (containingDeclaration !is ClassDescriptor || hasAccessorImplementation) { - trace.report(MUST_BE_INITIALIZED.on(property)) - } else { - trace.report(MUST_BE_INITIALIZED_OR_BE_ABSTRACT.on(property)) - } + reportMustBeInitialized(propertyDescriptor, containingDeclaration, hasAnyAccessorImplementation, property, trace) } } else if (property.typeReference == null && !languageVersionSettings.supportsFeature(LanguageFeature.ShortSyntaxForPropertyGetters)) { trace.report( @@ -803,6 +799,30 @@ class DeclarationsChecker( } } + private fun reportMustBeInitialized( + propertyDescriptor: PropertyDescriptor, + containingDeclaration: DeclarationDescriptor, + hasAnyAccessorImplementation: Boolean, + property: KtProperty, + trace: BindingTrace, + ) { + check(propertyDescriptor.getEffectiveModality(languageVersionSettings) != Modality.ABSTRACT) { + "${::reportMustBeInitialized.name} isn't called for abstract properties" + } + val suggestMakingItFinal = containingDeclaration is ClassDescriptor && + !propertyDescriptor.hasSetterAccessorImplementation() && + propertyDescriptor.getEffectiveModality(languageVersionSettings) != Modality.FINAL && + trace.bindingContext.get(IS_DEFINITELY_NOT_ASSIGNED_IN_CONSTRUCTOR, propertyDescriptor) == false + val suggestMakingItAbstract = containingDeclaration is ClassDescriptor && !hasAnyAccessorImplementation + + when { + suggestMakingItFinal && suggestMakingItAbstract -> trace.report(MUST_BE_INITIALIZED_OR_FINAL_OR_ABSTRACT.on(property)) + suggestMakingItFinal -> trace.report(MUST_BE_INITIALIZED_OR_BE_FINAL.on(property)) + suggestMakingItAbstract -> trace.report(MUST_BE_INITIALIZED_OR_BE_ABSTRACT.on(property)) + else -> trace.report(MUST_BE_INITIALIZED.on(property)) + } + } + private fun noExplicitTypeOrGetterType(property: KtProperty) = property.typeReference == null && (property.getter == null || (property.getter!!.hasBlockBody() && property.getter!!.returnTypeReference == null)) @@ -1067,10 +1087,14 @@ class DeclarationsChecker( return !modifierList.hasModifier(KtTokens.OVERRIDE_KEYWORD) } - fun PropertyDescriptor.hasAccessorImplementation(): Boolean { - getter?.let { if (it.hasBody()) return true } - setter?.let { if (it.hasBody()) return true } - return false - } + private fun PropertyDescriptor.hasSetterAccessorImplementation(): Boolean = setter?.hasBody() == true + fun PropertyDescriptor.hasAnyAccessorImplementation(): Boolean = hasSetterAccessorImplementation() || getter?.hasBody() == true } } + +fun PropertyDescriptor.getEffectiveModality(languageVersionSettings: LanguageVersionSettings): Modality = + when (languageVersionSettings.supportsFeature(LanguageFeature.TakeIntoAccountEffectivelyFinalInMustBeInitializedCheck) && + modality == Modality.OPEN && (containingDeclaration as? ClassDescriptor)?.modality == Modality.FINAL) { + true -> Modality.FINAL + false -> modality + } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/LateinitModifierApplicabilityChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/LateinitModifierApplicabilityChecker.kt index 323d6e2f09b..4909bd5b28d 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/LateinitModifierApplicabilityChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/LateinitModifierApplicabilityChecker.kt @@ -17,7 +17,6 @@ package org.jetbrains.kotlin.resolve import org.jetbrains.kotlin.builtins.KotlinBuiltIns -import org.jetbrains.kotlin.builtins.UnsignedTypes import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.config.LanguageVersionSettings import org.jetbrains.kotlin.descriptors.ClassDescriptor @@ -30,7 +29,7 @@ import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtCallableDeclaration import org.jetbrains.kotlin.psi.KtParameter import org.jetbrains.kotlin.psi.KtProperty -import org.jetbrains.kotlin.resolve.DeclarationsChecker.Companion.hasAccessorImplementation +import org.jetbrains.kotlin.resolve.DeclarationsChecker.Companion.hasAnyAccessorImplementation import org.jetbrains.kotlin.resolve.descriptorUtil.inlineClassRepresentation import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.TypeUtils @@ -108,7 +107,7 @@ object LateinitModifierApplicabilityChecker { if (descriptor is PropertyDescriptor) { val isAbstract = descriptor.modality == Modality.ABSTRACT val hasDelegateExpressionOrInitializer = ktDeclaration is KtProperty && ktDeclaration.hasDelegateExpressionOrInitializer() - val hasAccessorImplementation = descriptor.hasAccessorImplementation() + val hasAccessorImplementation = descriptor.hasAnyAccessorImplementation() val hasBackingField = trace.bindingContext.get(BindingContext.BACKING_FIELD_REQUIRED, descriptor) ?: false if (ktDeclaration is KtParameter) { diff --git a/compiler/testData/diagnostics/tests/backingField/InitOpenSetter.fir.kt b/compiler/testData/diagnostics/tests/backingField/InitOpenSetter.fir.kt index 95947d2bc09..c238b403efd 100644 --- a/compiler/testData/diagnostics/tests/backingField/InitOpenSetter.fir.kt +++ b/compiler/testData/diagnostics/tests/backingField/InitOpenSetter.fir.kt @@ -2,7 +2,7 @@ abstract class My(val v: Int) { // Ok: variable is just abstract abstract var x: Int - open var y: Int + open var y: Int open var z: Int diff --git a/compiler/testData/diagnostics/tests/backingField/InitOpenSetter.kt b/compiler/testData/diagnostics/tests/backingField/InitOpenSetter.kt index b708cdd35d7..f5ad3e422c4 100644 --- a/compiler/testData/diagnostics/tests/backingField/InitOpenSetter.kt +++ b/compiler/testData/diagnostics/tests/backingField/InitOpenSetter.kt @@ -2,7 +2,7 @@ abstract class My(val v: Int) { // Ok: variable is just abstract abstract var x: Int - open var y: Int + open var y: Int open var z: Int diff --git a/compiler/testData/diagnostics/tests/backingField/MustBeInitializedEffectivelyFinalOff.kt b/compiler/testData/diagnostics/tests/backingField/MustBeInitializedEffectivelyFinalOff.kt index 19441d7af92..d1d174c6075 100644 --- a/compiler/testData/diagnostics/tests/backingField/MustBeInitializedEffectivelyFinalOff.kt +++ b/compiler/testData/diagnostics/tests/backingField/MustBeInitializedEffectivelyFinalOff.kt @@ -5,7 +5,7 @@ open class Base { } class Foo : Base() { - override var x: String + override var x: String init { x = "" diff --git a/compiler/testData/diagnostics/tests/backingField/ValDeferredInitInOpenClass.fir.kt b/compiler/testData/diagnostics/tests/backingField/ValDeferredInitInOpenClass.fir.kt index 9e7fd4dbd3c..2aa02db4760 100644 --- a/compiler/testData/diagnostics/tests/backingField/ValDeferredInitInOpenClass.fir.kt +++ b/compiler/testData/diagnostics/tests/backingField/ValDeferredInitInOpenClass.fir.kt @@ -11,7 +11,7 @@ open class Foo { val a0: Int val e0: Int val c0: Int = 1 - open val b0: Int + open val b0: Int open val f0: Int open val d0: Int = 1 @@ -19,7 +19,7 @@ open class Foo { val a1: Int; get() = field val e1: Int; get() = field val c1: Int = 1; get() = field - open val b1: Int; get() = field + open val b1: Int; get() = field open val f1: Int; get() = field open val d1: Int = 1; get() = field @@ -27,7 +27,7 @@ open class Foo { val a2: Int; get val e2: Int; get val c2: Int = 1; get - open val b2: Int; get + open val b2: Int; get open val f2: Int; get open val d2: Int = 1; get diff --git a/compiler/testData/diagnostics/tests/backingField/VarDeferredInitInOpenClass.kt b/compiler/testData/diagnostics/tests/backingField/VarDeferredInitInOpenClass.kt index 1ad40793d72..81a7e468a39 100644 --- a/compiler/testData/diagnostics/tests/backingField/VarDeferredInitInOpenClass.kt +++ b/compiler/testData/diagnostics/tests/backingField/VarDeferredInitInOpenClass.kt @@ -13,28 +13,28 @@ open class Foo { var a00: Int; var a01: Int; set(v) { field = v }; var a02: Int; set; var a03: Int; set(v) {}; var e00: Int; var e01: Int; set(v) { field = v }; var e02: Int; set; var e03: Int; set(v) {}; var c00: Int = 1; var c01: Int = 1; set(v) { field = v }; var c02: Int = 1; set; var c03: Int = 1; set(v) {}; - open var b00: Int; open var b01: Int; set(v) { field = v }; open var b02: Int; set; open var b03: Int; set(v) {}; + open var b00: Int; open var b01: Int; set(v) { field = v }; open var b02: Int; set; open var b03: Int; set(v) {}; open var f00: Int; open var f01: Int; set(v) { field = v }; open var f02: Int; set; open var f03: Int; set(v) {}; open var d00: Int = 1; open var d01: Int = 1; set(v) { field = v }; open var d02: Int = 1; set; open var d03: Int = 1; set(v) {}; // getter with field var a10: Int; get() = field; var a11: Int; set(v) { field = v } get() = field; var a12: Int; set get() = field; var a13: Int; set(v) {} get() = field; var e10: Int; get() = field; var e11: Int; set(v) { field = v } get() = field; var e12: Int; set get() = field; var e13: Int; set(v) {} get() = field; var c10: Int = 1; get() = field; var c11: Int = 1; set(v) { field = v } get() = field; var c12: Int = 1; set get() = field; var c13: Int = 1; set(v) {} get() = field; - open var b10: Int; get() = field; open var b11: Int; set(v) { field = v } get() = field; open var b12: Int; set get() = field; open var b13: Int; set(v) {} get() = field; + open var b10: Int; get() = field; open var b11: Int; set(v) { field = v } get() = field; open var b12: Int; set get() = field; open var b13: Int; set(v) {} get() = field; open var f10: Int; get() = field; open var f11: Int; set(v) { field = v } get() = field; open var f12: Int; set get() = field; open var f13: Int; set(v) {} get() = field; open var d10: Int = 1; get() = field; open var d11: Int = 1; set(v) { field = v } get() = field; open var d12: Int = 1; set get() = field; open var d13: Int = 1; set(v) {} get() = field; // getter with empty body var a20: Int; get; var a21: Int; set(v) { field = v } get; var a22: Int; set get; var a23: Int; set(v) {} get; var e20: Int; get; var e21: Int; set(v) { field = v } get; var e22: Int; set get; var e23: Int; set(v) {} get; var c20: Int = 1; get; var c21: Int = 1; set(v) { field = v } get; var c22: Int = 1; set get; var c23: Int = 1; set(v) {} get; - open var b20: Int; get; open var b21: Int; set(v) { field = v } get; open var b22: Int; set get; open var b23: Int; set(v) {} get; + open var b20: Int; get; open var b21: Int; set(v) { field = v } get; open var b22: Int; set get; open var b23: Int; set(v) {} get; open var f20: Int; get; open var f21: Int; set(v) { field = v } get; open var f22: Int; set get; open var f23: Int; set(v) {} get; open var d20: Int = 1; get; open var d21: Int = 1; set(v) { field = v } get; open var d22: Int = 1; set get; open var d23: Int = 1; set(v) {} get; // getter no field var a30: Int; get() = 1; var a31: Int; set(v) { field = v } get() = 1; var a32: Int; set get() = 1; var a33: Int; set(v) {} get() = 1; var e30: Int; get() = 1; var e31: Int; set(v) { field = v } get() = 1; var e32: Int; set get() = 1; var e33: Int; set(v) {} get() = 1; var c30: Int = 1; get() = 1; var c31: Int = 1; set(v) { field = v } get() = 1; var c32: Int = 1; set get() = 1; var c33: Int = 1; set(v) {} get() = 1; - open var b30: Int; get() = 1; open var b31: Int; set(v) { field = v } get() = 1; open var b32: Int; set get() = 1; open var b33: Int; set(v) {} get() = 1; + open var b30: Int; get() = 1; open var b31: Int; set(v) { field = v } get() = 1; open var b32: Int; set get() = 1; open var b33: Int; set(v) {} get() = 1; open var f30: Int; get() = 1; open var f31: Int; set(v) { field = v } get() = 1; open var f32: Int; set get() = 1; open var f33: Int; set(v) {} get() = 1; open var d30: Int = 1; get() = 1; open var d31: Int = 1; set(v) { field = v } get() = 1; open var d32: Int = 1; set get() = 1; open var d33: Int = 1; set(v) {} get() = 1;