From 0d19942d2c146eaf7ef029a0424d7d1bf79301ef Mon Sep 17 00:00:00 2001 From: Nikolay Lunyak Date: Wed, 30 Aug 2023 15:54:11 +0300 Subject: [PATCH] [FIR] Make reporting of local class diagnostics more consistent with K1 It would be nice to report more appropriate diagnostics at the corresponding places, but right now it's more important to fix greenness-redness problems. Plus, this is already how K1 works. ^KT-59900 Fixed --- .../cfg/innerClassInAnonymousObject.kt | 2 +- .../diagnostics/localEntitytNotAllowed.kt | 8 ++--- .../FirLocalEntityNotAllowedChecker.kt | 5 ++- .../declaration/FirNestedClassChecker.kt | 34 +++++++++---------- .../innerClassInEnumEntryClass_lv12.fir.kt | 2 +- .../innerClassInEnumEntryClass_lv13.fir.kt | 2 +- .../diagnostics/tests/inner/kt6026.fir.kt | 8 ----- .../diagnostics/tests/inner/kt6026.kt | 1 + .../diagnostics/tests/localInterfaces.fir.kt | 14 -------- .../diagnostics/tests/localInterfaces.kt | 1 + .../nestedClassInAnonymousObject.fir.kt | 11 ------ .../objects/nestedClassInAnonymousObject.kt | 1 + 12 files changed, 31 insertions(+), 58 deletions(-) delete mode 100644 compiler/testData/diagnostics/tests/inner/kt6026.fir.kt delete mode 100644 compiler/testData/diagnostics/tests/localInterfaces.fir.kt delete mode 100644 compiler/testData/diagnostics/tests/objects/nestedClassInAnonymousObject.fir.kt diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/innerClassInAnonymousObject.kt b/compiler/fir/analysis-tests/testData/resolve/cfg/innerClassInAnonymousObject.kt index 54a804a31d7..04191412972 100644 --- a/compiler/fir/analysis-tests/testData/resolve/cfg/innerClassInAnonymousObject.kt +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/innerClassInAnonymousObject.kt @@ -1,7 +1,7 @@ // !DUMP_CFG val x = object { - class Nested { + class Nested { fun foo() {} } } diff --git a/compiler/fir/analysis-tests/testData/resolve/diagnostics/localEntitytNotAllowed.kt b/compiler/fir/analysis-tests/testData/resolve/diagnostics/localEntitytNotAllowed.kt index a31b4de7fe3..7149e1f9922 100644 --- a/compiler/fir/analysis-tests/testData/resolve/diagnostics/localEntitytNotAllowed.kt +++ b/compiler/fir/analysis-tests/testData/resolve/diagnostics/localEntitytNotAllowed.kt @@ -8,16 +8,16 @@ object A { val a = object : Any() { object D { object G - interface Z + interface Z } - interface Y + interface Y } fun b() { object E { object F - interface M + interface M } interface N @@ -25,7 +25,7 @@ object A { val c = object : Any() { val t = "test" - interface U + interface U } } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirLocalEntityNotAllowedChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirLocalEntityNotAllowedChecker.kt index 02cc23ebccb..51479a5bc88 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirLocalEntityNotAllowedChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirLocalEntityNotAllowedChecker.kt @@ -11,6 +11,7 @@ import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.diagnostics.reportOn +import org.jetbrains.kotlin.fir.declarations.FirClass import org.jetbrains.kotlin.fir.declarations.FirRegularClass import org.jetbrains.kotlin.fir.declarations.utils.isCompanion import org.jetbrains.kotlin.fir.declarations.utils.visibility @@ -21,10 +22,12 @@ object FirLocalEntityNotAllowedChecker : FirRegularClassChecker() { return } + val container = context.containingDeclarations.lastOrNull() + when { declaration.classKind == ClassKind.OBJECT && !declaration.isCompanion -> reporter.reportOn(declaration.source, FirErrors.LOCAL_OBJECT_NOT_ALLOWED, declaration.name, context) - declaration.classKind == ClassKind.INTERFACE -> + declaration.classKind == ClassKind.INTERFACE && container !is FirClass -> reporter.reportOn(declaration.source, FirErrors.LOCAL_INTERFACE_NOT_ALLOWED, declaration.name, context) else -> { } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirNestedClassChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirNestedClassChecker.kt index 49037ea2252..d9a70d99652 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirNestedClassChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirNestedClassChecker.kt @@ -6,12 +6,13 @@ package org.jetbrains.kotlin.fir.analysis.checkers.declaration import org.jetbrains.kotlin.descriptors.ClassKind +import org.jetbrains.kotlin.descriptors.EffectiveVisibility import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.NESTED_CLASS_NOT_ALLOWED import org.jetbrains.kotlin.diagnostics.reportOn -import org.jetbrains.kotlin.fir.declarations.FirClass -import org.jetbrains.kotlin.fir.declarations.FirRegularClass +import org.jetbrains.kotlin.fir.declarations.* +import org.jetbrains.kotlin.fir.declarations.utils.effectiveVisibility import org.jetbrains.kotlin.fir.declarations.utils.isCompanion import org.jetbrains.kotlin.fir.declarations.utils.isInner import org.jetbrains.kotlin.fir.declarations.utils.isLocal @@ -22,25 +23,24 @@ object FirNestedClassChecker : FirRegularClassChecker() { override fun check(declaration: FirRegularClass, context: CheckerContext, reporter: DiagnosticReporter) { // Local enums / objects / companion objects are handled with different diagnostic codes. if ((declaration.classKind.isSingleton || declaration.classKind == ClassKind.ENUM_CLASS) && declaration.isLocal) return - val containingDeclaration = context.containingDeclarations.lastOrNull() ?: return + val containingDeclaration = context.containingDeclarations.lastOrNull() as? FirClass ?: return - when (containingDeclaration) { - is FirRegularClass -> { - if (!declaration.isInner && (containingDeclaration.isInner || containingDeclaration.isLocal)) { - reporter.reportOn(declaration.source, NESTED_CLASS_NOT_ALLOWED, declaration.description, context) - } - } - is FirClass -> { - // Since 1.3, enum entries can contain inner classes only. - // Companion objects are reported with code WRONG_MODIFIER_CONTAINING_DECLARATION instead - if (containingDeclaration.classKind == ClassKind.ENUM_ENTRY && !declaration.isInner && !declaration.isCompanion) { - reporter.reportOn(declaration.source, NESTED_CLASS_NOT_ALLOWED, declaration.description, context) - } - } - else -> {} + // Since 1.3, enum entries can contain inner classes only. + // Companion objects are reported with code WRONG_MODIFIER_CONTAINING_DECLARATION instead + if (containingDeclaration.classKind == ClassKind.ENUM_ENTRY && !declaration.isInner && !declaration.isCompanion) { + reporter.reportOn(declaration.source, NESTED_CLASS_NOT_ALLOWED, declaration.description, context) + return + } + + val containerIsLocal = containingDeclaration.effectiveVisibility == EffectiveVisibility.Local + + if (!declaration.isInner && (containingDeclaration.isInner || containerIsLocal || context.isInsideAnonymousObject)) { + reporter.reportOn(declaration.source, NESTED_CLASS_NOT_ALLOWED, declaration.description, context) } } + private val CheckerContext.isInsideAnonymousObject get() = containingDeclarations.any { it is FirAnonymousObject } + // Note: here we don't differentiate anonymous object like in FE1.0 // (org.jetbrains.kotlin.resolve.ModifiersChecker.DetailedClassKind) because this case has been ruled out in the first place. private val FirRegularClass.description: String diff --git a/compiler/testData/diagnostics/tests/inner/innerClassInEnumEntryClass_lv12.fir.kt b/compiler/testData/diagnostics/tests/inner/innerClassInEnumEntryClass_lv12.fir.kt index ddbafdd925b..86c86f3d384 100644 --- a/compiler/testData/diagnostics/tests/inner/innerClassInEnumEntryClass_lv12.fir.kt +++ b/compiler/testData/diagnostics/tests/inner/innerClassInEnumEntryClass_lv12.fir.kt @@ -6,7 +6,7 @@ enum class Enum { class TestNested - interface TestInterface + interface TestInterface object TestObject diff --git a/compiler/testData/diagnostics/tests/inner/innerClassInEnumEntryClass_lv13.fir.kt b/compiler/testData/diagnostics/tests/inner/innerClassInEnumEntryClass_lv13.fir.kt index 7632eae4f5d..8d02cab549f 100644 --- a/compiler/testData/diagnostics/tests/inner/innerClassInEnumEntryClass_lv13.fir.kt +++ b/compiler/testData/diagnostics/tests/inner/innerClassInEnumEntryClass_lv13.fir.kt @@ -6,7 +6,7 @@ enum class Enum { class TestNested - interface TestInterface + interface TestInterface object TestObject diff --git a/compiler/testData/diagnostics/tests/inner/kt6026.fir.kt b/compiler/testData/diagnostics/tests/inner/kt6026.fir.kt deleted file mode 100644 index 13f0c4837d9..00000000000 --- a/compiler/testData/diagnostics/tests/inner/kt6026.fir.kt +++ /dev/null @@ -1,8 +0,0 @@ -// KT-6026 Exception on instantiating a nested class in an anonymous object - -val oo = object { - // Forbidden in KT-13510 - class Nested - - fun f1() = Nested(11) -} diff --git a/compiler/testData/diagnostics/tests/inner/kt6026.kt b/compiler/testData/diagnostics/tests/inner/kt6026.kt index af574e2006b..0d79cb35d5f 100644 --- a/compiler/testData/diagnostics/tests/inner/kt6026.kt +++ b/compiler/testData/diagnostics/tests/inner/kt6026.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // KT-6026 Exception on instantiating a nested class in an anonymous object val oo = object { diff --git a/compiler/testData/diagnostics/tests/localInterfaces.fir.kt b/compiler/testData/diagnostics/tests/localInterfaces.fir.kt deleted file mode 100644 index d71e05f5bf7..00000000000 --- a/compiler/testData/diagnostics/tests/localInterfaces.fir.kt +++ /dev/null @@ -1,14 +0,0 @@ -// !DIAGNOSTICS: -UNUSED_VARIABLE - -fun foo() { - interface a {} - val b = object { - interface c {} - } - class A { - interface d {} - } - val f = { - interface e {} - } -} diff --git a/compiler/testData/diagnostics/tests/localInterfaces.kt b/compiler/testData/diagnostics/tests/localInterfaces.kt index f87ae26abbe..84805139dbc 100644 --- a/compiler/testData/diagnostics/tests/localInterfaces.kt +++ b/compiler/testData/diagnostics/tests/localInterfaces.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // !DIAGNOSTICS: -UNUSED_VARIABLE fun foo() { diff --git a/compiler/testData/diagnostics/tests/objects/nestedClassInAnonymousObject.fir.kt b/compiler/testData/diagnostics/tests/objects/nestedClassInAnonymousObject.fir.kt deleted file mode 100644 index 1a182ce94d9..00000000000 --- a/compiler/testData/diagnostics/tests/objects/nestedClassInAnonymousObject.fir.kt +++ /dev/null @@ -1,11 +0,0 @@ -class X { - val foo = object { - class Foo - } - - fun test() { - object { - class Foo - } - } -} diff --git a/compiler/testData/diagnostics/tests/objects/nestedClassInAnonymousObject.kt b/compiler/testData/diagnostics/tests/objects/nestedClassInAnonymousObject.kt index bf6a6d54f3d..2abb2c91062 100644 --- a/compiler/testData/diagnostics/tests/objects/nestedClassInAnonymousObject.kt +++ b/compiler/testData/diagnostics/tests/objects/nestedClassInAnonymousObject.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL class X { val foo = object { class Foo