FIR checker: report (expected|unnecessary) lateinit

This commit is contained in:
Jinseong Jeon
2021-03-08 12:35:02 -08:00
committed by Mikhail Glukhikh
parent 815e40620c
commit d2da15e039
25 changed files with 98 additions and 73 deletions
@@ -344,6 +344,7 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
val MUST_BE_INITIALIZED by error<FirSourceElement, KtProperty>(PositioningStrategy.DECLARATION_SIGNATURE)
val MUST_BE_INITIALIZED_OR_BE_ABSTRACT by error<FirSourceElement, KtProperty>(PositioningStrategy.DECLARATION_SIGNATURE)
val EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT by error<FirSourceElement, KtProperty>(PositioningStrategy.DECLARATION_SIGNATURE)
val UNNECESSARY_LATEINIT by warning<FirSourceElement, KtProperty>(PositioningStrategy.LATEINIT_MODIFIER)
val BACKING_FIELD_IN_INTERFACE by error<FirSourceElement, KtProperty>(PositioningStrategy.DECLARATION_SIGNATURE)
val EXTENSION_PROPERTY_WITH_BACKING_FIELD by error<FirSourceElement, KtExpression>()
@@ -368,6 +369,7 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
// TODO: need to cover `by` as well as delegate expression
val EXPECTED_DELEGATED_PROPERTY by error<FirSourceElement, KtPropertyDelegate>()
val EXPECTED_LATEINIT_PROPERTY by error<FirSourceElement, KtModifierListOwner>(PositioningStrategy.LATEINIT_MODIFIER)
}
val DESTRUCTING_DECLARATION by object : DiagnosticGroup("Destructuring declaration") {
@@ -235,6 +235,7 @@ object FirErrors {
val MUST_BE_INITIALIZED by error0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
val MUST_BE_INITIALIZED_OR_BE_ABSTRACT by error0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
val EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT by error0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
val UNNECESSARY_LATEINIT by warning0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.LATEINIT_MODIFIER)
val BACKING_FIELD_IN_INTERFACE by error0<FirSourceElement, KtProperty>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
val EXTENSION_PROPERTY_WITH_BACKING_FIELD by error0<FirSourceElement, KtExpression>()
val PROPERTY_INITIALIZER_NO_BACKING_FIELD by error0<FirSourceElement, KtExpression>()
@@ -252,6 +253,7 @@ object FirErrors {
val EXPECTED_DECLARATION_WITH_BODY by error0<FirSourceElement, KtDeclaration>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
val EXPECTED_PROPERTY_INITIALIZER by error0<FirSourceElement, KtExpression>()
val EXPECTED_DELEGATED_PROPERTY by error0<FirSourceElement, KtPropertyDelegate>()
val EXPECTED_LATEINIT_PROPERTY by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.LATEINIT_MODIFIER)
// Destructuring declaration
val INITIALIZER_REQUIRED_FOR_DESTRUCTURING_DECLARATION by error0<FirSourceElement, KtDestructuringDeclaration>()
@@ -75,6 +75,7 @@ internal fun checkExpectDeclarationVisibilityAndBody(
}
}
// Matched FE 1.0's [DeclarationsChecker#checkPropertyInitializer].
internal fun checkPropertyInitializer(
containingClass: FirRegularClass?,
property: FirProperty,
@@ -136,20 +137,28 @@ internal fun checkPropertyInitializer(
}
}
else -> {
val propertySource = property.source ?: return
val isExternal = property.isEffectivelyExternal(containingClass, context)
if (backingFieldRequired && !inInterface && !property.isLateInit && !isExpect && !isInitialized && !isExternal) {
property.source?.let {
if (property.receiverTypeRef != null && !property.hasAccessorImplementation) {
reporter.reportOn(it, FirErrors.EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT, context)
} else { // TODO: can be suppressed not to report diagnostics about no body
if (containingClass == null || property.hasAccessorImplementation) {
reporter.reportOn(it, FirErrors.MUST_BE_INITIALIZED, context)
} else {
reporter.reportOn(it, FirErrors.MUST_BE_INITIALIZED_OR_BE_ABSTRACT, context)
}
if (property.receiverTypeRef != null && !property.hasAccessorImplementation) {
reporter.reportOn(propertySource, FirErrors.EXTENSION_PROPERTY_MUST_HAVE_ACCESSORS_OR_BE_ABSTRACT, context)
} else { // TODO: can be suppressed not to report diagnostics about no body
if (containingClass == null || property.hasAccessorImplementation) {
reporter.reportOn(propertySource, FirErrors.MUST_BE_INITIALIZED, context)
} else {
reporter.reportOn(propertySource, FirErrors.MUST_BE_INITIALIZED_OR_BE_ABSTRACT, context)
}
}
}
if (property.isLateInit) {
if (isExpect) {
reporter.reportOn(propertySource, FirErrors.EXPECTED_LATEINIT_PROPERTY, context)
}
// TODO: like [BindingContext.MUST_BE_LATEINIT], we should consider variable with uninitialized error.
if (backingFieldRequired && !inInterface && isInitialized) {
reporter.reportOn(propertySource, FirErrors.UNNECESSARY_LATEINIT, context)
}
}
}
}
}
@@ -177,4 +186,4 @@ internal fun FirRegularClass.isInlineOrValueClass(): Boolean {
val modifierList = with(FirModifierList) { source.getModifierList() }
return isInline || modifierList?.modifiers?.any { it.token == KtTokens.VALUE_KEYWORD } == true
}
}
@@ -148,7 +148,14 @@ object FirMemberPropertiesChecker : FirRegularClassChecker() {
// So, our source of truth should be the full modifier list retrieved from the source.
val modifierList = with(FirModifierList) { property.source.getModifierList() }
checkPropertyInitializer(containingDeclaration, property, modifierList, isInitialized, reporter, context)
checkPropertyInitializer(
containingDeclaration,
property,
modifierList,
isInitialized,
reporter,
context
)
checkPropertyAccessors(property, reporter, context)
checkExpectDeclarationVisibilityAndBody(property, source, reporter, context)
@@ -28,7 +28,14 @@ object FirTopLevelPropertiesChecker : FirFileChecker() {
// So, our source of truth should be the full modifier list retrieved from the source.
val modifierList = with(FirModifierList) { source.getModifierList() }
checkPropertyInitializer(null, property, modifierList, property.initializer != null, reporter, context)
checkPropertyInitializer(
containingClass = null,
property,
modifierList,
isInitialized = property.initializer != null,
reporter,
context
)
checkPropertyAccessors(property, reporter, context)
checkExpectDeclarationVisibilityAndBody(property, source, reporter, context)
}
@@ -66,6 +66,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ERROR_FROM_JAVA_R
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.ERROR_IN_CONTRACT_DESCRIPTION
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_LATEINIT_PROPERTY
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPECTED_PRIVATE_DECLARATION
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPECTED_PROPERTY_INITIALIZER
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.EXPLICIT_DELEGATION_CALL_REQUIRED
@@ -181,6 +182,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.TYPE_PARAMETERS_I
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.TYPE_PARAMETER_AS_SUPERTYPE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.TYPE_PARAMETER_IN_CATCH_CLAUSE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNINITIALIZED_VARIABLE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNNECESSARY_LATEINIT
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNRESOLVED_LABEL
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNRESOLVED_REFERENCE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNSAFE_CALL
@@ -515,6 +517,7 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension {
map.put(MUST_BE_INITIALIZED, "Property must be initialized")
map.put(MUST_BE_INITIALIZED_OR_BE_ABSTRACT, "Property must be initialized 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(BACKING_FIELD_IN_INTERFACE, "Property in an interface cannot have a backing field")
map.put(EXTENSION_PROPERTY_WITH_BACKING_FIELD, "Extension property cannot be initialized because it has no backing field")
@@ -534,6 +537,7 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension {
map.put(EXPECTED_PROPERTY_INITIALIZER, "Expected property cannot have an initializer")
map.put(EXPECTED_DELEGATED_PROPERTY, "Expected property cannot be delegated")
map.put(EXPECTED_PRIVATE_DECLARATION, "Expected declaration cannot be private")
map.put(EXPECTED_LATEINIT_PROPERTY, "Expected property cannot be lateinit")
// Destructuring declaration
map.put(INITIALIZER_REQUIRED_FOR_DESTRUCTURING_DECLARATION, "Initializer required for destructuring declaration")
@@ -0,0 +1,6 @@
class Test {
<!UNNECESSARY_LATEINIT!>lateinit<!> var someRunnable: Runnable
init {
someRunnable = Runnable { someRunnable.run() }
}
}
@@ -1,4 +1,3 @@
// FIR_IDENTICAL
class Test {
lateinit var someRunnable: Runnable
init {
@@ -1,9 +0,0 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER
class Foo {
lateinit var bar: String
constructor(baz: Int) {
bar = ""
}
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// !DIAGNOSTICS: -UNUSED_PARAMETER
class Foo {
@@ -1,7 +0,0 @@
class Foo {
lateinit var bar: String
init {
bar = ""
}
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
class Foo {
<!UNNECESSARY_LATEINIT!>lateinit<!> var bar: String
@@ -1,13 +0,0 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER
class Foo {
lateinit var bar: String
constructor() {
bar = ""
}
constructor(baz: Int) {
bar = ""
}
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// !DIAGNOSTICS: -UNUSED_PARAMETER
class Foo {
@@ -1,15 +0,0 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER
class Foo {
lateinit var bar: String
constructor() {
bar = ""
}
constructor(a: Int) : this() {
}
constructor(a: Int, b: Int) : this(a) {
}
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// !DIAGNOSTICS: -UNUSED_PARAMETER
class Foo {
@@ -0,0 +1,8 @@
class Foo {
<!UNNECESSARY_LATEINIT!>lateinit<!> var bar: String
constructor(baz: Int) {
// At best, we should have error here despite of lateinit
bar += baz
}
}
@@ -1,4 +1,3 @@
// FIR_IDENTICAL
class Foo {
lateinit var bar: String
@@ -1,13 +0,0 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER
class Foo {
lateinit var bar: String
constructor() {
bar = ""
}
constructor(a: Int) : <!CYCLIC_CONSTRUCTOR_DELEGATION_CALL!>this<!>(a) {
bar = "a"
}
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// !DIAGNOSTICS: -UNUSED_PARAMETER
class Foo {
@@ -23,7 +23,7 @@ expect class Foo {
get() = "no"
set(value) {}
lateinit var lateinitVar: String
<!EXPECTED_LATEINIT_PROPERTY!>lateinit<!> var lateinitVar: String
val delegated: String by <!EXPECTED_DELEGATED_PROPERTY!>Delegate<!>
}
@@ -28,7 +28,7 @@ expect var customAccessorVar: String
expect const val constVal: Int
expect lateinit var lateinitVar: String
expect <!EXPECTED_LATEINIT_PROPERTY!>lateinit<!> var lateinitVar: String
expect val delegated: String by <!EXPECTED_DELEGATED_PROPERTY!>Delegate<!>
object Delegate { operator fun getValue(x: Any?, y: Any?): String = "" }
@@ -1021,6 +1021,12 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert
token,
)
}
add(FirErrors.UNNECESSARY_LATEINIT) { firDiagnostic ->
UnnecessaryLateinitImpl(
firDiagnostic as FirPsiDiagnostic<*>,
token,
)
}
add(FirErrors.BACKING_FIELD_IN_INTERFACE) { firDiagnostic ->
BackingFieldInInterfaceImpl(
firDiagnostic as FirPsiDiagnostic<*>,
@@ -1111,6 +1117,12 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert
token,
)
}
add(FirErrors.EXPECTED_LATEINIT_PROPERTY) { firDiagnostic ->
ExpectedLateinitPropertyImpl(
firDiagnostic as FirPsiDiagnostic<*>,
token,
)
}
add(FirErrors.INITIALIZER_REQUIRED_FOR_DESTRUCTURING_DECLARATION) { firDiagnostic ->
InitializerRequiredForDestructuringDeclarationImpl(
firDiagnostic as FirPsiDiagnostic<*>,
@@ -719,6 +719,10 @@ sealed class KtFirDiagnostic<PSI: PsiElement> : KtDiagnosticWithPsi<PSI> {
override val diagnosticClass get() = ExtensionPropertyMustHaveAccessorsOrBeAbstract::class
}
abstract class UnnecessaryLateinit : KtFirDiagnostic<KtProperty>() {
override val diagnosticClass get() = UnnecessaryLateinit::class
}
abstract class BackingFieldInInterface : KtFirDiagnostic<KtProperty>() {
override val diagnosticClass get() = BackingFieldInInterface::class
}
@@ -779,6 +783,10 @@ sealed class KtFirDiagnostic<PSI: PsiElement> : KtDiagnosticWithPsi<PSI> {
override val diagnosticClass get() = ExpectedDelegatedProperty::class
}
abstract class ExpectedLateinitProperty : KtFirDiagnostic<KtModifierListOwner>() {
override val diagnosticClass get() = ExpectedLateinitProperty::class
}
abstract class InitializerRequiredForDestructuringDeclaration : KtFirDiagnostic<KtDestructuringDeclaration>() {
override val diagnosticClass get() = InitializerRequiredForDestructuringDeclaration::class
}
@@ -1164,6 +1164,13 @@ internal class ExtensionPropertyMustHaveAccessorsOrBeAbstractImpl(
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class UnnecessaryLateinitImpl(
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,
) : KtFirDiagnostic.UnnecessaryLateinit(), KtAbstractFirDiagnostic<KtProperty> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class BackingFieldInInterfaceImpl(
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,
@@ -1269,6 +1276,13 @@ internal class ExpectedDelegatedPropertyImpl(
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class ExpectedLateinitPropertyImpl(
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,
) : KtFirDiagnostic.ExpectedLateinitProperty(), KtAbstractFirDiagnostic<KtModifierListOwner> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class InitializerRequiredForDestructuringDeclarationImpl(
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,