FIR checker: report VAL_REASSIGNMENT_VIA_BACKING_FIELD(_ERROR)

This commit is contained in:
Jinseong Jeon
2021-03-15 23:14:00 +03:00
committed by Mikhail Glukhikh
parent a6d11b8914
commit 93289aa899
19 changed files with 149 additions and 62 deletions
@@ -2091,6 +2091,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti
runTest("compiler/testData/diagnostics/tests/backingField/kt782packageLevel.kt");
}
@Test
@TestMetadata("LocalDeclarations.kt")
public void testLocalDeclarations() throws Exception {
runTest("compiler/testData/diagnostics/tests/backingField/LocalDeclarations.kt");
}
@Test
@TestMetadata("SetterWithExplicitType.kt")
public void testSetterWithExplicitType() throws Exception {
@@ -427,6 +427,12 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
val VAL_REASSIGNMENT by error<FirSourceElement, KtExpression> {
parameter<FirPropertySymbol>("variable")
}
val VAL_REASSIGNMENT_VIA_BACKING_FIELD by warning<FirSourceElement, KtExpression> {
parameter<FirPropertySymbol>("variable")
}
val VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR by error<FirSourceElement, KtExpression> {
parameter<FirPropertySymbol>("variable")
}
val WRONG_INVOCATION_KIND by warning<FirSourceElement, PsiElement> {
parameter<AbstractFirBasedSymbol<*>>("declaration")
parameter<EventOccurrencesRange>("requiredRange")
@@ -275,6 +275,8 @@ object FirErrors {
// Control flow diagnostics
val UNINITIALIZED_VARIABLE by error1<FirSourceElement, KtSimpleNameExpression, FirPropertySymbol>()
val VAL_REASSIGNMENT by error1<FirSourceElement, KtExpression, FirPropertySymbol>()
val VAL_REASSIGNMENT_VIA_BACKING_FIELD by warning1<FirSourceElement, KtExpression, FirPropertySymbol>()
val VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR by error1<FirSourceElement, KtExpression, FirPropertySymbol>()
val WRONG_INVOCATION_KIND by warning3<FirSourceElement, PsiElement, AbstractFirBasedSymbol<*>, EventOccurrencesRange, EventOccurrencesRange>()
val LEAKED_IN_PLACE_LAMBDA by error1<FirSourceElement, PsiElement, AbstractFirBasedSymbol<*>>()
val WRONG_IMPLIES_CONDITION by warning0<FirSourceElement, PsiElement>()
@@ -5,9 +5,11 @@
package org.jetbrains.kotlin.fir.analysis.checkers.declaration
import org.jetbrains.kotlin.config.LanguageFeature
import org.jetbrains.kotlin.descriptors.ClassKind
import org.jetbrains.kotlin.descriptors.Modality
import org.jetbrains.kotlin.descriptors.Visibilities
import org.jetbrains.kotlin.fir.FirElement
import org.jetbrains.kotlin.fir.FirSourceElement
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.checkers.modality
@@ -16,7 +18,11 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn
import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertyAccessor
import org.jetbrains.kotlin.fir.expressions.FirVariableAssignment
import org.jetbrains.kotlin.fir.languageVersionSettings
import org.jetbrains.kotlin.fir.references.FirBackingFieldReference
import org.jetbrains.kotlin.fir.types.FirImplicitTypeRef
import org.jetbrains.kotlin.fir.visitors.FirVisitorVoid
import org.jetbrains.kotlin.lexer.KtTokens
internal fun isInsideExpectClass(containingClass: FirRegularClass, context: CheckerContext): Boolean {
@@ -168,8 +174,36 @@ internal fun checkPropertyAccessors(
reporter: DiagnosticReporter,
context: CheckerContext
) {
property.setter?.source?.let {
if (property.isVal) {
if (property.isVal) {
if (property.getter != null) {
var reassignment: FirBackingFieldReference? = null
// TODO: consider replacing this with normal VariableAssignmentChecker and reporting all 'field = ...' in getter (not just 1st)
// This way is a bit hacky and does not handle diagnostic suppression normally
val visitor = object : FirVisitorVoid() {
override fun visitElement(element: FirElement) {
element.acceptChildren(this)
}
override fun visitVariableAssignment(variableAssignment: FirVariableAssignment) {
// Report the first violation (to match FE 1.0 behavior)
if (reassignment != null) return
val backingFieldReference = variableAssignment.lValue as? FirBackingFieldReference
if (backingFieldReference?.resolvedSymbol?.fir == property) {
reassignment = backingFieldReference
}
}
}
property.getter?.body?.accept(visitor, null)
reassignment?.source?.let {
if (context.session.languageVersionSettings.supportsFeature(LanguageFeature.RestrictionOfValReassignmentViaBackingField)) {
reporter.reportOn(it, FirErrors.VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR, property.symbol, context)
} else {
reporter.reportOn(it, FirErrors.VAL_REASSIGNMENT_VIA_BACKING_FIELD, property.symbol, context)
}
}
}
property.setter?.source?.let {
reporter.reportOn(it, FirErrors.VAL_WITH_SETTER, context)
}
}
@@ -204,6 +204,8 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.USELESS_VARARG_ON
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VALUE_CLASS_CANNOT_BE_CLONEABLE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VALUE_PARAMETER_WITH_NO_TYPE_ANNOTATION
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAL_REASSIGNMENT
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAL_REASSIGNMENT_VIA_BACKING_FIELD
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAL_WITH_SETTER
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VARARG_OUTSIDE_PARENTHESES
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VARIABLE_EXPECTED
@@ -594,6 +596,8 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension {
// Control flow diagnostics
map.put(UNINITIALIZED_VARIABLE, "{0} must be initialized before access", PROPERTY_NAME)
map.put(VAL_REASSIGNMENT, "Val cannot be reassigned", PROPERTY_NAME)
map.put(VAL_REASSIGNMENT_VIA_BACKING_FIELD, "Reassignment of read-only property via backing field is deprecated", PROPERTY_NAME)
map.put(VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR, "Reassignment of read-only property via backing field", PROPERTY_NAME)
map.put(
WRONG_INVOCATION_KIND,
"{2} wrong invocation kind: given {3} case, but {4} case is possible",
@@ -1,7 +0,0 @@
// !LANGUAGE: +RestrictionOfValReassignmentViaBackingField
val my: Int = 1
get() {
field++
return field
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// !LANGUAGE: +RestrictionOfValReassignmentViaBackingField
val my: Int = 1
@@ -1,7 +0,0 @@
// !LANGUAGE: -RestrictionOfValReassignmentViaBackingField
val my: Int = 1
get() {
field++
return field
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// !LANGUAGE: -RestrictionOfValReassignmentViaBackingField
val my: Int = 1
@@ -0,0 +1,35 @@
// FIR_IDENTICAL
// !LANGUAGE: +RestrictionOfValReassignmentViaBackingField
class Outer {
val i: Int = 1
get() {
class Inner {
var i: Int = 2
get() {
field++
return field
}
val j: Int = 3
get() {
<!VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR!>field<!> = 42
return field
}
fun innerMember() {
<!VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR!>field<!>++
}
}
return field
}
val j: Int = 4
get() {
fun local() {
<!VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR!>field<!>++
field++
}
local()
return field
}
}
@@ -0,0 +1,10 @@
package
public final class Outer {
public constructor Outer()
public final val i: kotlin.Int = 1
public final val j: kotlin.Int = 4
public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
}
@@ -1,23 +0,0 @@
// !LANGUAGE: +RestrictionOfValReassignmentViaBackingField
package a
import java.util.HashSet
val a: MutableSet<String>? = null
get() {
if (a == null) {
field = HashSet()
}
return a
}
class R {
val b: String? = null
get() {
if (b == null) {
field = "b"
}
return b
}
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// !LANGUAGE: +RestrictionOfValReassignmentViaBackingField
package a
@@ -1,23 +0,0 @@
// !LANGUAGE: -RestrictionOfValReassignmentViaBackingField
package a
import java.util.HashSet
val a: MutableSet<String>? = null
get() {
if (a == null) {
field = HashSet()
}
return a
}
class R {
val b: String? = null
get() {
if (b == null) {
field = "b"
}
return b
}
}
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// !LANGUAGE: -RestrictionOfValReassignmentViaBackingField
package a
@@ -2097,6 +2097,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
runTest("compiler/testData/diagnostics/tests/backingField/kt782packageLevel.kt");
}
@Test
@TestMetadata("LocalDeclarations.kt")
public void testLocalDeclarations() throws Exception {
runTest("compiler/testData/diagnostics/tests/backingField/LocalDeclarations.kt");
}
@Test
@TestMetadata("SetterWithExplicitType.kt")
public void testSetterWithExplicitType() throws Exception {
@@ -1229,6 +1229,20 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert
token,
)
}
add(FirErrors.VAL_REASSIGNMENT_VIA_BACKING_FIELD) { firDiagnostic ->
ValReassignmentViaBackingFieldImpl(
firSymbolBuilder.buildVariableSymbol(firDiagnostic.a.fir as FirProperty),
firDiagnostic as FirPsiDiagnostic<*>,
token,
)
}
add(FirErrors.VAL_REASSIGNMENT_VIA_BACKING_FIELD_ERROR) { firDiagnostic ->
ValReassignmentViaBackingFieldErrorImpl(
firSymbolBuilder.buildVariableSymbol(firDiagnostic.a.fir as FirProperty),
firDiagnostic as FirPsiDiagnostic<*>,
token,
)
}
add(FirErrors.WRONG_INVOCATION_KIND) { firDiagnostic ->
WrongInvocationKindImpl(
firSymbolBuilder.buildSymbol(firDiagnostic.a.fir as FirDeclaration),
@@ -862,6 +862,16 @@ sealed class KtFirDiagnostic<PSI: PsiElement> : KtDiagnosticWithPsi<PSI> {
abstract val variable: KtVariableSymbol
}
abstract class ValReassignmentViaBackingField : KtFirDiagnostic<KtExpression>() {
override val diagnosticClass get() = ValReassignmentViaBackingField::class
abstract val variable: KtVariableSymbol
}
abstract class ValReassignmentViaBackingFieldError : KtFirDiagnostic<KtExpression>() {
override val diagnosticClass get() = ValReassignmentViaBackingFieldError::class
abstract val variable: KtVariableSymbol
}
abstract class WrongInvocationKind : KtFirDiagnostic<PsiElement>() {
override val diagnosticClass get() = WrongInvocationKind::class
abstract val declaration: KtSymbol
@@ -1403,6 +1403,22 @@ internal class ValReassignmentImpl(
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class ValReassignmentViaBackingFieldImpl(
override val variable: KtVariableSymbol,
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,
) : KtFirDiagnostic.ValReassignmentViaBackingField(), KtAbstractFirDiagnostic<KtExpression> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class ValReassignmentViaBackingFieldErrorImpl(
override val variable: KtVariableSymbol,
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,
) : KtFirDiagnostic.ValReassignmentViaBackingFieldError(), KtAbstractFirDiagnostic<KtExpression> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class WrongInvocationKindImpl(
override val declaration: KtSymbol,
override val requiredRange: EventOccurrencesRange,