FIR: fix position and reporting of PRIVATE_SETTER_*

Previously `FirPropertyAccessor.source` references the `KtProperty` if
the user code contains a setter or getter that doesn't contain a body.
For example, with the following

```
val i: Int = 1
  private set
```

The `FirPropertyAccessor` would reference the `KtProperty` as the
source.

This change makes `FirPropertyAccessor` reference `KtPropertyAccessor`
as the source if possible, regardless of whether the body is present or
not.
This commit is contained in:
Tianyu Geng
2021-02-18 16:28:27 -08:00
committed by Mikhail Glukhikh
parent bdeecfc188
commit 5bdea9652b
14 changed files with 56 additions and 55 deletions
@@ -44,6 +44,7 @@ enum class PositioningStrategy(private val strategy: String) {
LATEINIT_MODIFIER("LATEINIT_MODIFIER"), LATEINIT_MODIFIER("LATEINIT_MODIFIER"),
SELECTOR_BY_QUALIFIED("SELECTOR_BY_QUALIFIED"), SELECTOR_BY_QUALIFIED("SELECTOR_BY_QUALIFIED"),
REFERENCE_BY_QUALIFIED("REFERENCE_BY_QUALIFIED"), REFERENCE_BY_QUALIFIED("REFERENCE_BY_QUALIFIED"),
PRIVATE_MODIFIER("PRIVATE_MODIFIER"),
; ;
@@ -319,8 +319,8 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
val ABSTRACT_PROPERTY_WITH_GETTER by error<FirSourceElement, KtPropertyAccessor>() val ABSTRACT_PROPERTY_WITH_GETTER by error<FirSourceElement, KtPropertyAccessor>()
val ABSTRACT_PROPERTY_WITH_SETTER by error<FirSourceElement, KtPropertyAccessor>() val ABSTRACT_PROPERTY_WITH_SETTER by error<FirSourceElement, KtPropertyAccessor>()
val PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY by error<FirSourceElement, PsiElement>() val PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY by error<FirSourceElement, KtModifierListOwner>(PositioningStrategy.PRIVATE_MODIFIER)
val PRIVATE_SETTER_FOR_OPEN_PROPERTY by error<FirSourceElement, PsiElement>() val PRIVATE_SETTER_FOR_OPEN_PROPERTY by error<FirSourceElement, KtModifierListOwner>(PositioningStrategy.PRIVATE_MODIFIER)
val EXPECTED_PRIVATE_DECLARATION by error<FirSourceElement, KtModifierListOwner>(PositioningStrategy.VISIBILITY_MODIFIER) val EXPECTED_PRIVATE_DECLARATION by error<FirSourceElement, KtModifierListOwner>(PositioningStrategy.VISIBILITY_MODIFIER)
} }
@@ -214,8 +214,8 @@ object FirErrors {
val DELEGATED_PROPERTY_IN_INTERFACE by error0<FirSourceElement, KtPropertyDelegate>() val DELEGATED_PROPERTY_IN_INTERFACE by error0<FirSourceElement, KtPropertyDelegate>()
val ABSTRACT_PROPERTY_WITH_GETTER by error0<FirSourceElement, KtPropertyAccessor>() val ABSTRACT_PROPERTY_WITH_GETTER by error0<FirSourceElement, KtPropertyAccessor>()
val ABSTRACT_PROPERTY_WITH_SETTER by error0<FirSourceElement, KtPropertyAccessor>() val ABSTRACT_PROPERTY_WITH_SETTER by error0<FirSourceElement, KtPropertyAccessor>()
val PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY by error0<FirSourceElement, PsiElement>() val PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.PRIVATE_MODIFIER)
val PRIVATE_SETTER_FOR_OPEN_PROPERTY by error0<FirSourceElement, PsiElement>() val PRIVATE_SETTER_FOR_OPEN_PROPERTY by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.PRIVATE_MODIFIER)
val EXPECTED_PRIVATE_DECLARATION by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER) val EXPECTED_PRIVATE_DECLARATION by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
// Multi-platform projects // Multi-platform projects
@@ -5,12 +5,13 @@
package org.jetbrains.kotlin.fir.analysis.checkers.declaration package org.jetbrains.kotlin.fir.analysis.checkers.declaration
import org.jetbrains.kotlin.fir.FirFakeSourceElementKind
import org.jetbrains.kotlin.fir.FirSourceElement import org.jetbrains.kotlin.fir.FirSourceElement
import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
import org.jetbrains.kotlin.fir.declarations.FirProperty import org.jetbrains.kotlin.fir.declarations.FirProperty
import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertyGetter
import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertySetter
import org.jetbrains.kotlin.fir.declarations.isLateInit import org.jetbrains.kotlin.fir.declarations.isLateInit
import org.jetbrains.kotlin.fir.types.ConeKotlinType import org.jetbrains.kotlin.fir.types.ConeKotlinType
import org.jetbrains.kotlin.fir.types.ConeTypeParameterType import org.jetbrains.kotlin.fir.types.ConeTypeParameterType
@@ -71,8 +72,8 @@ object FirInapplicableLateinitChecker : FirPropertyChecker() {
else -> type.isNullable else -> type.isNullable
} }
private fun FirProperty.hasGetter() = getter != null && getter?.source != null && getter?.source?.kind !is FirFakeSourceElementKind private fun FirProperty.hasGetter() = getter != null && getter !is FirDefaultPropertyGetter
private fun FirProperty.hasSetter() = setter != null && setter?.source != null && setter?.source?.kind !is FirFakeSourceElementKind private fun FirProperty.hasSetter() = setter != null && setter !is FirDefaultPropertySetter
private fun DiagnosticReporter.reportOn(source: FirSourceElement?, target: String, context: CheckerContext) { private fun DiagnosticReporter.reportOn(source: FirSourceElement?, target: String, context: CheckerContext) {
source?.let { report(FirErrors.INAPPLICABLE_LATEINIT_MODIFIER.on(it, target), context) } source?.let { report(FirErrors.INAPPLICABLE_LATEINIT_MODIFIER.on(it, target), context) }
@@ -71,14 +71,14 @@ object FirMemberPropertyChecker : FirRegularClassChecker() {
reporter.reportOn(it, FirErrors.ABSTRACT_DELEGATED_PROPERTY, context) reporter.reportOn(it, FirErrors.ABSTRACT_DELEGATED_PROPERTY, context)
} }
checkAccessor(property.getter, property.delegate) { src, _ -> checkAccessor(property.getter, property.delegate) { src, _, hasBody ->
reporter.reportOn(src, FirErrors.ABSTRACT_PROPERTY_WITH_GETTER, context) if (hasBody) reporter.reportOn(src, FirErrors.ABSTRACT_PROPERTY_WITH_GETTER, context)
} }
checkAccessor(property.setter, property.delegate) { src, symbol -> checkAccessor(property.setter, property.delegate) { src, symbol, hasBody ->
if (symbol.fir.visibility == Visibilities.Private && property.visibility != Visibilities.Private) { when {
reporter.reportOn(src, FirErrors.PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY, context) symbol.fir.visibility == Visibilities.Private && property.visibility != Visibilities.Private ->
} else { reporter.reportOn(src, FirErrors.PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY, context)
reporter.reportOn(src, FirErrors.ABSTRACT_PROPERTY_WITH_SETTER, context) hasBody -> reporter.reportOn(src, FirErrors.ABSTRACT_PROPERTY_WITH_SETTER, context)
} }
} }
} }
@@ -98,7 +98,7 @@ object FirMemberPropertyChecker : FirRegularClassChecker() {
} }
val isOpen = property.isOpen || hasOpenModifier val isOpen = property.isOpen || hasOpenModifier
if (isOpen) { if (isOpen) {
checkAccessor(property.setter, property.delegate) { src, symbol -> checkAccessor(property.setter, property.delegate) { src, symbol, _ ->
if (symbol.fir.visibility == Visibilities.Private && property.visibility != Visibilities.Private) { if (symbol.fir.visibility == Visibilities.Private && property.visibility != Visibilities.Private) {
reporter.reportOn(src, FirErrors.PRIVATE_SETTER_FOR_OPEN_PROPERTY, context) reporter.reportOn(src, FirErrors.PRIVATE_SETTER_FOR_OPEN_PROPERTY, context)
} }
@@ -111,11 +111,11 @@ object FirMemberPropertyChecker : FirRegularClassChecker() {
private fun checkAccessor( private fun checkAccessor(
accessor: FirPropertyAccessor?, accessor: FirPropertyAccessor?,
delegate: FirExpression?, delegate: FirExpression?,
report: (FirSourceElement, FirPropertyAccessorSymbol) -> Unit, report: (FirSourceElement, FirPropertyAccessorSymbol, hasBody: Boolean) -> Unit,
) { ) {
if (accessor != null && accessor !is FirDefaultPropertyAccessor && accessor.hasBody && delegate == null) { if (accessor != null && delegate == null) {
accessor.source?.let { accessor.source?.let {
report.invoke(it, accessor.symbol) report.invoke(it, accessor.symbol, accessor.hasBody)
} }
} }
} }
@@ -347,8 +347,7 @@ class RawFirBuilder(
} }
if (this == null || !hasBody()) { if (this == null || !hasBody()) {
val propertySource = val propertySource =
if (this != null && hasBody()) property.toFirSourceElement() this?.toFirSourceElement() ?: property.toFirPsiSourceElement(FirFakeSourceElementKind.DefaultAccessor)
else property.toFirPsiSourceElement(FirFakeSourceElementKind.DefaultAccessor)
return FirDefaultPropertyAccessor return FirDefaultPropertyAccessor
.createGetterOrSetter( .createGetterOrSetter(
propertySource, propertySource,
@@ -1,14 +1,14 @@
abstract class My { abstract class My {
abstract var x: Int abstract var x: Int
public get public get
private set <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
abstract val y: Int abstract val y: Int
protected get protected get
abstract protected var z: Int abstract protected var z: Int
internal get internal get
private set <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
abstract internal val w: Int abstract internal val w: Int
protected get protected get
@@ -17,5 +17,5 @@ abstract class My {
protected set protected set
open var t: Int = 0 open var t: Int = 0
private set <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set
} }
@@ -9,8 +9,8 @@ abstract class MyAbstractClass() {
var b: Int private set var b: Int private set
var b1: Int = 0; private set var b1: Int = 0; private set
abstract var b2: Int private set abstract var b2: Int <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
abstract var b3: Int = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>0<!>; private set abstract var b3: Int = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>0<!>; <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
var c: Int set(v: Int) { field = v } var c: Int set(v: Int) { field = v }
var c1: Int = 0; set(v: Int) { field = v } var c1: Int = 0; set(v: Int) { field = v }
+4 -4
View File
@@ -7,10 +7,10 @@ interface MyTrait {
abstract val a2: Int abstract val a2: Int
abstract val a3: Int = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>1<!> abstract val a3: Int = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>1<!>
var b: Int private set var b: Int <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
var b1: Int = <!PROPERTY_INITIALIZER_IN_INTERFACE!>0<!>; private set var b1: Int = <!PROPERTY_INITIALIZER_IN_INTERFACE!>0<!>; <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set
abstract var b2: Int private set abstract var b2: Int <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
abstract var b3: Int = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>0<!>; private set abstract var b3: Int = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>0<!>; <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
<!BACKING_FIELD_IN_INTERFACE!>var c: Int<!> set(v: Int) { field = v } <!BACKING_FIELD_IN_INTERFACE!>var c: Int<!> set(v: Int) { field = v }
<!BACKING_FIELD_IN_INTERFACE!>var c1: Int<!> = <!PROPERTY_INITIALIZER_IN_INTERFACE!>0<!>; set(v: Int) { field = v } <!BACKING_FIELD_IN_INTERFACE!>var c1: Int<!> = <!PROPERTY_INITIALIZER_IN_INTERFACE!>0<!>; set(v: Int) { field = v }
@@ -31,40 +31,40 @@ class B : A, AA() {
open class C : A, AA() { open class C : A, AA() {
override var a: Int = 0 override var a: Int = 0
// Errors here and below // Errors here and below
private set <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set
override var b: Int = 1 override var b: Int = 1
private set <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set
override var c: Int = 2 override var c: Int = 2
private set <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set
override var d: Int = 3 override var d: Int = 3
private set <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set
} }
abstract class D : A, AA() { abstract class D : A, AA() {
override var a: Int = 0 override var a: Int = 0
// Errors here and below // Errors here and below
private set <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set
override var b: Int = 1 override var b: Int = 1
private set <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set
override var c: Int = 2 override var c: Int = 2
private set <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set
override var d: Int = 3 override var d: Int = 3
private set <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set
} }
interface E : A { interface E : A {
override var a: Int override var a: Int
get() = 0 get() = 0
// Errors here and below // Errors here and below
<!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private set(arg) {}<!> <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set(arg) {}
override var b: Int override var b: Int
get() = 0 get() = 0
<!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private set(arg) {}<!> <!PRIVATE_SETTER_FOR_OPEN_PROPERTY!>private<!> set(arg) {}
} }
@@ -11,8 +11,8 @@ enum class MyEnum() {
var b: Int private set var b: Int private set
var b1: Int = 0; private set var b1: Int = 0; private set
abstract var b2: Int private set abstract var b2: Int <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
abstract var b3: Int = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>0<!>; private set abstract var b3: Int = <!ABSTRACT_PROPERTY_WITH_INITIALIZER!>0<!>; <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
var c: Int set(v: Int) { field = v } var c: Int set(v: Int) { field = v }
var c1: Int = 0; set(v: Int) { field = v } var c1: Int = 0; set(v: Int) { field = v }
@@ -5,11 +5,11 @@ interface Test<in I, out O> {
<!PRIVATE_PROPERTY_IN_INTERFACE!>private<!> val private_val: I <!PRIVATE_PROPERTY_IN_INTERFACE!>private<!> val private_val: I
var interlan_private_set: O var interlan_private_set: O
private set <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
public var public_private_set: O public var public_private_set: O
private set <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
protected var protected_private_set: O protected var protected_private_set: O
private set <!PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY!>private<!> set
<!PRIVATE_PROPERTY_IN_INTERFACE!>private<!> var private_private_set: O <!PRIVATE_PROPERTY_IN_INTERFACE!>private<!> var private_private_set: O
private set private set
@@ -653,11 +653,11 @@ sealed class KtFirDiagnostic<PSI: PsiElement> : KtDiagnosticWithPsi<PSI> {
override val diagnosticClass get() = AbstractPropertyWithSetter::class override val diagnosticClass get() = AbstractPropertyWithSetter::class
} }
abstract class PrivateSetterForAbstractProperty : KtFirDiagnostic<PsiElement>() { abstract class PrivateSetterForAbstractProperty : KtFirDiagnostic<KtModifierListOwner>() {
override val diagnosticClass get() = PrivateSetterForAbstractProperty::class override val diagnosticClass get() = PrivateSetterForAbstractProperty::class
} }
abstract class PrivateSetterForOpenProperty : KtFirDiagnostic<PsiElement>() { abstract class PrivateSetterForOpenProperty : KtFirDiagnostic<KtModifierListOwner>() {
override val diagnosticClass get() = PrivateSetterForOpenProperty::class override val diagnosticClass get() = PrivateSetterForOpenProperty::class
} }
@@ -1056,14 +1056,14 @@ internal class AbstractPropertyWithSetterImpl(
internal class PrivateSetterForAbstractPropertyImpl( internal class PrivateSetterForAbstractPropertyImpl(
firDiagnostic: FirPsiDiagnostic<*>, firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken, override val token: ValidityToken,
) : KtFirDiagnostic.PrivateSetterForAbstractProperty(), KtAbstractFirDiagnostic<PsiElement> { ) : KtFirDiagnostic.PrivateSetterForAbstractProperty(), KtAbstractFirDiagnostic<KtModifierListOwner> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic) override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
} }
internal class PrivateSetterForOpenPropertyImpl( internal class PrivateSetterForOpenPropertyImpl(
firDiagnostic: FirPsiDiagnostic<*>, firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken, override val token: ValidityToken,
) : KtFirDiagnostic.PrivateSetterForOpenProperty(), KtAbstractFirDiagnostic<PsiElement> { ) : KtFirDiagnostic.PrivateSetterForOpenProperty(), KtAbstractFirDiagnostic<KtModifierListOwner> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic) override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
} }