JS: prohibit overriding external functions with optional parameters by non-external fake functions

compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.kt

@@ -15,6 +15,10 @@ external interface I {
     fun f(x: Int = noImpl)
 }
 
+interface J {
+    fun f(x: Int = 23)
+}
+
 open external class D {
     open fun f(x: Int)
 }
@@ -29,4 +33,18 @@ class F : D(), I {
 
 external class G : D(), I {
     override fun f(x: Int) {}
-}
+}
+
+open class X {
+    fun f(<!UNUSED_PARAMETER!>x<!>: Int) {}
+}
+
+open external class XE {
+    fun f(x: Int)
+}
+
+class <!OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS_WITH_FAKE!>Y<!> : X(), I
+
+external class YE: XE(), I
+
+class Z : X(), J

compiler/testData/diagnostics/testsWithJsStdLib/native/overrideOptionalParam.txt

@@ -62,3 +62,50 @@ public external interface I {
     public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
     public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
 }
+
+public interface J {
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public abstract fun f(/*0*/ x: kotlin.Int = ...): kotlin.Unit
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public open class X {
+    public constructor X()
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public final fun f(/*0*/ x: kotlin.Int): kotlin.Unit
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public external open class XE {
+    public constructor XE()
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public final fun f(/*0*/ x: kotlin.Int): kotlin.Unit
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public final class Y : X, I {
+    public constructor Y()
+    public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public final override /*2*/ /*fake_override*/ fun f(/*0*/ x: kotlin.Int = ...): kotlin.Unit
+    public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public external final class YE : XE, I {
+    public constructor YE()
+    public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public final override /*2*/ /*fake_override*/ fun f(/*0*/ x: kotlin.Int = ...): kotlin.Unit
+    public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public final class Z : X, J {
+    public constructor Z()
+    public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public final override /*2*/ /*fake_override*/ fun f(/*0*/ x: kotlin.Int = ...): kotlin.Unit
+    public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String
+}

js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt

@@ -69,6 +69,8 @@ private val DIAGNOSTIC_FACTORY_TO_RENDERER by lazy {
         put(ErrorsJs.RUNTIME_ANNOTATION_NOT_SUPPORTED, "Reflection is not supported in JavaScript target, therefore you won't be able " +
                                                        "to read this annotation in run-time")
         put(ErrorsJs.OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS, "Overriding `external` function with optional parameters")
+        put(ErrorsJs.OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS_WITH_FAKE,
+            "Overriding `external` function with optional parameters by declaration from superclass: {0}", Renderers.COMPACT)
 
         this
     }

js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java

@@ -18,6 +18,7 @@ package org.jetbrains.kotlin.js.resolve.diagnostics;
 
 import com.intellij.psi.PsiElement;
 import org.jetbrains.kotlin.descriptors.DeclarationDescriptor;
+import org.jetbrains.kotlin.descriptors.FunctionDescriptor;
 import org.jetbrains.kotlin.diagnostics.*;
 import org.jetbrains.kotlin.psi.KtDeclaration;
 import org.jetbrains.kotlin.psi.KtElement;
@@ -70,6 +71,8 @@ public interface ErrorsJs {
     DiagnosticFactory0<PsiElement> RUNTIME_ANNOTATION_NOT_SUPPORTED = DiagnosticFactory0.create(WARNING, DECLARATION_SIGNATURE_OR_DEFAULT);
     DiagnosticFactory0<KtElement> OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS =
             DiagnosticFactory0.create(ERROR, PositioningStrategies.DECLARATION_SIGNATURE_OR_DEFAULT);
+    DiagnosticFactory1<KtElement, FunctionDescriptor> OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS_WITH_FAKE =
+            DiagnosticFactory1.create(ERROR, PositioningStrategies.DECLARATION_SIGNATURE_OR_DEFAULT);
 
     @SuppressWarnings("UnusedDeclaration")
     Object _initializer = new Object() {

js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsInheritanceChecker.kt

@@ -16,14 +16,14 @@
 
 package org.jetbrains.kotlin.js.resolve.diagnostics
 
-import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
-import org.jetbrains.kotlin.descriptors.FunctionDescriptor
+import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.diagnostics.DiagnosticSink
 import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.checkers.SimpleDeclarationChecker
 import org.jetbrains.kotlin.resolve.descriptorUtil.hasDefaultValue
+import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter
 
 object JsInheritanceChecker : SimpleDeclarationChecker {
     override fun check(
@@ -37,10 +37,17 @@ object JsInheritanceChecker : SimpleDeclarationChecker {
         ) {
             diagnosticHolder.report(ErrorsJs.OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS.on(declaration))
         }
+        else if (descriptor is ClassDescriptor && !DescriptorUtils.isEffectivelyExternal(descriptor)) {
+            val fakeOverriddenMethod = findFakeMethodOverridingExternalWithOptionalParams(descriptor)
+            if (fakeOverriddenMethod != null) {
+                diagnosticHolder.report(ErrorsJs.OVERRIDING_EXTERNAL_FUN_WITH_OPTIONAL_PARAMS_WITH_FAKE.on(
+                        declaration, fakeOverriddenMethod))
+            }
+        }
     }
 
     private fun isOverridingExternalWithOptionalParams(function: FunctionDescriptor): Boolean {
-        if (!function.kind.isReal) return false
+        if (!function.kind.isReal && function.modality == Modality.ABSTRACT) return false
 
         for (overriddenFunction in function.overriddenDescriptors.filter { DescriptorUtils.isEffectivelyExternal(it) }) {
             if (overriddenFunction.valueParameters.any { it.hasDefaultValue() }) return true
@@ -48,4 +55,12 @@ object JsInheritanceChecker : SimpleDeclarationChecker {
 
         return false
     }
+
+    private fun findFakeMethodOverridingExternalWithOptionalParams(cls: ClassDescriptor): FunctionDescriptor? {
+        val members = cls.unsubstitutedMemberScope.getContributedDescriptors(DescriptorKindFilter.CALLABLES)
+                .mapNotNull { it as? FunctionDescriptor }
+                .filter { it.containingDeclaration == cls && !it.kind.isReal }
+
+        return members.firstOrNull { isOverridingExternalWithOptionalParams(it) }
+    }
 }