azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

FIR checker/IDE: Add checker and quickfix for VAL_WITH_SETTER.

Browse Source
This commit is contained in:
Mark Punzalan
2021-02-16 23:34:10 +00:00
committed by Ilya Kirillov
parent 99c47a0487
commit 1c94372b6c
16 changed files with 116 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt
+1
View File
@@ -322,6 +322,7 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
val PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY by error<FirSourceElement, KtModifierListOwner>(PositioningStrategy.PRIVATE_MODIFIER)
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 VAL_WITH_SETTER by error<FirSourceElement, KtPropertyAccessor>()
}
val MPP_PROJECTS by object : DiagnosticGroup("Multi-platform projects") {
compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt
+1
View File
@@ -217,6 +217,7 @@ object FirErrors {
val PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY by error0<FirSourceElement, KtModifierListOwner>(SourceElementPositioningStrategies.PRIVATE_MODIFIER)
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 VAL_WITH_SETTER by error0<FirSourceElement, KtPropertyAccessor>()
// Multi-platform projects
val EXPECTED_DECLARATION_WITH_BODY by error0<FirSourceElement, KtDeclaration>(SourceElementPositioningStrategies.DECLARATION_SIGNATURE)
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirDeclarationCheckerUtils.kt
+8 -4
View File
@@ -41,16 +41,14 @@ internal fun checkExpectDeclarationVisibilityAndBody(
}
}
internal fun checkPropertyInitializer(
internal fun checkProperty(
containingClass: FirRegularClass?,
property: FirProperty,
modifierList: FirModifierList?,
reporter: DiagnosticReporter,
context: CheckerContext
) {
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) {
@@ -69,6 +67,12 @@ internal fun checkPropertyInitializer(
}
}
property.setter?.source?.let {
if (property.isVal) {
reporter.reportOn(it, FirErrors.VAL_WITH_SETTER, context)
}
}
val isExpect = property.isExpect || modifierList?.modifiers?.any { it.token == KtTokens.EXPECT_KEYWORD } == true
when {
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertyChecker.kt
+4 -4
View File
@@ -39,6 +39,10 @@ object FirMemberPropertyChecker : FirRegularClassChecker() {
// 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() }
checkProperty(containingDeclaration, property, modifierList, reporter, context)
checkExpectDeclarationVisibilityAndBody(property, source, modifierList, reporter, context)
val hasAbstractModifier = modifierList?.modifiers?.any { it.token == KtTokens.ABSTRACT_KEYWORD } == true
val isAbstract = property.isAbstract || hasAbstractModifier
if (containingDeclaration.isInterface &&
@@ -83,8 +87,6 @@ object FirMemberPropertyChecker : FirRegularClassChecker() {
}
}
checkPropertyInitializer(containingDeclaration, property, reporter, context)
val hasOpenModifier = modifierList?.modifiers?.any { it.token == KtTokens.OPEN_KEYWORD } == true
if (hasOpenModifier &&
containingDeclaration.isInterface &&
@@ -104,8 +106,6 @@ object FirMemberPropertyChecker : FirRegularClassChecker() {
}
}
}
checkExpectDeclarationVisibilityAndBody(property, source, modifierList, reporter, context)
}
private fun checkAccessor(
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertyChecker.kt
+3 -1
View File
@@ -24,9 +24,11 @@ object FirTopLevelPropertyChecker : FirFileChecker() {
private fun checkProperty(property: FirProperty, reporter: DiagnosticReporter, context: CheckerContext) {
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) { source.getModifierList() }
checkPropertyInitializer(null, property, reporter, context)
checkProperty(null, property, modifierList, reporter, context)
checkExpectDeclarationVisibilityAndBody(property, source, modifierList, reporter, context)
}
}
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt
+2
View File
@@ -170,6 +170,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNSAFE_OPERATOR_C
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UNUSED_VARIABLE
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.UPPER_BOUND_VIOLATED
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.USELESS_VARARG_ON_PARAMETER
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VAL_WITH_SETTER
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VARIABLE_EXPECTED
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VARIABLE_INITIALIZER_IS_REDUNDANT
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.VARIABLE_NEVER_READ
@@ -477,6 +478,7 @@ class FirDefaultErrorMessages : DefaultErrorMessages.Extension {
map.put(ABSTRACT_PROPERTY_WITH_SETTER, "Property with setter implementation cannot be abstract")
map.put(PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY, "Private setters are not allowed for abstract properties")
map.put(PRIVATE_SETTER_FOR_OPEN_PROPERTY, "Private setters are not allowed for open properties")
map.put(VAL_WITH_SETTER, "A 'val'-property cannot have a setter")
// Multi-platform projects
map.put(EXPECTED_DECLARATION_WITH_BODY, "Expected declaration must not have a body")
compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/converter/DeclarationsConverter.kt
+9 -9
View File
@@ -1017,16 +1017,16 @@ class DeclarationsConverter(
it.containingClassAttr = lookupTag
}
}
this.setter =
if (isVar) {
convertedAccessors.find { it.isSetter }
?: FirDefaultPropertySetter(
null, session, FirDeclarationOrigin.Source, returnType, propertyVisibility
).also {
currentDispatchReceiverType()?.lookupTag?.let { lookupTag ->
it.containingClassAttr = lookupTag
}
// NOTE: We still need the setter even for a val property so we can report errors (e.g., VAL_WITH_SETTER).
this.setter = convertedAccessors.find { it.isSetter }
?: if (isVar) {
FirDefaultPropertySetter(
null, session, FirDeclarationOrigin.Source, returnType, propertyVisibility
).also {
currentDispatchReceiverType()?.lookupTag?.let { lookupTag ->
it.containingClassAttr = lookupTag
}
}
} else null
// Upward propagation of `inline` and `external` modifiers (from accessors to property)
compiler/fir/raw-fir/psi2fir/src/org/jetbrains/kotlin/fir/builder/RawFirBuilder.kt
+66 -57
View File
@@ -334,7 +334,7 @@ class RawFirBuilder(
property: KtProperty,
propertyTypeRef: FirTypeRef,
isGetter: Boolean,
): FirPropertyAccessor {
): FirPropertyAccessor? {
val accessorVisibility =
if (this?.visibility != null && this.visibility != Visibilities.Unknown) this.visibility else property.visibility
// Downward propagation of `inline` and `external` modifiers (from property to its accessors)
@@ -345,67 +345,78 @@ class RawFirBuilder(
isExternal = property.hasModifier(EXTERNAL_KEYWORD) ||
this@toFirPropertyAccessor?.hasModifier(EXTERNAL_KEYWORD) == true
}
if (this == null || !hasBody()) {
val propertySource =
this?.toFirSourceElement() ?: property.toFirPsiSourceElement(FirFakeSourceElementKind.DefaultAccessor)
return FirDefaultPropertyAccessor
.createGetterOrSetter(
propertySource,
baseSession,
FirDeclarationOrigin.Source,
propertyTypeRef,
accessorVisibility,
isGetter
)
.also {
if (this != null) {
it.extractAnnotationsFrom(this)
return when {
this != null && hasBody() -> {
// Property has a non-default getter or setter.
// NOTE: We still need the setter even for a val property so we can report errors (e.g., VAL_WITH_SETTER).
val source = this.toFirSourceElement()
val accessorTarget = FirFunctionTarget(labelName = null, isLambda = false)
buildPropertyAccessor {
this.source = source
session = baseSession
origin = FirDeclarationOrigin.Source
returnTypeRef = if (isGetter) {
returnTypeReference?.convertSafe() ?: propertyTypeRef
} else {
returnTypeReference.toFirOrUnitType()
}
it.status = status
this.isGetter = isGetter
this.status = status
extractAnnotationsTo(this)
this@RawFirBuilder.context.firFunctionTargets += accessorTarget
extractValueParametersTo(this, propertyTypeRef)
if (!isGetter && valueParameters.isEmpty()) {
valueParameters += buildDefaultSetterValueParameter {
this.source = source.fakeElement(FirFakeSourceElementKind.DefaultAccessor)
session = baseSession
origin = FirDeclarationOrigin.Source
returnTypeRef = propertyTypeRef
symbol = FirVariableSymbol(NAME_FOR_DEFAULT_VALUE_PARAMETER)
}
}
symbol = FirPropertyAccessorSymbol()
val outerContractDescription = this@toFirPropertyAccessor.obtainContractDescription()
val bodyWithContractDescription = this@toFirPropertyAccessor.buildFirBody()
this.body = bodyWithContractDescription.first
val contractDescription = outerContractDescription ?: bodyWithContractDescription.second
contractDescription?.let {
this.contractDescription = it
}
}.also {
currentDispatchReceiverType()?.lookupTag?.let { lookupTag ->
it.containingClassAttr = lookupTag
}
}
}
val source = this.toFirSourceElement()
val accessorTarget = FirFunctionTarget(labelName = null, isLambda = false)
return buildPropertyAccessor {
this.source = source
session = baseSession
origin = FirDeclarationOrigin.Source
returnTypeRef = if (isGetter) {
returnTypeReference?.convertSafe() ?: propertyTypeRef
} else {
returnTypeReference.toFirOrUnitType()
}
this.isGetter = isGetter
this.status = status
extractAnnotationsTo(this)
this@RawFirBuilder.context.firFunctionTargets += accessorTarget
extractValueParametersTo(this, propertyTypeRef)
if (!isGetter && valueParameters.isEmpty()) {
valueParameters += buildDefaultSetterValueParameter {
this.source = source.fakeElement(FirFakeSourceElementKind.DefaultAccessor)
session = baseSession
origin = FirDeclarationOrigin.Source
returnTypeRef = propertyTypeRef
symbol = FirVariableSymbol(NAME_FOR_DEFAULT_VALUE_PARAMETER)
accessorTarget.bind(it)
this@RawFirBuilder.context.firFunctionTargets.removeLast()
}
}
symbol = FirPropertyAccessorSymbol()
val outerContractDescription = this@toFirPropertyAccessor.obtainContractDescription()
val bodyWithContractDescription = this@toFirPropertyAccessor.buildFirBody()
this.body = bodyWithContractDescription.first
val contractDescription = outerContractDescription ?: bodyWithContractDescription.second
contractDescription?.let {
this.contractDescription = it
isGetter || property.isVar -> {
// Default getter for val/var properties, and default setter for var properties.
val propertySource =
this?.toFirSourceElement() ?: property.toFirPsiSourceElement(FirFakeSourceElementKind.DefaultAccessor)
FirDefaultPropertyAccessor
.createGetterOrSetter(
propertySource,
baseSession,
FirDeclarationOrigin.Source,
propertyTypeRef,
accessorVisibility,
isGetter
)
.also {
if (this != null) {
it.extractAnnotationsFrom(this)
}
it.status = status
currentDispatchReceiverType()?.lookupTag?.let { lookupTag ->
it.containingClassAttr = lookupTag
}
}
}
}.also {
currentDispatchReceiverType()?.lookupTag?.let { lookupTag ->
it.containingClassAttr = lookupTag
else -> {
// No default setter for val properties.
null
}
accessorTarget.bind(it)
this@RawFirBuilder.context.firFunctionTargets.removeLast()
}
}
@@ -1268,9 +1279,7 @@ class RawFirBuilder(
} else null
getter = this@toFirProperty.getter.toFirPropertyAccessor(this@toFirProperty, propertyType, isGetter = true)
setter = if (isVar) {
this@toFirProperty.setter.toFirPropertyAccessor(this@toFirProperty, propertyType, isGetter = false)
} else null
setter = this@toFirProperty.setter.toFirPropertyAccessor(this@toFirProperty, propertyType, isGetter = false)
// Upward propagation of `inline` and `external` modifiers (from accessors to property)
// Note that, depending on `var` or `val`, checking setter's modifiers should be careful: for `val`, setter doesn't
compiler/testData/diagnostics/tests/Properties.fir.kt
Vendored
+1 -1
View File
@@ -7,7 +7,7 @@ var x : Int = 1 + x
val xx : Int = <!PROPERTY_INITIALIZER_NO_BACKING_FIELD!>1 + x<!>
get() : Int = 1
set(value : Long) {}
<!VAL_WITH_SETTER!>set(value : Long) {}<!>
val p : Int = <!PROPERTY_INITIALIZER_NO_BACKING_FIELD!>1<!>
get() = 1
idea/idea-completion/testData/basic/common/CompletionInSetter.kt
Vendored
+1
View File
@@ -1,3 +1,4 @@
// FIR_COMPARISON
val a: Int = 1
get() {
return field
idea/idea-fir/src/org/jetbrains/kotlin/idea/quickfix/MainKtQuickFixRegistrar.kt
+1
View File
@@ -48,6 +48,7 @@ class MainKtQuickFixRegistrar : KtQuickFixRegistrar() {
registerPsiQuickFixes(KtFirDiagnostic.VarOverriddenByVal::class, ChangeVariableMutabilityFix.VAR_OVERRIDDEN_BY_VAL_FACTORY)
registerPsiQuickFixes(KtFirDiagnostic.VarAnnotationParameter::class, ChangeVariableMutabilityFix.VAR_ANNOTATION_PARAMETER_FACTORY)
registerPsiQuickFixes(KtFirDiagnostic.InapplicableLateinitModifier::class, ChangeVariableMutabilityFix.LATEINIT_VAL_FACTORY)
registerPsiQuickFixes(KtFirDiagnostic.ValWithSetter::class, ChangeVariableMutabilityFix.VAL_WITH_SETTER_FACTORY)
}
override val list: KtQuickFixesList = KtQuickFixesList.createCombined(
idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt
+6
View File
@@ -944,6 +944,12 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert
token,
)
}
add(FirErrors.VAL_WITH_SETTER) { firDiagnostic ->
ValWithSetterImpl(
firDiagnostic as FirPsiDiagnostic<*>,
token,
)
}
add(FirErrors.EXPECTED_DECLARATION_WITH_BODY) { firDiagnostic ->
ExpectedDeclarationWithBodyImpl(
firDiagnostic as FirPsiDiagnostic<*>,
idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt
+4
View File
@@ -666,6 +666,10 @@ sealed class KtFirDiagnostic<PSI: PsiElement> : KtDiagnosticWithPsi<PSI> {
override val diagnosticClass get() = ExpectedPrivateDeclaration::class
}
abstract class ValWithSetter : KtFirDiagnostic<KtPropertyAccessor>() {
override val diagnosticClass get() = ValWithSetter::class
}
abstract class ExpectedDeclarationWithBody : KtFirDiagnostic<KtDeclaration>() {
override val diagnosticClass get() = ExpectedDeclarationWithBody::class
}
idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt
+7
View File
@@ -1075,6 +1075,13 @@ internal class ExpectedPrivateDeclarationImpl(
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class ValWithSetterImpl(
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,
) : KtFirDiagnostic.ValWithSetter(), KtAbstractFirDiagnostic<KtPropertyAccessor> {
override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic)
}
internal class ExpectedDeclarationWithBodyImpl(
firDiagnostic: FirPsiDiagnostic<*>,
override val token: ValidityToken,
idea/testData/quickfix/variables/changeMutability/valWithSetter.kt
Vendored
+1
View File
@@ -3,3 +3,4 @@ class A() {
val a: Int = 0
<caret>set(v: Int) {}
}
/* FIR_COMPARISON */
idea/testData/quickfix/variables/changeMutability/valWithSetter.kt.after
Vendored
+1
View File
@@ -3,3 +3,4 @@ class A() {
var a: Int = 0
set(v: Int) {}
}
/* FIR_COMPARISON */