From de03124f5028ee723fc66ad130dee2526b979cda Mon Sep 17 00:00:00 2001 From: Dmitriy Novozhilov Date: Tue, 16 Mar 2021 19:12:25 +0300 Subject: [PATCH] [FIR] Fix reporting EXPOSED_PROPERTY_TYPE_IN_CONSTRUCTOR warning --- .../diagnostics/FirDiagnosticsList.kt | 20 +++++++++++++++---- .../fir/analysis/diagnostics/FirErrors.kt | 1 + .../FirExposedVisibilityDeclarationChecker.kt | 7 ++++++- .../fir/lightTree/fir/ValueParameter.kt | 6 ++---- .../kotlin/fir/builder/RawFirBuilder.kt | 1 + .../fir/declarations/FirDeclarationUtil.kt | 3 ++- .../delegationWithPrivateConstructor.kt | 1 - .../propertyInPrivateConstructor.fir.kt | 2 +- .../propertyInSimpleConstructor.fir.kt | 2 +- .../diagnostics/tests/exposed/simple.fir.kt | 2 +- .../sealed/privateTypeInConstructor.fir.kt | 4 ++-- .../diagnostics/KtFirDataClassConverters.kt | 9 +++++++++ .../api/fir/diagnostics/KtFirDiagnostics.kt | 7 +++++++ .../fir/diagnostics/KtFirDiagnosticsImpl.kt | 10 ++++++++++ 14 files changed, 59 insertions(+), 16 deletions(-) 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 28956af4663..355a72d19cc 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 @@ -26,6 +26,8 @@ import org.jetbrains.kotlin.fir.types.ConeKotlinType import org.jetbrains.kotlin.lexer.KtModifierKeywordToken import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.psi.* +import kotlin.properties.PropertyDelegateProvider +import kotlin.properties.ReadOnlyProperty @Suppress("UNUSED_VARIABLE", "LocalVariableName", "ClassName", "unused") @@ -133,9 +135,9 @@ object DIAGNOSTICS_LIST : DiagnosticList() { val EXPOSED_VISIBILITY by object : DiagnosticGroup("Exposed visibility") { val EXPOSED_TYPEALIAS_EXPANDED_TYPE by exposedVisibilityError(PositioningStrategy.DECLARATION_NAME) val EXPOSED_FUNCTION_RETURN_TYPE by exposedVisibilityError(PositioningStrategy.DECLARATION_NAME) - val EXPOSED_RECEIVER_TYPE by exposedVisibilityError() val EXPOSED_PROPERTY_TYPE by exposedVisibilityError(PositioningStrategy.DECLARATION_NAME) + val EXPOSED_PROPERTY_TYPE_IN_CONSTRUCTOR by exposedVisibilityWarning(PositioningStrategy.DECLARATION_NAME) val EXPOSED_PARAMETER_TYPE by exposedVisibilityError(/* // NB: for parameter FE 1.0 reports not on a name for some reason */) val EXPOSED_SUPER_INTERFACE by exposedVisibilityError() val EXPOSED_SUPER_CLASS by exposedVisibilityError() @@ -508,10 +510,20 @@ object DIAGNOSTICS_LIST : DiagnosticList() { } } -private inline fun DiagnosticGroup.exposedVisibilityError( - positioningStrategy: PositioningStrategy = PositioningStrategy.DEFAULT -) = error(positioningStrategy) { +private val exposedVisibilityDiagnosticInit: DiagnosticBuilder.() -> Unit = { parameter("elementVisibility") parameter("restrictingDeclaration") parameter("restrictingVisibility") } + +private inline fun DiagnosticGroup.exposedVisibilityError( + positioningStrategy: PositioningStrategy = PositioningStrategy.DEFAULT +): PropertyDelegateProvider> { + return error(positioningStrategy, exposedVisibilityDiagnosticInit) +} + +private inline fun DiagnosticGroup.exposedVisibilityWarning( + positioningStrategy: PositioningStrategy = PositioningStrategy.DEFAULT +): PropertyDelegateProvider> { + return warning(positioningStrategy, exposedVisibilityDiagnosticInit) +} 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 eb8598e7be6..3ddf5ad6838 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 @@ -133,6 +133,7 @@ object FirErrors { val EXPOSED_FUNCTION_RETURN_TYPE by error3(SourceElementPositioningStrategies.DECLARATION_NAME) val EXPOSED_RECEIVER_TYPE by error3() val EXPOSED_PROPERTY_TYPE by error3(SourceElementPositioningStrategies.DECLARATION_NAME) + val EXPOSED_PROPERTY_TYPE_IN_CONSTRUCTOR by warning3(SourceElementPositioningStrategies.DECLARATION_NAME) val EXPOSED_PARAMETER_TYPE by error3() val EXPOSED_SUPER_INTERFACE by error3() val EXPOSED_SUPER_CLASS by error3() diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExposedVisibilityDeclarationChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExposedVisibilityDeclarationChecker.kt index de888202952..bb619f6e971 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExposedVisibilityDeclarationChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExposedVisibilityDeclarationChecker.kt @@ -149,9 +149,14 @@ object FirExposedVisibilityDeclarationChecker : FirMemberDeclarationChecker() { declaration.returnTypeRef.coneTypeSafe() ?.findVisibilityExposure(context, propertyVisibility) if (restricting != null) { + val diagnostic = if (declaration.fromPrimaryConstructor == true) { + FirErrors.EXPOSED_PROPERTY_TYPE_IN_CONSTRUCTOR + } else { + FirErrors.EXPOSED_PROPERTY_TYPE + } reporter.reportOn( declaration.source, - FirErrors.EXPOSED_PROPERTY_TYPE, + diagnostic, propertyVisibility, restricting, restricting.getEffectiveVisibility(context), diff --git a/compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/fir/ValueParameter.kt b/compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/fir/ValueParameter.kt index fad51108fcd..6929a44f261 100644 --- a/compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/fir/ValueParameter.kt +++ b/compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/fir/ValueParameter.kt @@ -6,14 +6,11 @@ package org.jetbrains.kotlin.fir.lightTree.fir import org.jetbrains.kotlin.fir.* -import org.jetbrains.kotlin.fir.declarations.FirDeclarationOrigin -import org.jetbrains.kotlin.fir.declarations.FirProperty -import org.jetbrains.kotlin.fir.declarations.FirValueParameter +import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.builder.buildProperty import org.jetbrains.kotlin.fir.declarations.impl.FirDeclarationStatusImpl import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertyGetter import org.jetbrains.kotlin.fir.declarations.impl.FirDefaultPropertySetter -import org.jetbrains.kotlin.fir.declarations.isFromVararg import org.jetbrains.kotlin.fir.diagnostics.DiagnosticKind import org.jetbrains.kotlin.fir.diagnostics.ConeSimpleDiagnostic import org.jetbrains.kotlin.fir.expressions.builder.buildQualifiedAccessExpression @@ -88,6 +85,7 @@ class ValueParameter( if (firValueParameter.isVararg) { this.isFromVararg = true } + this.fromPrimaryConstructor = true } } } diff --git a/compiler/fir/raw-fir/psi2fir/src/org/jetbrains/kotlin/fir/builder/RawFirBuilder.kt b/compiler/fir/raw-fir/psi2fir/src/org/jetbrains/kotlin/fir/builder/RawFirBuilder.kt index cdb796dca85..1d46cda0d1a 100644 --- a/compiler/fir/raw-fir/psi2fir/src/org/jetbrains/kotlin/fir/builder/RawFirBuilder.kt +++ b/compiler/fir/raw-fir/psi2fir/src/org/jetbrains/kotlin/fir/builder/RawFirBuilder.kt @@ -466,6 +466,7 @@ open class RawFirBuilder( if (firParameter.isVararg) { isFromVararg = true } + fromPrimaryConstructor = true } } diff --git a/compiler/fir/tree/src/org/jetbrains/kotlin/fir/declarations/FirDeclarationUtil.kt b/compiler/fir/tree/src/org/jetbrains/kotlin/fir/declarations/FirDeclarationUtil.kt index 70ea2df64fd..21007aa16c2 100644 --- a/compiler/fir/tree/src/org/jetbrains/kotlin/fir/declarations/FirDeclarationUtil.kt +++ b/compiler/fir/tree/src/org/jetbrains/kotlin/fir/declarations/FirDeclarationUtil.kt @@ -177,11 +177,12 @@ val FirMemberDeclaration.containerSource: SourceElement? } private object IsFromVarargKey : FirDeclarationDataKey() - private object IsReferredViaField : FirDeclarationDataKey() +private object IsFromPrimaryConstructor : FirDeclarationDataKey() var FirProperty.isFromVararg: Boolean? by FirDeclarationDataRegistry.data(IsFromVarargKey) var FirProperty.isReferredViaField: Boolean? by FirDeclarationDataRegistry.data(IsReferredViaField) +var FirProperty.fromPrimaryConstructor: Boolean? by FirDeclarationDataRegistry.data(IsFromPrimaryConstructor) // See [BindingContext.BACKING_FIELD_REQUIRED] val FirProperty.hasBackingField: Boolean diff --git a/compiler/testData/codegen/box/delegation/delegationWithPrivateConstructor.kt b/compiler/testData/codegen/box/delegation/delegationWithPrivateConstructor.kt index bcfd8a9132a..0fcef72f872 100644 --- a/compiler/testData/codegen/box/delegation/delegationWithPrivateConstructor.kt +++ b/compiler/testData/codegen/box/delegation/delegationWithPrivateConstructor.kt @@ -1,4 +1,3 @@ -// IGNORE_FIR_DIAGNOSTICS class MyObject private constructor(val delegate: Interface) : Interface by delegate { constructor() : this(Delegate()) } diff --git a/compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.fir.kt b/compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.fir.kt index b4c6e1e296d..e92aa5799ba 100644 --- a/compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.fir.kt +++ b/compiler/testData/diagnostics/tests/exposed/propertyInPrivateConstructor.fir.kt @@ -1,3 +1,3 @@ private enum class Foo { A, B } -class Bar private constructor(val foo: Foo) \ No newline at end of file +class Bar private constructor(val foo: Foo) diff --git a/compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.fir.kt b/compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.fir.kt index fba4dc687d6..334d70c3a02 100644 --- a/compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.fir.kt +++ b/compiler/testData/diagnostics/tests/exposed/propertyInSimpleConstructor.fir.kt @@ -1,3 +1,3 @@ private enum class Foo { A, B } -class Bar(val foo: Foo) \ No newline at end of file +class Bar(val foo: Foo) diff --git a/compiler/testData/diagnostics/tests/exposed/simple.fir.kt b/compiler/testData/diagnostics/tests/exposed/simple.fir.kt index 0112ddeec17..5519e569f24 100644 --- a/compiler/testData/diagnostics/tests/exposed/simple.fir.kt +++ b/compiler/testData/diagnostics/tests/exposed/simple.fir.kt @@ -6,7 +6,7 @@ public interface Your: My { fun foo(): T } -public class DerivedMy>(val x: My): Base() { +public class DerivedMy>(val x: My): Base() { constructor(xx: My?, x: My): this(xx ?: x) diff --git a/compiler/testData/diagnostics/tests/sealed/privateTypeInConstructor.fir.kt b/compiler/testData/diagnostics/tests/sealed/privateTypeInConstructor.fir.kt index 79e080e9b09..f8cebb13a70 100644 --- a/compiler/testData/diagnostics/tests/sealed/privateTypeInConstructor.fir.kt +++ b/compiler/testData/diagnostics/tests/sealed/privateTypeInConstructor.fir.kt @@ -4,13 +4,13 @@ private class Bar sealed class SealedFoo( - val x: Bar, + val x: Bar, private val y: Bar, z: Bar ) abstract class AbstractFoo( - val x: Bar, + val x: Bar, private val y: Bar, z: Bar ) diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt index 16d4637f034..f7093066fa6 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDataClassConverters.kt @@ -457,6 +457,15 @@ internal val KT_DIAGNOSTIC_CONVERTER = KtDiagnosticConverterBuilder.buildConvert token, ) } + add(FirErrors.EXPOSED_PROPERTY_TYPE_IN_CONSTRUCTOR) { firDiagnostic -> + ExposedPropertyTypeInConstructorImpl( + firDiagnostic.a.toVisibility(), + firSymbolBuilder.buildSymbol(firDiagnostic.b as FirDeclaration), + firDiagnostic.c.toVisibility(), + firDiagnostic as FirPsiDiagnostic<*>, + token, + ) + } add(FirErrors.EXPOSED_PARAMETER_TYPE) { firDiagnostic -> ExposedParameterTypeImpl( firDiagnostic.a.toVisibility(), diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt index ae64f1a3794..c9c81b6966c 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnostics.kt @@ -330,6 +330,13 @@ sealed class KtFirDiagnostic : KtDiagnosticWithPsi { abstract val restrictingVisibility: Visibility } + abstract class ExposedPropertyTypeInConstructor : KtFirDiagnostic() { + override val diagnosticClass get() = ExposedPropertyTypeInConstructor::class + abstract val elementVisibility: Visibility + abstract val restrictingDeclaration: KtSymbol + abstract val restrictingVisibility: Visibility + } + abstract class ExposedParameterType : KtFirDiagnostic() { override val diagnosticClass get() = ExposedParameterType::class abstract val elementVisibility: Visibility diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt index 142aded74fd..bea8645439b 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/diagnostics/KtFirDiagnosticsImpl.kt @@ -529,6 +529,16 @@ internal class ExposedPropertyTypeImpl( override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic) } +internal class ExposedPropertyTypeInConstructorImpl( + override val elementVisibility: Visibility, + override val restrictingDeclaration: KtSymbol, + override val restrictingVisibility: Visibility, + firDiagnostic: FirPsiDiagnostic<*>, + override val token: ValidityToken, +) : KtFirDiagnostic.ExposedPropertyTypeInConstructor(), KtAbstractFirDiagnostic { + override val firDiagnostic: FirPsiDiagnostic<*> by weakRef(firDiagnostic) +} + internal class ExposedParameterTypeImpl( override val elementVisibility: Visibility, override val restrictingDeclaration: KtSymbol,