From b46d4e53995e03608a4bb56e7648de4a954a4814 Mon Sep 17 00:00:00 2001 From: Nikolay Lunyak Date: Wed, 28 Jun 2023 16:02:02 +0300 Subject: [PATCH] [FIR] Check classes for `EXPECTED_PRIVATE_DECLARATION` --- .../checkers/CommonDeclarationCheckers.kt | 2 +- .../FirExpectConsistencyChecker.kt | 45 +++++++++++++++++++ .../declaration/FirMemberFunctionsChecker.kt | 2 - .../declaration/FirMemberPropertiesChecker.kt | 1 - .../FirTopLevelFunctionsChecker.kt | 4 -- .../FirTopLevelPropertiesChecker.kt | 18 -------- .../headerClass/privateMembers.fir.kt | 2 +- .../privateTopLevelDeclarations.fir.kt | 2 +- 8 files changed, 48 insertions(+), 28 deletions(-) create mode 100644 compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExpectConsistencyChecker.kt diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt index a79f2a69903..d1bba9c7371 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonDeclarationCheckers.kt @@ -8,7 +8,6 @@ package org.jetbrains.kotlin.fir.analysis.checkers import org.jetbrains.kotlin.fir.analysis.cfa.AbstractFirPropertyInitializationChecker import org.jetbrains.kotlin.fir.analysis.cfa.FirCallsEffectAnalyzer import org.jetbrains.kotlin.fir.analysis.cfa.FirPropertyInitializationAnalyzer -import org.jetbrains.kotlin.fir.analysis.cfa.FirReturnsImpliesAnalyzer import org.jetbrains.kotlin.fir.analysis.checkers.cfa.FirControlFlowChecker import org.jetbrains.kotlin.fir.analysis.checkers.declaration.* import org.jetbrains.kotlin.fir.analysis.checkers.expression.FirAnonymousFunctionParametersChecker @@ -32,6 +31,7 @@ object CommonDeclarationCheckers : DeclarationCheckers() { FirAnnotationChecker, FirPublishedApiChecker, FirOptInMarkedDeclarationChecker, + FirExpectConsistencyChecker, ) override val callableDeclarationCheckers: Set diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExpectConsistencyChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExpectConsistencyChecker.kt new file mode 100644 index 00000000000..6bce6d60c91 --- /dev/null +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExpectConsistencyChecker.kt @@ -0,0 +1,45 @@ +/* + * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.fir.analysis.checkers.declaration + +import org.jetbrains.kotlin.KtFakeSourceElementKind +import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.diagnostics.DiagnosticReporter +import org.jetbrains.kotlin.diagnostics.reportOn +import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors +import org.jetbrains.kotlin.fir.declarations.* +import org.jetbrains.kotlin.fir.declarations.utils.hasBody +import org.jetbrains.kotlin.fir.declarations.utils.isExpect +import org.jetbrains.kotlin.fir.declarations.utils.visibility + +// See old FE's [DeclarationsChecker] +object FirExpectConsistencyChecker : FirBasicDeclarationChecker() { + override fun check(declaration: FirDeclaration, context: CheckerContext, reporter: DiagnosticReporter) { + val isTopLevel = context.containingDeclarations.size == 1 + val isInsideClass = context.containingDeclarations.lastOrNull() is FirClass + + if ( + declaration !is FirMemberDeclaration || + !isTopLevel && !isInsideClass || + !declaration.isExpect || + declaration is FirConstructor + ) { + return + } + + val source = declaration.source ?: return + if (source.kind is KtFakeSourceElementKind) return + + if (Visibilities.isPrivate(declaration.visibility)) { + reporter.reportOn(source, FirErrors.EXPECTED_PRIVATE_DECLARATION, context) + } + + if (declaration is FirSimpleFunction && declaration.hasBody) { + reporter.reportOn(source, FirErrors.EXPECTED_DECLARATION_WITH_BODY, context) + } + } +} diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberFunctionsChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberFunctionsChecker.kt index a717fe7d343..a6fd546e184 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberFunctionsChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberFunctionsChecker.kt @@ -70,7 +70,5 @@ object FirMemberFunctionsChecker : FirSimpleFunctionChecker() { reporter.reportOn(source, FirErrors.NON_ABSTRACT_FUNCTION_WITH_NO_BODY, functionSymbol, context) } } - - checkExpectDeclarationVisibilityAndBody(function, source, reporter, context) } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt index be3a58bef2e..2ea3e8b984a 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt @@ -79,7 +79,6 @@ object FirMemberPropertiesChecker : FirClassChecker() { context, reachable ) - checkExpectDeclarationVisibilityAndBody(property, source, reporter, context) val hasAbstractModifier = KtTokens.ABSTRACT_KEYWORD in modifierList val isAbstract = property.isAbstract || hasAbstractModifier diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelFunctionsChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelFunctionsChecker.kt index b1005a21efc..d10c1ceec67 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelFunctionsChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelFunctionsChecker.kt @@ -29,8 +29,6 @@ object FirTopLevelFunctionsChecker : FirSimpleFunctionChecker() { val source = declaration.source ?: return if (source.kind is KtFakeSourceElementKind) 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. if (declaration.hasModifier(KtTokens.ABSTRACT_KEYWORD)) return if (declaration.isExternal) return if (!declaration.hasBody && @@ -39,7 +37,5 @@ object FirTopLevelFunctionsChecker : FirSimpleFunctionChecker() { ) { reporter.reportOn(source, FirErrors.NON_MEMBER_FUNCTION_NO_BODY, declaration.symbol, context) } - - checkExpectDeclarationVisibilityAndBody(declaration, source, reporter, context) } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt index 12af0abc436..1c096495365 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt @@ -46,24 +46,6 @@ object FirTopLevelPropertiesChecker : FirPropertyChecker() { reporter, context ) - checkExpectDeclarationVisibilityAndBody(declaration, source, reporter, context) - } -} - -// TODO: check class too -internal fun checkExpectDeclarationVisibilityAndBody( - declaration: FirMemberDeclaration, - source: KtSourceElement, - reporter: DiagnosticReporter, - context: CheckerContext -) { - if (declaration.isExpect) { - if (Visibilities.isPrivate(declaration.visibility)) { - reporter.reportOn(source, FirErrors.EXPECTED_PRIVATE_DECLARATION, context) - } - if (declaration is FirSimpleFunction && declaration.hasBody) { - reporter.reportOn(source, FirErrors.EXPECTED_DECLARATION_WITH_BODY, context) - } } } diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/privateMembers.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/privateMembers.fir.kt index 4104850d04a..5ffa533791a 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/privateMembers.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/privateMembers.fir.kt @@ -6,7 +6,7 @@ expect class A { private val bar: String private fun Int.memExt(): Any - private class Nested + private class Nested } // MODULE: m1-jvm()()(m1-common) diff --git a/compiler/testData/diagnostics/tests/multiplatform/privateTopLevelDeclarations.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/privateTopLevelDeclarations.fir.kt index 2ed710c64ae..76c53634f9f 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/privateTopLevelDeclarations.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/privateTopLevelDeclarations.fir.kt @@ -5,7 +5,7 @@ private expect val bar: String private expect fun Int.memExt(): Any -private expect class Foo +private expect class Foo // MODULE: m2-jvm()()(m1-common) // FILE: jvm.kt