Report interop functions with non-stable parameter names

^KT-34602
2020-07-22 17:32:22 +07:00
parent 3d9093583f
commit b47946cbba
9 changed files with 217 additions and 9 deletions
@@ -714,7 +714,8 @@ public interface Errors {
    DiagnosticFactory1<PsiElement, BadNamedArgumentsTarget> NAMED_ARGUMENTS_NOT_ALLOWED = DiagnosticFactory1.create(ERROR);

    enum BadNamedArgumentsTarget {
-        NON_KOTLIN_FUNCTION,
+        NON_KOTLIN_FUNCTION, // a function provided by non-Kotlin artifact, ex: Java function
+        INTEROP_FUNCTION, // deserialized Kotlin function that serves as a bridge to a function written in another language, ex: Obj-C
        INVOKE_ON_FUNCTION_TYPE,
        EXPECTED_CLASS_MEMBER,
    }
@@ -197,6 +197,8 @@ public class DefaultErrorMessages {
            switch (target) {
                case NON_KOTLIN_FUNCTION:
                    return "non-Kotlin functions";
+                case INTEROP_FUNCTION:
+                    return "interop functions with ambiguous parameter names";
                case INVOKE_ON_FUNCTION_TYPE:
                    return "function types";
                case EXPECTED_CLASS_MEMBER:
@@ -11,8 +11,7 @@ import org.jetbrains.kotlin.builtins.isExtensionFunctionType
 import org.jetbrains.kotlin.config.LanguageFeature
 import org.jetbrains.kotlin.diagnostics.Errors
 import org.jetbrains.kotlin.diagnostics.Errors.*
-import org.jetbrains.kotlin.diagnostics.Errors.BadNamedArgumentsTarget.INVOKE_ON_FUNCTION_TYPE
-import org.jetbrains.kotlin.diagnostics.Errors.BadNamedArgumentsTarget.NON_KOTLIN_FUNCTION
+import org.jetbrains.kotlin.diagnostics.Errors.BadNamedArgumentsTarget.*
 import org.jetbrains.kotlin.diagnostics.reportDiagnosticOnce
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.isNull
@@ -32,6 +31,7 @@ import org.jetbrains.kotlin.resolve.constants.TypedCompileTimeConstant
 import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator
 import org.jetbrains.kotlin.resolve.descriptorUtil.module
 import org.jetbrains.kotlin.resolve.scopes.receivers.ExpressionReceiver
+import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedCallableMemberDescriptor
 import org.jetbrains.kotlin.types.KotlinType
 import org.jetbrains.kotlin.types.checker.intersectWrappedTypes
 import org.jetbrains.kotlin.types.expressions.ControlStructureTypingUtils
@@ -260,7 +260,11 @@ class DiagnosticReporterByTrackingStrategy(
            NamedArgumentNotAllowed::class.java -> trace.report(
                NAMED_ARGUMENTS_NOT_ALLOWED.on(
                    nameReference,
-                    if ((diagnostic as NamedArgumentNotAllowed).descriptor is FunctionInvokeDescriptor) INVOKE_ON_FUNCTION_TYPE else NON_KOTLIN_FUNCTION
+                    when ((diagnostic as NamedArgumentNotAllowed).descriptor) {
+                        is FunctionInvokeDescriptor -> INVOKE_ON_FUNCTION_TYPE
+                        is DeserializedCallableMemberDescriptor -> INTEROP_FUNCTION
+                        else -> NON_KOTLIN_FUNCTION
+                    }
                )
            )
            ArgumentPassedTwice::class.java -> trace.report(ARGUMENT_PASSED_TWICE.on(nameReference))
@@ -31,6 +31,7 @@ import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilKt;
 import org.jetbrains.kotlin.resolve.calls.components.ArgumentsUtilsKt;
 import org.jetbrains.kotlin.resolve.calls.model.*;
 import org.jetbrains.kotlin.resolve.calls.tasks.TracingStrategy;
+import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedCallableMemberDescriptor;

 import java.util.*;

@@ -185,10 +186,16 @@ public class ValueArgumentsToParametersMapper {
                        report(NAMED_ARGUMENTS_NOT_ALLOWED.on(nameReference, EXPECTED_CLASS_MEMBER));
                    }
                    else if (!candidate.hasStableParameterNames()) {
-                        report(NAMED_ARGUMENTS_NOT_ALLOWED.on(
-                                nameReference,
-                                candidate instanceof FunctionInvokeDescriptor ? INVOKE_ON_FUNCTION_TYPE : NON_KOTLIN_FUNCTION
-                        ));
+                        BadNamedArgumentsTarget badNamedArgumentsTarget;
+                        if (candidate instanceof FunctionInvokeDescriptor) {
+                            badNamedArgumentsTarget = INVOKE_ON_FUNCTION_TYPE;
+                        } else if (candidate instanceof DeserializedCallableMemberDescriptor) {
+                            badNamedArgumentsTarget = INTEROP_FUNCTION;
+                        } else {
+                            badNamedArgumentsTarget = NON_KOTLIN_FUNCTION;
+                        }
+
+                        report(NAMED_ARGUMENTS_NOT_ALLOWED.on(nameReference, badNamedArgumentsTarget));
                    }
                }

@@ -0,0 +1,15 @@
+package library
+
+fun foo(a: Int, b: String, c: Boolean) = Unit
+
+class Foo(a: Int, b: String, c: Boolean) {
+    constructor(a: Int, b: String, c: Boolean, d: Double) : this(a, b, c)
+
+    fun foo(a: Int, b: String, c: Boolean) = Unit
+
+    class Bar(a: Int, b: String, c: Boolean) {
+        constructor(a: Int, b: String, c: Boolean, d: Double) : this(a, b, c)
+
+        fun bar(a: Int, b: String, c: Boolean) = Unit
+    }
+}
@@ -0,0 +1,48 @@
+// !WITH_NEW_INFERENCE
+// !LANGUAGE: +NewInference
+// MODULE: m1-common
+// FILE: test.kt
+
+import library.*
+
+fun test() {
+    foo(
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>a<!> = 42,
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>b<!> = "hello",
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>c<!> = true
+    )
+
+    Foo(
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>a<!> = 42,
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>b<!> = "hello",
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>c<!> = true
+    ).foo(
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>a<!> = 42,
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>b<!> = "hello",
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>c<!> = true
+    )
+
+    Foo(
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>a<!> = 42,
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>b<!> = "hello",
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>c<!> = true,
+        3.14
+    )
+
+    Foo.Bar(
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>a<!> = 42,
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>b<!> = "hello",
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>c<!> = true
+    ).bar(
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>a<!> = 42,
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>b<!> = "hello",
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>c<!> = true
+    )
+
+    Foo.Bar(
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>a<!> = 42,
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>b<!> = "hello",
+        <!NAMED_ARGUMENTS_NOT_ALLOWED!>c<!> = true,
+        3.14
+    )
+}
@@ -0,0 +1,3 @@
+package
+
+public fun test(): kotlin.Unit
@@ -0,0 +1,128 @@
+/*
+ * 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.checkers
+
+import org.jetbrains.kotlin.analyzer.AnalysisResult
+import org.jetbrains.kotlin.analyzer.ModuleInfo
+import org.jetbrains.kotlin.analyzer.common.CommonDependenciesContainer
+import org.jetbrains.kotlin.analyzer.common.CommonPlatformAnalyzerServices
+import org.jetbrains.kotlin.analyzer.common.CommonResolverForModuleFactory
+import org.jetbrains.kotlin.builtins.DefaultBuiltIns
+import org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles
+import org.jetbrains.kotlin.config.JvmTarget
+import org.jetbrains.kotlin.config.LanguageVersionSettings
+import org.jetbrains.kotlin.context.ModuleContext
+import org.jetbrains.kotlin.descriptors.ModuleDescriptor
+import org.jetbrains.kotlin.descriptors.PackageFragmentProvider
+import org.jetbrains.kotlin.descriptors.impl.ModuleDescriptorImpl
+import org.jetbrains.kotlin.name.Name
+import org.jetbrains.kotlin.platform.CommonPlatforms
+import org.jetbrains.kotlin.platform.TargetPlatform
+import org.jetbrains.kotlin.psi.KtFile
+import org.jetbrains.kotlin.resolve.BindingTrace
+import org.jetbrains.kotlin.resolve.PlatformDependentAnalyzerServices
+import org.jetbrains.kotlin.serialization.NonStableParameterNamesSerializationTest
+import org.jetbrains.kotlin.storage.StorageManager
+import org.jetbrains.kotlin.test.KlibTestUtil
+import org.jetbrains.kotlin.test.KotlinTestUtils
+import java.io.File
+
+class InteropFunctionsWithNonStableParameterNamesDiagnosticsTest : AbstractDiagnosticsTest() {
+    private lateinit var klibFile: File
+
+    override fun setUp() {
+        super.setUp()
+
+        val tmpDir = KotlinTestUtils.tmpDirForTest(this)
+        klibFile = prepareKlibWithNonStableParameterNames(tmpDir)
+    }
+
+    override fun shouldSkipJvmSignatureDiagnostics(groupedByModule: Map<TestModule?, List<TestFile>>) = true
+
+    override fun getEnvironmentConfigFiles(): EnvironmentConfigFiles = EnvironmentConfigFiles.METADATA_CONFIG_FILES
+
+    override fun createModule(moduleName: String, storageManager: StorageManager): ModuleDescriptorImpl =
+        ModuleDescriptorImpl(Name.special("<$moduleName>"), storageManager, DefaultBuiltIns.Instance)
+
+    override fun getAdditionalDependencies(module: ModuleDescriptorImpl): List<ModuleDescriptorImpl> =
+        listOf(KlibTestUtil.deserializeKlibToCommonModule(klibFile))
+
+    override fun analyzeModuleContents(
+        moduleContext: ModuleContext,
+        files: List<KtFile>,
+        moduleTrace: BindingTrace,
+        languageVersionSettings: LanguageVersionSettings,
+        separateModules: Boolean,
+        jvmTarget: JvmTarget
+    ): AnalysisResult {
+        return CommonResolverForModuleFactory.analyzeFiles(
+            files = files,
+            moduleName = moduleContext.module.name,
+            dependOnBuiltIns = true,
+            languageVersionSettings = languageVersionSettings,
+            targetPlatform = CommonPlatforms.defaultCommonPlatform,
+            capabilities = mapOf(
+//                MODULE_FILES to files
+            ),
+            dependenciesContainer = CommonDependenciesContainerImpl(moduleContext.module.allDependencyModules)
+        ) { content ->
+            environment.createPackagePartProvider(content.moduleContentScope)
+        }
+    }
+
+    fun testInteropFunctionsWithNonStableParameterNames() {
+        doTest(File(TEST_DATA_DIR, "test.kt").path)
+    }
+
+    private class CommonDependenciesContainerImpl(dependees: Collection<ModuleDescriptor>) : CommonDependenciesContainer {
+        private class ModuleInfoImpl(val module: ModuleDescriptor) : ModuleInfo {
+            override val name: Name get() = module.name
+
+            override fun dependencies(): List<ModuleInfo> = listOf(this)
+            override fun dependencyOnBuiltIns(): ModuleInfo.DependencyOnBuiltIns = ModuleInfo.DependencyOnBuiltIns.LAST
+
+            override val platform: TargetPlatform get() = CommonPlatforms.defaultCommonPlatform
+            override val analyzerServices: PlatformDependentAnalyzerServices get() = CommonPlatformAnalyzerServices
+        }
+
+        private val dependeeModuleInfos: List<ModuleInfoImpl> = dependees.map(::ModuleInfoImpl)
+
+        override val moduleInfos: List<ModuleInfo> get() = dependeeModuleInfos
+
+        override fun moduleDescriptorForModuleInfo(moduleInfo: ModuleInfo): ModuleDescriptor {
+            // let's assume there is a few module infos at all
+            return dependeeModuleInfos.firstOrNull { it === moduleInfo }?.module
+                ?: error("Unknown module info $moduleInfo")
+        }
+
+        override fun registerDependencyForAllModules(moduleInfo: ModuleInfo, descriptorForModule: ModuleDescriptorImpl) = Unit
+        override fun packageFragmentProviderForModuleInfo(moduleInfo: ModuleInfo): PackageFragmentProvider? = null
+
+        override val friendModuleInfos: List<ModuleInfo> get() = emptyList()
+        override val refinesModuleInfos: List<ModuleInfo> get() = dependeeModuleInfos
+    }
+
+    companion object {
+        private const val TEST_DATA_DIR = "compiler/testData/diagnostics/nonStableParameterNames"
+
+        private fun prepareKlibWithNonStableParameterNames(tmpDir: File): File {
+            val libraryName = "library"
+
+            val klibFile = tmpDir.resolve("$libraryName.klib")
+            KlibTestUtil.compileCommonSourcesToKlib(listOf(File(TEST_DATA_DIR, "library.kt")), libraryName, klibFile)
+
+            val module = KlibTestUtil.deserializeKlibToCommonModule(klibFile)
+            NonStableParameterNamesSerializationTest.collectCallablesForPatch(module).forEach { it.setHasStableParameterNames(false) }
+
+            klibFile.delete()
+            KlibTestUtil.serializeCommonModuleToKlib(module, libraryName, klibFile)
+
+            return klibFile
+        }
+
+    }
+}
+
@@ -51,7 +51,7 @@ class NonStableParameterNamesSerializationTest : TestCaseWithTmpdir() {
    companion object {
        private const val SOURCE_FILE = "compiler/testData/serialization/nonStableParameterNames/test.kt"

-        private fun collectCallablesForPatch(module: ModuleDescriptorImpl): List<FunctionDescriptorImpl> {
+        fun collectCallablesForPatch(module: ModuleDescriptorImpl): List<FunctionDescriptorImpl> {
            fun DeclarationDescriptor.castToFunctionImpl(): FunctionDescriptorImpl =
                assertedCast { "Not an instance of ${FunctionDescriptorImpl::class.java}: ${this::class.java}, $this" }