Distinguish classes from different sourcesets in SealedInheritorInSameModuleChecker

#KT-45848 Fixed #KT-46031
2021-04-09 16:01:20 +03:00
parent 66e4d5664a
commit 803d47daaa
20 changed files with 197 additions and 4 deletions
@@ -62,8 +62,9 @@ class K2MetadataCompiler : CLICompiler<K2MetadataCompilerArguments>() {
        val pluginLoadResult = loadPlugins(paths, arguments, configuration)
        if (pluginLoadResult != ExitCode.OK) return pluginLoadResult

+        val commonSources = arguments.commonSources?.toSet() ?: emptySet()
        for (arg in arguments.freeArgs) {
-            configuration.addKotlinSourceRoot(arg, isCommon = true)
+            configuration.addKotlinSourceRoot(arg, isCommon = arg in commonSources)
        }
        if (arguments.classpath != null) {
            configuration.addJvmClasspathRoots(arguments.classpath!!.split(File.pathSeparatorChar).map(::File))
@@ -18118,6 +18118,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti
                    KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/multiplatform/exhaustiveness"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true);
                }

+                @Test
+                @TestMetadata("commonSealedWithPlatformInheritor.kt")
+                public void testCommonSealedWithPlatformInheritor() throws Exception {
+                    runTest("compiler/testData/diagnostics/tests/multiplatform/exhaustiveness/commonSealedWithPlatformInheritor.kt");
+                }
+
                @Test
                @TestMetadata("expectEnum.kt")
                public void testExpectEnum() throws Exception {
@@ -6,14 +6,17 @@
 package org.jetbrains.kotlin.resolve.checkers

 import org.jetbrains.kotlin.descriptors.ClassDescriptor
+import org.jetbrains.kotlin.descriptors.ClassifierDescriptor
 import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
-import org.jetbrains.kotlin.descriptors.impl.ModuleDescriptorImpl
 import org.jetbrains.kotlin.descriptors.isSealed
 import org.jetbrains.kotlin.diagnostics.Errors
 import org.jetbrains.kotlin.psi.KtClassOrObject
 import org.jetbrains.kotlin.psi.KtDeclaration
+import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.resolve.bindingContextUtil.getAbbreviatedTypeOrType
 import org.jetbrains.kotlin.resolve.descriptorUtil.module
+import org.jetbrains.kotlin.resolve.multiplatform.isCommonSource
+import org.jetbrains.kotlin.resolve.source.PsiSourceFile

 object SealedInheritorInSameModuleChecker : DeclarationChecker {
    override fun check(declaration: KtDeclaration, descriptor: DeclarationDescriptor, context: DeclarationCheckerContext) {
@@ -24,16 +27,50 @@ object SealedInheritorInSameModuleChecker : DeclarationChecker {
            val superType = typeReference.getAbbreviatedTypeOrType(context.trace.bindingContext)?.unwrap() ?: continue
            val superClass = superType.constructor.declarationDescriptor ?: continue
            if (superClass.isSealed()) {
+                /*
+                 * If this condition is true then we compile code from CLI mode and class came from
+                 *   common source set (in CLI mode we have single ModuleDescriptor for all MPP modules)
+                 *
+                 * We don't want to analyze classes from common modules, because there are problems with determining
+                 *   relation between module of class and module of super class (it's allowed to declare sealed
+                 *   inheritors between `expect` class declaration and its actual), so we assume that incorrect
+                 *   declarations will be reported while compiling common module to get metadata
+                 *
+                 *   common               expect sealed class Base
+                 *     |                  class A : Base() // OK, A in same module with Base
+                 *     |
+                 * jvm-js-native          class B : Base() // OK, B inherits `expect` class, not `actual`
+                 *     |
+                 *   jvm-js               actual sealed class Base
+                 *     |                  class C : Base() // OK, C in same module with actual Base
+                 *     |
+                 *   jvm-js-2             class D : Base() // Error, D not in same module with actual Base
+                 *     |
+                 *    jvm                 class E : Base() // Error, E not in same module with actual Base
+                 *
+                 * In this hierarchy error on `D` will be reported in process of compiling `jvm-js-2` module in metadata mode
+                 *   (so `jvm-js-2` assumed as "platform" module and others are "common") and won't be reported during
+                 *   compilation of `jvm`.
+                 *
+                 * There is one problem with this condition: if modules hierarchy is a bamboo (no modules with two children)
+                 *   then we won't compile intermediate modules in metadata mode, only the last one, so error on class `D`
+                 *   won't be reported. See KT-46031
+                 */
+                if (descriptor.isFromCommonSource && superClass.module == currentModule) {
+                    return
+                }
                /*
                 * It's allowed to declare inheritors of expect sealed class in any dependent module until actual
                 *   counterpart for this class will be declared. So if super class is resolved to expect sealed
                 *   class then its allowed to declare inheritor
                 */
-                val inheritorAllowed = superClass.isExpect || superClass.module == currentModule
-                if (!inheritorAllowed) {
+                if (!(superClass.isExpect || (superClass.module == currentModule && !superClass.isFromCommonSource))) {
                    context.trace.report(Errors.SEALED_INHERITOR_IN_DIFFERENT_MODULE.on(typeReference))
                }
            }
        }
    }
+
+    private val ClassifierDescriptor.isFromCommonSource: Boolean
+        get() = ((this.source.containingFile as? PsiSourceFile)?.psiFile as? KtFile)?.isCommonSource ?: false
 }
@@ -0,0 +1,6 @@
+$TESTDATA_DIR$/inheritorOfExpectSealedClass/common-1.kt
+$TESTDATA_DIR$/inheritorOfExpectSealedClass/common-2.kt
+$TESTDATA_DIR$/inheritorOfExpectSealedClass/common-3.kt
+-d
+$TEMP_DIR$
+-Xcommon-sources=$TESTDATA_DIR$/inheritorOfExpectSealedClass/common-1.kt,$TESTDATA_DIR$/inheritorOfExpectSealedClass/common-2.kt
@@ -0,0 +1,4 @@
+compiler/testData/cli/metadata/inheritorOfExpectSealedClass/common-3.kt:1:17: error: inheritance of sealed classes or interfaces from different module is prohibited
+class Derived : Base()
+                ^
+COMPILATION_ERROR
@@ -0,0 +1 @@
+expect sealed class Base
@@ -0,0 +1 @@
+actual sealed class Base
@@ -0,0 +1 @@
+class Derived : Base()
@@ -0,0 +1,5 @@
+$TESTDATA_DIR$/optionalExpectationUsage.kt
+-d
+$TEMP_DIR$
+-cp
+$TESTDATA_DIR$/../../../../dist/common/kotlin-stdlib-common.jar
@@ -0,0 +1,7 @@
+@kotlin.ExperimentalMultiplatform
+@kotlin.OptionalExpectation
+expect annotation class Ann()
+
+@Ann
+@kotlin.ExperimentalMultiplatform
+fun foo() {}
@@ -0,0 +1 @@
+OK
@@ -0,0 +1,19 @@
+// !LANGUAGE: +MultiPlatformProjects
+// ISSUE: KT-45848
+// MODULE: m1-common
+
+sealed class Base
+
+class Derived : Base()
+
+fun test_1(b: Base) = when (b) {
+    is Derived -> 1
+}
+
+// MODULE: m1-jvm(m1-common)
+
+class PlatfromDerived : Base() // must be an error
+
+fun test_2(b: Base) = when (b) {
+    is Derived -> 1
+}
@@ -0,0 +1,19 @@
+// !LANGUAGE: +MultiPlatformProjects
+// ISSUE: KT-45848
+// MODULE: m1-common
+
+sealed class Base
+
+class Derived : Base()
+
+fun test_1(b: Base) = <!NO_ELSE_IN_WHEN{JVM}!>when<!> (b) {
+    is Derived -> 1
+}
+
+// MODULE: m1-jvm(m1-common)
+
+class PlatfromDerived : <!SEALED_INHERITOR_IN_DIFFERENT_MODULE!>Base<!>() // must be an error
+
+fun test_2(b: Base) = <!NO_ELSE_IN_WHEN!>when<!> (b) {
+    is Derived -> 1
+}
@@ -0,0 +1,46 @@
+// -- Module: <m1-common> --
+package
+
+public fun test_1(/*0*/ b: Base): kotlin.Int
+
+public sealed class Base {
+    protected constructor Base()
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public final class Derived : Base {
+    public constructor Derived()
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+// -- Module: <m1-jvm> --
+package
+
+public fun test_1(/*0*/ b: Base): kotlin.Int
+public fun test_2(/*0*/ b: Base): kotlin.Int
+
+public sealed class Base {
+    protected constructor Base()
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public final class Derived : Base {
+    public constructor Derived()
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public final class PlatfromDerived : Base {
+    public constructor PlatfromDerived()
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
@@ -18124,6 +18124,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
                    KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/multiplatform/exhaustiveness"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true);
                }

+                @Test
+                @TestMetadata("commonSealedWithPlatformInheritor.kt")
+                public void testCommonSealedWithPlatformInheritor() throws Exception {
+                    runTest("compiler/testData/diagnostics/tests/multiplatform/exhaustiveness/commonSealedWithPlatformInheritor.kt");
+                }
+
                @Test
                @TestMetadata("expectEnum.kt")
                public void testExpectEnum() throws Exception {
@@ -1195,6 +1195,11 @@ public class CliTestGenerated extends AbstractCliTest {
            runTest("compiler/testData/cli/metadata/anonymousObjectType.args");
        }

+        @TestMetadata("inheritorOfExpectSealedClass.args")
+        public void testInheritorOfExpectSealedClass() throws Exception {
+            runTest("compiler/testData/cli/metadata/inheritorOfExpectSealedClass.args");
+        }
+
        @TestMetadata("kotlinPackage.args")
        public void testKotlinPackage() throws Exception {
            runTest("compiler/testData/cli/metadata/kotlinPackage.args");
@@ -1204,5 +1209,10 @@ public class CliTestGenerated extends AbstractCliTest {
        public void testModuleName() throws Exception {
            runTest("compiler/testData/cli/metadata/moduleName.args");
        }
+
+        @TestMetadata("optionalExpectationUsage.args")
+        public void testOptionalExpectationUsage() throws Exception {
+            runTest("compiler/testData/cli/metadata/optionalExpectationUsage.args");
+        }
    }
 }
@@ -0,0 +1,9 @@
+// ISSUE: KT-45848
+
+sealed class <!LINE_MARKER("descr='Is subclassed by Derived PlatfromDerived'")!>Base<!>
+
+class Derived : Base()
+
+fun test_1(b: Base) = <!NO_ELSE_IN_WHEN{JVM}!>when<!> (b) {
+    is Derived -> 1
+}
@@ -0,0 +1,4 @@
+MODULE common { platform=[JVM, JS, Native] }
+MODULE jvm { platform=[JVM] }
+
+jvm -> common { kind=DEPENDS_ON }
@@ -0,0 +1,5 @@
+class PlatfromDerived : <!SEALED_INHERITOR_IN_DIFFERENT_MODULE!>Base<!>() // must be an error
+
+fun test_2(b: Base) = when (b) {
+    is Derived -> 1
+}
@@ -44,6 +44,11 @@ public class MultiplatformAnalysisTestGenerated extends AbstractMultiplatformAna
        runTest("idea/testData/multiplatform/callableReferences/");
    }

+    @TestMetadata("commonSealedWithPlatformInheritor")
+    public void testCommonSealedWithPlatformInheritor() throws Exception {
+        runTest("idea/testData/multiplatform/commonSealedWithPlatformInheritor/");
+    }
+
    @TestMetadata("constructorsOfExpect")
    public void testConstructorsOfExpect() throws Exception {
        runTest("idea/testData/multiplatform/constructorsOfExpect/");