New default checks for mixed hierarchies

Old and new schemes

compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/JvmDefaultChecker.kt

@@ -5,6 +5,7 @@
 
 package org.jetbrains.kotlin.resolve.jvm.checkers
 
+import com.intellij.openapi.project.Project
 import org.jetbrains.kotlin.config.JvmAnalysisFlags
 import org.jetbrains.kotlin.config.JvmDefaultMode
 import org.jetbrains.kotlin.config.JvmTarget
@@ -14,15 +15,21 @@ import org.jetbrains.kotlin.load.kotlin.computeJvmDescriptor
 import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
 import org.jetbrains.kotlin.resolve.DescriptorUtils.*
+import org.jetbrains.kotlin.resolve.LanguageVersionSettingsProvider
 import org.jetbrains.kotlin.resolve.OverridingUtil
 import org.jetbrains.kotlin.resolve.checkers.DeclarationChecker
 import org.jetbrains.kotlin.resolve.checkers.DeclarationCheckerContext
+import org.jetbrains.kotlin.resolve.descriptorUtil.getSuperClassNotAny
 import org.jetbrains.kotlin.resolve.descriptorUtil.isEffectivelyPrivateApi
+import org.jetbrains.kotlin.resolve.descriptorUtil.module
 import org.jetbrains.kotlin.resolve.jvm.annotations.*
 import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm
 import org.jetbrains.kotlin.util.getNonPrivateTraitMembersForDelegation
+import org.jetbrains.kotlin.utils.addToStdlib.firstNotNullResult
 
