[FIR] Make FirNotImplementedOverrideChecker to run only for non-expect classes or in metadata compilation

^KT-64187 Fixed Review: https://jetbrains.team/p/kt/reviews/14807 I also checked that IDE sets metadataCompilation to `true` (Actually, it looks like IDE performs analysis two times with the flag having different values, but whatever, maybe it's even better this way)
2024-03-04 14:53:02 +01:00
parent 9d566465e6
commit 989ec97bb0
17 changed files with 167 additions and 31 deletions
@@ -9,6 +9,22 @@ object AnalysisFlags {
    @JvmStatic
    val skipMetadataVersionCheck by AnalysisFlag.Delegates.Boolean

+    /**
+     * Metadata compilation is run for every non-platform fragment (for common and intermediate fragments).
+     *
+     * The initial purpose of the metadata compilation is to produce metadata artifacts for non-platform source sets.
+     * These metadata artifacts are needed only for IDE.
+     *
+     * But metadata compilation is also used to report diagnostics when regular compilation can't do that for whatever reason (My opinion,
+     * that the reason is broken design of `expect`/`actual` KT-64130).
+     * E.g. `ABSTRACT_NOT_IMPLEMENTED` (KT-66205)
+     *
+     * This flag is true in two case:
+     * 1. Metadata compilation (`:compile*KotlinMetadata` Gradle task)
+     * 2. IDE analysis
+     *
+     * Known metadata compilation problems: KT-66382
+     */
    @JvmStatic
    val metadataCompilation by AnalysisFlag.Delegates.Boolean

@@ -100,8 +100,7 @@ object CommonDeclarationCheckers : DeclarationCheckers() {
        get() = setOf(
            FirOverrideChecker.Regular,
            FirOverrideChecker.ForExpectClass,
-            FirNotImplementedOverrideChecker.Regular,
-            FirNotImplementedOverrideChecker.ForExpectClass,
+            FirNotImplementedOverrideChecker,
            FirNotImplementedOverrideSimpleEnumEntryChecker.Regular,
            FirNotImplementedOverrideSimpleEnumEntryChecker.ForExpectClass,
            FirThrowableSubclassChecker,
@@ -6,6 +6,7 @@
 package org.jetbrains.kotlin.fir.analysis.checkers.declaration

 import org.jetbrains.kotlin.KtFakeSourceElementKind
+import org.jetbrains.kotlin.config.AnalysisFlags
 import org.jetbrains.kotlin.descriptors.ClassKind
 import org.jetbrains.kotlin.descriptors.Modality
 import org.jetbrains.kotlin.diagnostics.DiagnosticReporter
@@ -41,22 +42,9 @@ import org.jetbrains.kotlin.util.ImplementationStatus
 import kotlin.contracts.ExperimentalContracts
 import kotlin.contracts.contract

-sealed class FirNotImplementedOverrideChecker(mppKind: MppCheckerKind) : FirClassChecker(mppKind) {
-    object Regular : FirNotImplementedOverrideChecker(MppCheckerKind.Platform) {
-        override fun check(declaration: FirClass, context: CheckerContext, reporter: DiagnosticReporter) {
-            if (declaration.isExpect) return
-            super.check(declaration, context, reporter)
-        }
-    }
-
-    object ForExpectClass : FirNotImplementedOverrideChecker(MppCheckerKind.Common) {
-        override fun check(declaration: FirClass, context: CheckerContext, reporter: DiagnosticReporter) {
-            if (!declaration.isExpect) return
-            super.check(declaration, context, reporter)
-        }
-    }
-
+object FirNotImplementedOverrideChecker : FirClassChecker(MppCheckerKind.Platform) {
    override fun check(declaration: FirClass, context: CheckerContext, reporter: DiagnosticReporter) {
+        if (declaration.isExpect && !context.languageVersionSettings.getFlag(AnalysisFlags.metadataCompilation)) return
        val source = declaration.source ?: return
        val sourceKind = source.kind
        if (sourceKind is KtFakeSourceElementKind && sourceKind != KtFakeSourceElementKind.EnumInitializer) return
@@ -5,7 +5,7 @@ interface Base {
    fun foo()
 }

-expect open <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class Foo<!> : Base
+expect open <!ABSTRACT_MEMBER_NOT_IMPLEMENTED{METADATA}!>class Foo<!> : Base

 // MODULE: m2-jvm()()(m1-common)
 // FILE: jvm.kt
@@ -0,0 +1,15 @@
+// MODULE: m1-common
+// FILE: common.kt
+
+interface Base {
+    fun foo()
+}
+
+expect open class Foo : Base
+
+// MODULE: m2-jvm()()(m1-common)
+// FILE: jvm.kt
+
+actual open class Foo : Base {
+    final override fun <!ACTUAL_WITHOUT_EXPECT!>foo<!>() {}
+}
@@ -3,7 +3,7 @@
 interface Base {
    fun foo()
 }
-expect open <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class Foo<!>() : Base
+expect open <!ABSTRACT_MEMBER_NOT_IMPLEMENTED{METADATA}!>class Foo<!>() : Base


 // MODULE: m2-jvm()()(m1-common)
@@ -0,0 +1,17 @@
+// MODULE: m1-common
+// FILE: common.kt
+interface Base {
+    fun foo()
+}
+expect open class Foo() : Base
+
+
+// MODULE: m2-jvm()()(m1-common)
+// FILE: jvm.kt
+
+// Mismatched scope must be reported here. But it's false negative checker in K1.
+// For some reason, K1 says that modality of `exect_Foo.foo` is `abstract`.
+// https://youtrack.jetbrains.com/issue/KT-59739
+actual open class Foo : Base {
+    override fun <!ACTUAL_WITHOUT_EXPECT!>foo<!>() {}
+}
@@ -6,9 +6,9 @@ interface B {
    override fun toString(): String
 }

-expect value <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class C<!>(val s: String) : B
+expect value <!ABSTRACT_MEMBER_NOT_IMPLEMENTED{METADATA}!>class C<!>(val s: String) : B

-expect value <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class D<!>(val s: String) : B
+expect value <!ABSTRACT_MEMBER_NOT_IMPLEMENTED{METADATA}!>class D<!>(val s: String) : B

 // MODULE: m1-jvm()()(m1-common)
 // FILE: jvm.kt
@@ -0,0 +1,22 @@
+// WITH_STDLIB
+// MODULE: m1-common
+// FILE: common.kt
+
+interface B {
+    override fun toString(): String
+}
+
+expect value class C(val s: String) : B
+
+expect value class D(val s: String) : B
+
+// MODULE: m1-jvm()()(m1-common)
+// FILE: jvm.kt
+
+@JvmInline
+actual value class C(actual val s: String) : B {
+    override fun toString(): String = s
+}
+
+@JvmInline
+actual value class D(actual val s: String) : B
@@ -5,7 +5,7 @@ interface Base {
    fun foo()
 }

-expect <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>object Implementation<!> : Base
+expect <!ABSTRACT_MEMBER_NOT_IMPLEMENTED{METADATA}!>object Implementation<!> : Base

 // MODULE: m1-jvm()()(m1-common)
 // FILE: jvm.kt
@@ -0,0 +1,17 @@
+// MODULE: m1-common
+// FILE: common.kt
+
+interface Base {
+    fun foo()
+}
+
+expect object Implementation : Base
+
+// MODULE: m1-jvm()()(m1-common)
+// FILE: jvm.kt
+
+abstract class RealImplementation : Base {
+    override fun foo() {}
+}
+
+actual object Implementation : RealImplementation(), Base
@@ -5,13 +5,13 @@ interface Foo {
    fun foo()
 }

-expect <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class ImplicitFoo<!> : Foo
+expect <!ABSTRACT_MEMBER_NOT_IMPLEMENTED{METADATA}!>class ImplicitFoo<!> : Foo

 expect class ExplicitFoo : Foo {
    override fun foo()
 }

-expect <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class ImplicitFooCheck<!> : Foo
+expect <!ABSTRACT_MEMBER_NOT_IMPLEMENTED{METADATA}!>class ImplicitFooCheck<!> : Foo

 // MODULE: m2-jvm()()(m1-common)
 // FILE: jvm.kt
@@ -0,0 +1,27 @@
+// MODULE: m1-common
+// FILE: common.kt
+
+interface Foo {
+    fun foo()
+}
+
+expect class ImplicitFoo : Foo
+
+expect class ExplicitFoo : Foo {
+    override fun foo()
+}
+
+expect class ImplicitFooCheck : Foo
+
+// MODULE: m2-jvm()()(m1-common)
+// FILE: jvm.kt
+
+actual class ImplicitFoo : Foo {
+    override fun foo() {}
+}
+
+actual class ExplicitFoo : Foo {
+    actual override fun foo() {}
+}
+
+actual <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class ImplicitFooCheck<!> : Foo
@@ -4,7 +4,7 @@
 expect abstract class BaseA() {
    abstract fun foo()
 }
-expect open <!ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED!>class BaseAImpl<!>() : BaseA
+expect open <!ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED{METADATA}!>class BaseAImpl<!>() : BaseA

 <!ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED, ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED{METADATA}!>class DerivedA1<!> : BaseAImpl()
 class DerivedA2 : BaseAImpl() {
@@ -16,7 +16,7 @@ class DerivedA2 : BaseAImpl() {
 expect interface BaseB {
    fun foo()
 }
-expect open <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class BaseBImpl<!>() : BaseB
+expect open <!ABSTRACT_MEMBER_NOT_IMPLEMENTED{METADATA}!>class BaseBImpl<!>() : BaseB

 <!ABSTRACT_MEMBER_NOT_IMPLEMENTED, ABSTRACT_MEMBER_NOT_IMPLEMENTED{METADATA}!>class DerivedB1<!> : BaseBImpl()
 class DerivedB2 : BaseBImpl() {
@@ -58,7 +58,7 @@ sealed class BaseEImpl() : BaseE {
 expect interface BaseF {
    fun foo()
 }
-expect <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class BaseFImpl<!>() : BaseF
+expect <!ABSTRACT_MEMBER_NOT_IMPLEMENTED{METADATA}!>class BaseFImpl<!>() : BaseF



@@ -4,7 +4,7 @@
 expect abstract class BaseA() {
    abstract fun foo()
 }
-expect open <!ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED!>class BaseAImpl<!>() : BaseA
+expect open class BaseAImpl() : BaseA

 <!ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED!>class DerivedA1<!> : BaseAImpl()
 class DerivedA2 : BaseAImpl() {
@@ -16,7 +16,7 @@ class DerivedA2 : BaseAImpl() {
 expect interface BaseB {
    fun foo()
 }
-expect open <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class BaseBImpl<!>() : BaseB
+expect open class BaseBImpl() : BaseB

 <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class DerivedB1<!> : BaseBImpl()
 class DerivedB2 : BaseBImpl() {
@@ -58,7 +58,7 @@ sealed class BaseEImpl() : BaseE {
 expect interface BaseF {
    fun foo()
 }
-expect <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class BaseFImpl<!>() : BaseF
+expect class BaseFImpl() : BaseF



@@ -5,7 +5,7 @@ expect abstract class Base {
    abstract fun foo()
 }

-expect <!ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED!>class DerivedImplicit<!> : Base
+expect <!ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED{METADATA}!>class DerivedImplicit<!> : Base

 expect class DerivedExplicit : Base {
    override fun foo()
@@ -0,0 +1,35 @@
+// MODULE: m1-common
+// FILE: common.kt
+
+expect abstract class Base {
+    abstract fun foo()
+}
+
+expect class DerivedImplicit : Base
+
+expect class DerivedExplicit : Base {
+    override fun foo()
+}
+
+expect class DerivedExplicitCheck : Base {
+    override fun foo()
+}
+
+// MODULE: m2-jvm()()(m1-common)
+// FILE: jvm.kt
+
+actual abstract class Base {
+    actual abstract fun foo()
+}
+
+actual class DerivedImplicit : Base() {
+    override fun foo() {}
+}
+
+actual class DerivedExplicit : Base() {
+    actual override fun foo() {}
+}
+
+actual class DerivedExplicitCheck : Base() {
+    override fun <!ACTUAL_MISSING!>foo<!>() {}
+}