-class JvmDefaultChecker(val jvmTarget: JvmTarget) : DeclarationChecker {
+class JvmDefaultChecker(val jvmTarget: JvmTarget, project: Project) : DeclarationChecker {
+
+    private val ideService = LanguageVersionSettingsProvider.getInstance(project)
 
     override fun check(declaration: KtDeclaration, descriptor: DeclarationDescriptor, context: DeclarationCheckerContext) {
         val jvmDefaultMode = context.languageVersionSettings.getFlag(JvmAnalysisFlags.jvmDefaultMode)
@@ -75,46 +82,79 @@ class JvmDefaultChecker(val jvmTarget: JvmTarget) : DeclarationChecker {
                     }
                 }
             }
-        } else if (jvmDefaultMode.isCompatibility &&
+        }
-            !isInterface(descriptor) &&
+
-            !isAnnotationClass(descriptor) &&
+        if (!jvmDefaultMode.isEnabled || descriptor !is ClassDescriptor || isInterface(descriptor) || isAnnotationClass(descriptor)) return
-            descriptor is ClassDescriptor &&
+        // JvmDefaults members checks across class hierarchy:
-            !descriptor.hasJvmDefaultNoCompatibilityAnnotation()
+        // 1. If in old scheme class have implicit override with different signature than overriden method (e.g. generic specialization)
-        ) {
+        // report error because absent of it's can affect library ABI
-            val modality = descriptor.modality
+        // 2. If it's mixed hierarchy with implicit override in base class and override one in inherited derived interface report error.
-            //TODO: maybe remove this check for jvm compatibility
+        // Otherwise the implicit class override would be used for dispatching method calls (but not more specialized)
-            if (modality !== Modality.OPEN && modality !== Modality.ABSTRACT || descriptor.isEffectivelyPrivateApi) return
+        val performSpecializationCheck = jvmDefaultMode.isCompatibility && !descriptor.hasJvmDefaultNoCompatibilityAnnotation() &&
-            for ((inheritedMember, actualImplementation) in getNonPrivateTraitMembersForDelegation(
+                //TODO: maybe remove this check for JVM compatibility
-                descriptor,
+                !(descriptor.modality !== Modality.OPEN && descriptor.modality !== Modality.ABSTRACT || descriptor.isEffectivelyPrivateApi)
-                returnImplNotDelegate = true
+
-            )) {
+        //Should we check clash with implicit class member (that comes from old compilation scheme) and specialization for compatibility mode
-                if (actualImplementation.isCallableMemberCompiledToJvmDefault(jvmDefaultMode)) {
+        // If specialization check is reported clash one shouldn't be reported
-                    if (actualImplementation is FunctionDescriptor && inheritedMember is FunctionDescriptor) {
+        if (descriptor.getSuperClassNotAny() == null && !performSpecializationCheck) return
-                        processMember(inheritedMember, actualImplementation, context, declaration)
+
-                    } else if (actualImplementation is PropertyDescriptor && inheritedMember is PropertyDescriptor) {
+        getNonPrivateTraitMembersForDelegation(
-                        val getterImpl = actualImplementation.getter
+            descriptor,
-                        val getterInherited = inheritedMember.getter
+            returnImplNotDelegate = true
-                        if (getterImpl == null || getterInherited == null || processMember(getterImpl, getterImpl, context, declaration)) {
+        ).filter { (_, actualImplementation) -> actualImplementation.isCompiledToJvmDefaultWithProperMode(jvmDefaultMode) }
-                            if (actualImplementation.isVar && inheritedMember.isVar) {
+            .forEach { (inheritedMember, actualImplementation) ->
-                                val setterImpl = actualImplementation.setter
+                if (actualImplementation is FunctionDescriptor && inheritedMember is FunctionDescriptor) {
-                                val setterInherited = inheritedMember.setter
+                    if (checkSpecializationInCompatibilityMode(
-                                if (setterImpl != null && setterInherited != null) {
+                            inheritedMember,
-                                    processMember(setterImpl, setterImpl, context, declaration)
+                            actualImplementation,
-                                }
+                            context,
+                            declaration,
+                            performSpecializationCheck
+                        )
+                    ) {
+                        checkPossibleClashMember(inheritedMember, jvmDefaultMode, context, declaration)
+                    }
+                } else if (actualImplementation is PropertyDescriptor && inheritedMember is PropertyDescriptor) {
+                    val getterImpl = actualImplementation.getter
+                    val getterInherited = inheritedMember.getter
+                    if (getterImpl == null || getterInherited == null || !jvmDefaultMode.isCompatibility ||
+                        checkSpecializationInCompatibilityMode(
+                            getterInherited,
+                            getterImpl,
+                            context,
+                            declaration,
+                            performSpecializationCheck
+                        )
+                    ) {
+                        if (actualImplementation.isVar && inheritedMember.isVar) {
+                            val setterImpl = actualImplementation.setter
+                            val setterInherited = inheritedMember.setter
+                            if (setterImpl != null && setterInherited != null) {
+                                if (!checkSpecializationInCompatibilityMode(
+                                        setterInherited,
+                                        setterImpl,
+                                        context,
+                                        declaration,
+                                        performSpecializationCheck
+                                    )
+                                ) return@forEach
                             }
                         }
+
+                        checkPossibleClashMember(inheritedMember, jvmDefaultMode, context, declaration)
                     }
                 }
             }
-        }
     }
 
-    private fun processMember(
+    private fun checkSpecializationInCompatibilityMode(
         inheritedFun: FunctionDescriptor,
         actualImplementation: FunctionDescriptor,
         context: DeclarationCheckerContext,
-        declaration: KtDeclaration
+        declaration: KtDeclaration,
+        performSpecializationCheck: Boolean
     ): Boolean {
+        if (!performSpecializationCheck) return true
         val inheritedSignature = inheritedFun.computeJvmDescriptor(withReturnType = true, withName = false)
         val originalImplementation = actualImplementation.original
         val actualSignature = originalImplementation.computeJvmDescriptor(withReturnType = true, withName = false)
@@ -132,6 +172,39 @@ class JvmDefaultChecker(val jvmTarget: JvmTarget) : DeclarationChecker {
         return true
     }
 
+    private fun checkPossibleClashMember(
+        inheritedFun: CallableMemberDescriptor,
+        jvmDefaultMode: JvmDefaultMode,
+        context: DeclarationCheckerContext,
+        declaration: KtDeclaration
+    ) {
+        val clashMember = findPossibleClashMember(inheritedFun, jvmDefaultMode)
+        if (clashMember != null) {
+            context.trace.report(
+                ErrorsJvm.EXPLICIT_OVERRIDE_REQUIRED_IN_MIXED_MODE.on(
+                    declaration,
+                    getDirectMember(inheritedFun),
+                    getDirectMember(clashMember),
+                    jvmDefaultMode.description
+                )
+            )
+        }
+    }
+
+    private fun findPossibleClashMember(
+        inheritedFun: CallableMemberDescriptor,
+        jvmDefaultMode: JvmDefaultMode
+    ): CallableMemberDescriptor? {
+        val classDescriptor = inheritedFun.containingDeclaration
+        if (classDescriptor !is ClassDescriptor || classDescriptor.getSuperClassNotAny() == null) return null
+        val classMembers =
+            inheritedFun.overriddenDescriptors.filter { !isInterface(it.containingDeclaration) && !isAnnotationClass(it.containingDeclaration) }
+        val implicitDefaultImplsDelegate =
+            classMembers.firstOrNull { getNonPrivateTraitMembersForDelegation(it, true)?.isCompiledToJvmDefaultWithProperMode(jvmDefaultMode) == false }
+        if (implicitDefaultImplsDelegate != null) return implicitDefaultImplsDelegate
+        return classMembers.firstNotNullResult { findPossibleClashMember(it, jvmDefaultMode) }
+    }
+
     private fun checkJvmDefaultsInHierarchy(descriptor: DeclarationDescriptor, jvmDefaultMode: JvmDefaultMode): Boolean {
         if (jvmDefaultMode.isEnabled) return true
 
@@ -140,9 +213,16 @@ class JvmDefaultChecker(val jvmTarget: JvmTarget) : DeclarationChecker {
         return descriptor.unsubstitutedMemberScope.getContributedDescriptors().filterIsInstance<CallableMemberDescriptor>()
             .all { memberDescriptor ->
                 memberDescriptor.kind.isReal || OverridingUtil.filterOutOverridden(memberDescriptor.overriddenDescriptors.toSet()).all {
-                    !isInterface(it.containingDeclaration) || !it.isCompiledToJvmDefault(jvmDefaultMode) || it.modality == Modality.ABSTRACT
+                    !isInterface(it.containingDeclaration) || !it.isCompiledToJvmDefaultWithProperMode(jvmDefaultMode) || it.modality == Modality.ABSTRACT
                 }
             }
     }
 
+    private fun CallableMemberDescriptor.isCompiledToJvmDefaultWithProperMode(compilationDefaultMode: JvmDefaultMode): Boolean {
+        val jvmDefault =
+            if (this is DeserializedDescriptor) compilationDefaultMode else ideService?.getModuleLanguageVersionSettings(module)
+                ?.getFlag(JvmAnalysisFlags.jvmDefaultMode) ?: compilationDefaultMode
+        return isCompiledToJvmDefault(jvmDefault)
+    }
+
 }

compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java

@@ -170,6 +170,12 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension {
                 "Please refer to KT-39603 for details",
                 COMPACT, SHORT_NAMES_IN_TYPES);
 
+        MAP.put(EXPLICIT_OVERRIDE_REQUIRED_IN_MIXED_MODE,
+                "Explicit override is required for ''{0}'' in the ''-Xjvm-default={2}'' mode. " +
+                "Otherwise, implicit class override ''{1}'' (compiled in the old -Xjvm-default mode) " +
+                "is not fully overridden and might be incorrectly called at runtime",
+                SHORT_NAMES_IN_TYPES, SHORT_NAMES_IN_TYPES, TO_STRING);
+
         MAP.put(DANGEROUS_CHARACTERS, "Name contains characters which can cause problems on Windows: {0}", STRING);
     }
 

compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java

@@ -143,6 +143,7 @@ public interface ErrorsJvm {
     DiagnosticFactory0<PsiElement> FUNCTION_DELEGATE_MEMBER_NAME_CLASH = DiagnosticFactory0.create(ERROR);
 
     DiagnosticFactory2<KtDeclaration, CallableDescriptor, CallableDescriptor> EXPLICIT_OVERRIDE_REQUIRED_IN_COMPATIBILITY_MODE = DiagnosticFactory2.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
+    DiagnosticFactory3<KtDeclaration, CallableDescriptor, CallableDescriptor, String> EXPLICIT_OVERRIDE_REQUIRED_IN_MIXED_MODE = DiagnosticFactory3.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
 
     DiagnosticFactory1<PsiElement, String> DANGEROUS_CHARACTERS = DiagnosticFactory1.create(WARNING);
 

compiler/frontend/src/org/jetbrains/kotlin/resolve/LanguageVersionSettingsProvider.kt

@@ -0,0 +1,21 @@
+/*
+ * Copyright 2010-2020 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.resolve
+
+import com.intellij.openapi.components.ServiceManager
+import com.intellij.openapi.project.Project
+import org.jetbrains.kotlin.config.LanguageVersionSettings
+import org.jetbrains.kotlin.descriptors.ModuleDescriptor
+
+interface LanguageVersionSettingsProvider {
+
+    fun getModuleLanguageVersionSettings(module: ModuleDescriptor): LanguageVersionSettings?
+
+    companion object {
+        fun getInstance(project: Project): LanguageVersionSettingsProvider? =
+            ServiceManager.getService(project, LanguageVersionSettingsProvider::class.java)
+    }
+}

compiler/frontend/src/org/jetbrains/kotlin/util/declarationUtil.kt

@@ -98,13 +98,19 @@ fun getNonPrivateTraitMembersForDelegation(
     val result = linkedMapOf<CallableMemberDescriptor, CallableMemberDescriptor>()
     for (declaration in DescriptorUtils.getAllDescriptors(descriptor.defaultType.memberScope)) {
         if (declaration !is CallableMemberDescriptor) continue
-
+        result[declaration] = getNonPrivateTraitMembersForDelegation(declaration, returnImplNotDelegate) ?: continue
-        val traitMember = findInterfaceImplementation(declaration, returnImplNotDelegate)
-        if (traitMember == null ||
-            Visibilities.isPrivate(traitMember.visibility) ||
-            traitMember.visibility == Visibilities.INVISIBLE_FAKE
-        ) continue
-        result[declaration] = traitMember
     }
     return result
+}
+
+fun getNonPrivateTraitMembersForDelegation(
+    descriptor: CallableMemberDescriptor,
+    returnImplNotDelegate: Boolean = false,
+): CallableMemberDescriptor? {
+    val traitMember = findInterfaceImplementation(descriptor, returnImplNotDelegate)
+    if (traitMember == null ||
+        Visibilities.isPrivate(traitMember.visibility) ||
+        traitMember.visibility == Visibilities.INVISIBLE_FAKE
+    ) return null
+    return traitMember
 }

compiler/testData/compileKotlinAgainstCustomBinaries/jvmDefaultClashWithOld/library/a.kt

@@ -0,0 +1,13 @@
+package base
+
+interface Check {
+    fun test(): String {
+        return "fail";
+    }
+
+    var test: String
+        get() = "123"
+        set(value) { value.length}
+}
+
+open class CheckClass : Check

compiler/testData/compileKotlinAgainstCustomBinaries/jvmDefaultClashWithOld/output.txt

@@ -0,0 +1,7 @@
+compiler/testData/compileKotlinAgainstCustomBinaries/jvmDefaultClashWithOld/source.kt:15:7: error: explicit override is required for 'public open var test: String defined in SubCheckClass' in the '-Xjvm-default=all' mode. Otherwise, implicit class override 'public open var test: String defined in base.CheckClass' (compiled in the old -Xjvm-default mode) is not fully overridden and might be incorrectly called at runtime
+class SubCheckClass : CheckClass(), SubCheck
+      ^
+compiler/testData/compileKotlinAgainstCustomBinaries/jvmDefaultClashWithOld/source.kt:15:7: error: explicit override is required for 'public open fun test(): String defined in SubCheckClass' in the '-Xjvm-default=all' mode. Otherwise, implicit class override 'public open fun test(): String defined in base.CheckClass' (compiled in the old -Xjvm-default mode) is not fully overridden and might be incorrectly called at runtime
+class SubCheckClass : CheckClass(), SubCheck
+      ^
+COMPILATION_ERROR

compiler/testData/compileKotlinAgainstCustomBinaries/jvmDefaultClashWithOld/source.kt

@@ -0,0 +1,15 @@
+import base.*
+
+interface SubCheck : Check {
+    override fun test(): String {
+        return "OK"
+    }
+
+    override var test: String
+        get() = "OK"
+        set(value) {
+            value.length
+        }
+}
+
+class SubCheckClass : CheckClass(), SubCheck

compiler/testData/compileKotlinAgainstKotlin/jvm8/defaults/interop/likeMemberClash.kt

@@ -0,0 +1,26 @@
+// FULL_JDK
+// FILE: 1.kt
+// !JVM_DEFAULT_MODE: disable
+interface Check {
+    fun test(): String {
+        return "fail";
+    }
+}
+
+interface SubCheck : Check {
+    override fun test(): String {
+        return "OK"
+    }
+}
+
+
+open class CheckClass : Check
+
+// FILE: main.kt
+// !JVM_DEFAULT_MODE: all
+// JVM_TARGET: 1.8
+class SubCheckClass : CheckClass(), SubCheck
+
+fun box(): String {
+    return SubCheckClass().test()
+}

compiler/testData/compileKotlinAgainstKotlin/jvm8/defaults/interop/likeSpecialization.kt

@@ -0,0 +1,21 @@
+// WITH_RUNTIME
+// JVM_TARGET: 1.8
+// FILE: 1.kt
+// !JVM_DEFAULT_MODE: disable
+
+interface Foo<T> {
+    fun test(p: T) = p
+    val T.prop: String
+        get() = "K"
+}
+
+interface FooDerived: Foo<String>
+
+// FILE: main.kt
+// !JVM_DEFAULT_MODE: all-compatibility
+open class UnspecializedFromDerived : FooDerived
+
+fun box(): String {
+    val foo = UnspecializedFromDerived()
+    return foo.test("O") + with(foo) { "K".prop }
+}

compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstKotlinTestGenerated.java

@@ -510,6 +510,16 @@ public class CompileKotlinAgainstKotlinTestGenerated extends AbstractCompileKotl
                     KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/compileKotlinAgainstKotlin/jvm8/defaults/interop"), Pattern.compile("^(.+)\\.kt$"), null, true);
                 }
 
+                @TestMetadata("likeMemberClash.kt")
+                public void testLikeMemberClash() throws Exception {
+                    runTest("compiler/testData/compileKotlinAgainstKotlin/jvm8/defaults/interop/likeMemberClash.kt");
+                }
+
+                @TestMetadata("likeSpecialization.kt")
+                public void testLikeSpecialization() throws Exception {
+                    runTest("compiler/testData/compileKotlinAgainstKotlin/jvm8/defaults/interop/likeSpecialization.kt");
+                }
+
                 @TestMetadata("newAndOldSchemes.kt")
                 public void testNewAndOldSchemes() throws Exception {
                     runTest("compiler/testData/compileKotlinAgainstKotlin/jvm8/defaults/interop/newAndOldSchemes.kt");

compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstKotlinTestGenerated.java

@@ -505,6 +505,16 @@ public class IrCompileKotlinAgainstKotlinTestGenerated extends AbstractIrCompile
                     KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/compileKotlinAgainstKotlin/jvm8/defaults/interop"), Pattern.compile("^(.+)\\.kt$"), null, TargetBackend.JVM_IR, true);
                 }
 
+                @TestMetadata("likeMemberClash.kt")
+                public void testLikeMemberClash() throws Exception {
+                    runTest("compiler/testData/compileKotlinAgainstKotlin/jvm8/defaults/interop/likeMemberClash.kt");
+                }
+
+                @TestMetadata("likeSpecialization.kt")
+                public void testLikeSpecialization() throws Exception {
+                    runTest("compiler/testData/compileKotlinAgainstKotlin/jvm8/defaults/interop/likeSpecialization.kt");
+                }
+
                 @TestMetadata("newAndOldSchemes.kt")
                 public void testNewAndOldSchemes() throws Exception {
                     runTest("compiler/testData/compileKotlinAgainstKotlin/jvm8/defaults/interop/newAndOldSchemes.kt");

compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.kt

@@ -627,6 +627,11 @@ class CompileKotlinAgainstCustomBinariesTest : AbstractKotlinCompilerIntegration
         compileKotlin("source.kt", tmpdir, listOf(library), additionalOptions = listOf("-Xfriend-paths=${library.path}"))
     }
 
+    fun testJvmDefaultClashWithOld() {
+        val library = compileLibrary("library", additionalOptions = listOf("-Xjvm-default=disable"))
+        compileKotlin("source.kt", tmpdir, listOf(library), additionalOptions = listOf("-jvm-target", "1.8", "-Xjvm-default=all"))
+    }
+
     fun testInternalFromForeignModuleJs() {
         compileKotlin("source.kt", File(tmpdir, "usage.js"), listOf(compileJsLibrary("library")), K2JSCompiler())
     }

idea/idea-analysis/src/org/jetbrains/kotlin/idea/compiler/IdeLanguageVersionSettingsProvider.kt

@@ -0,0 +1,21 @@
+/*
+ * Copyright 2010-2020 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.idea.compiler
+
+import org.jetbrains.kotlin.analyzer.moduleInfo
+import org.jetbrains.kotlin.config.LanguageVersionSettings
+import org.jetbrains.kotlin.descriptors.ModuleDescriptor
+import org.jetbrains.kotlin.idea.caches.project.ModuleSourceInfo
+import org.jetbrains.kotlin.idea.project.languageVersionSettings
+import org.jetbrains.kotlin.resolve.LanguageVersionSettingsProvider
+
+class IdeLanguageVersionSettingsProvider : LanguageVersionSettingsProvider {
+    override fun getModuleLanguageVersionSettings(module: ModuleDescriptor): LanguageVersionSettings? {
+        // downcast to module source info, as we're not interested in non-source classes
+        val moduleInfo = module.moduleInfo as? ModuleSourceInfo ?: return null
+        return moduleInfo.module.languageVersionSettings
+    }
+}

idea/resources/META-INF/plugin-common.xml

@@ -196,6 +196,9 @@
     <projectService serviceInterface="org.jetbrains.kotlin.resolve.ModuleAnnotationsResolver"
                     serviceImplementation="org.jetbrains.kotlin.idea.compiler.IdeModuleAnnotationsResolver"/>
 
+    <projectService serviceInterface="org.jetbrains.kotlin.resolve.LanguageVersionSettingsProvider"
+                    serviceImplementation="org.jetbrains.kotlin.idea.compiler.IdeLanguageVersionSettingsProvider"/>
+
     <projectService serviceInterface="org.jetbrains.kotlin.asJava.LightClassGenerationSupport"
                     serviceImplementation="org.jetbrains.kotlin.idea.caches.resolve.IDELightClassGenerationSupport"/>