KT-2752: minor fixes after code review

compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt

@@ -25,11 +25,8 @@ import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.MemberComparator
 import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
-import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns
 import org.jetbrains.kotlin.types.KotlinType
-import org.jetbrains.kotlin.types.checker.KotlinTypeChecker
 import org.jetbrains.kotlin.types.isDynamic
-import org.jetbrains.kotlin.types.typeUtil.makeNullable
 import org.jetbrains.kotlin.utils.keysToMapExceptNulls
 
 object CodegenUtil {
@@ -153,15 +150,6 @@ object CodegenUtil {
                ?: error("ClassDescriptor of superType should not be null: ${specifier.text}")
     }
 
-    @JvmStatic
-    fun isEnumValueOfMethod(functionDescriptor: FunctionDescriptor): Boolean {
-        val methodTypeParameters = functionDescriptor.valueParameters
-        val nullableString = functionDescriptor.builtIns.stringType.makeNullable()
-        return DescriptorUtils.ENUM_VALUE_OF == functionDescriptor.name
-               && methodTypeParameters.size == 1
-               && KotlinTypeChecker.DEFAULT.isSubtypeOf(methodTypeParameters[0].type, nullableString)
-    }
-
     @JvmStatic
     fun getLineNumberForElement(statement: PsiElement, markEndOffset: Boolean): Int? {
         val file = statement.containingFile

compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java

@@ -820,7 +820,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
                 CollectionsKt.single(descriptor.getStaticScope().getContributedFunctions(ENUM_VALUE_OF, NoLookupLocation.FROM_BACKEND), new Function1<FunctionDescriptor, Boolean>() {
                     @Override
                     public Boolean invoke(FunctionDescriptor descriptor) {
-                        return CodegenUtil.isEnumValueOfMethod(descriptor);
+                        return DescriptorUtilsKt.isEnumValueOfMethod(descriptor);
                     }
                 });
         MethodVisitor mv = v.newMethod(JvmDeclarationOriginKt.OtherOrigin(myClass, valueOfFunction), ACC_PUBLIC | ACC_STATIC, ENUM_VALUE_OF.asString(),

core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt

@@ -29,10 +29,8 @@ import org.jetbrains.kotlin.name.FqNameUnsafe
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.constants.EnumValue
-import org.jetbrains.kotlin.types.KotlinType
+import org.jetbrains.kotlin.types.*
-import org.jetbrains.kotlin.types.TypeProjection
+import org.jetbrains.kotlin.types.checker.KotlinTypeChecker
-import org.jetbrains.kotlin.types.TypeSubstitutor
-import org.jetbrains.kotlin.types.Variance
 import org.jetbrains.kotlin.types.typeUtil.isAnyOrNullableAny
 import org.jetbrains.kotlin.utils.DFS
 import org.jetbrains.kotlin.utils.SmartList

js/js.frontend/src/org/jetbrains/kotlin/js/descriptorUtils/descriptorUtils.kt

@@ -18,15 +18,12 @@ package org.jetbrains.kotlin.js.descriptorUtils
 
 import com.intellij.openapi.util.text.StringUtil
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
-import org.jetbrains.kotlin.descriptors.FunctionDescriptor
 import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor
 import org.jetbrains.kotlin.descriptors.TypeParameterDescriptor
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns
 import org.jetbrains.kotlin.types.KotlinType
-import org.jetbrains.kotlin.types.TypeUtils
-import org.jetbrains.kotlin.types.checker.KotlinTypeChecker
 import org.jetbrains.kotlin.utils.addToStdlib.check
 
 val KotlinType.nameIfStandardType: Name?
@@ -59,11 +56,3 @@ fun KotlinType.getJetTypeFqName(printTypeArguments: Boolean): String {
 }
 
 fun ClassDescriptor.hasPrimaryConstructor(): Boolean = unsubstitutedPrimaryConstructor != null
-
-fun FunctionDescriptor.isEnumValueOfMethod(): Boolean {
-    val methodTypeParameters = valueParameters
-    val nullableString = TypeUtils.makeNullable(builtIns.stringType)
-    return DescriptorUtils.ENUM_VALUE_OF == name
-           && methodTypeParameters.size == 1
-           && KotlinTypeChecker.DEFAULT.isSubtypeOf(methodTypeParameters[0].type, nullableString)
-}

js/js.frontend/src/org/jetbrains/kotlin/js/naming/NameSuggestion.kt

@@ -18,12 +18,12 @@ package org.jetbrains.kotlin.js.naming
 
 import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.js.descriptorUtils.getJetTypeFqName
-import org.jetbrains.kotlin.js.descriptorUtils.isEnumValueOfMethod
 import org.jetbrains.kotlin.js.translate.utils.AnnotationsUtils.*
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.DescriptorUtils.isCompanionObject
 import org.jetbrains.kotlin.resolve.calls.tasks.isDynamic
 import org.jetbrains.kotlin.resolve.calls.util.FakeCallableDescriptorForObject
+import org.jetbrains.kotlin.resolve.descriptorUtil.isEnumValueOfMethod
 import java.util.*
 
 /**
@@ -32,7 +32,7 @@ import java.util.*
  * to check whether names clash, by the code generator to place declarations to corresponding scopes and to produce
  * fully-qualified names for static declarations.
  *
- * A new instance of this class can be created for each request, however, it's recommended to use shared instance, since
+ * A new instance of this class can be created for each request, however, it's recommended to use stable instance, since
  * [NameSuggestion] supports caching.
  */
 class NameSuggestion {
@@ -70,7 +70,7 @@ class NameSuggestion {
             is ModuleDescriptor -> return null
 
             is PackageFragmentDescriptor -> {
-                return if (!descriptor.name.isSpecial) {
+                return if (!descriptor.fqName.isRoot) {
                     SuggestedName(descriptor.fqName.pathSegments().map { it.asString() }, true, descriptor,
                                   descriptor.containingDeclaration)
                 }
@@ -102,7 +102,7 @@ class NameSuggestion {
     }
 
     private fun generateDefault(descriptor: DeclarationDescriptor): SuggestedName {
-        // Dynamic declarations always require shared names as defined in Kotlin source code
+        // Dynamic declarations always require stable names as defined in Kotlin source code
         if (descriptor.isDynamic()) {
             return SuggestedName(listOf(descriptor.name.asString()), true, descriptor, descriptor.containingDeclaration!!)
         }
@@ -167,7 +167,7 @@ class NameSuggestion {
     }
 
     companion object {
-        @JvmStatic private fun getMangledName(baseName: String, descriptor: DeclarationDescriptor): Pair<String, Boolean> {
+        private fun getMangledName(baseName: String, descriptor: DeclarationDescriptor): Pair<String, Boolean> {
             // If we have a callable descriptor (property or method) it can override method in a parent class.
             // Traverse to the topmost overridden method.
             // It does not matter which path to choose during traversal, since front-end must ensure
@@ -193,7 +193,7 @@ class NameSuggestion {
             return Pair(finalName, stable)
         }
 
-        @JvmStatic fun getArgumentTypesAsString(descriptor: CallableDescriptor): String {
+        private fun getArgumentTypesAsString(descriptor: CallableDescriptor): String {
             val argTypes = StringBuilder()
 
             val receiverParameter = descriptor.extensionReceiverParameter
@@ -206,7 +206,7 @@ class NameSuggestion {
             return argTypes.toString()
         }
 
-        @JvmStatic fun getStableMangledName(suggestedName: String, forCalculateId: String): String {
+        fun getStableMangledName(suggestedName: String, forCalculateId: String): String {
             val suffix = if (forCalculateId.isEmpty()) "" else "_${mangledId(forCalculateId)}\$"
             return suggestedName + suffix
         }
@@ -227,9 +227,7 @@ class NameSuggestion {
                 is PackageFragmentDescriptor -> descriptor.visibility.isPublicAPI
                 is ClassDescriptor -> {
                     // Open (abstract) public methods of classes or final public methods of open (abstract) classes should be stable
-                    if (descriptor.modality == Modality.OPEN || descriptor.modality == Modality.ABSTRACT ||
+                    if (containingDeclaration.modality == Modality.OPEN || containingDeclaration.modality == Modality.ABSTRACT) {
-                        containingDeclaration.modality == Modality.OPEN || containingDeclaration.modality == Modality.ABSTRACT
-                    ) {
                         return descriptor.visibility.isPublicAPI
                     }
 
@@ -255,7 +253,7 @@ class NameSuggestion {
             }
         }
 
-        @JvmStatic fun mangledId(forCalculateId: String): String {
+        private fun mangledId(forCalculateId: String): String {
             val absHashCode = Math.abs(forCalculateId.hashCode())
             return if (absHashCode != 0) Integer.toString(absHashCode, Character.MAX_RADIX) else ""
         }

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

@@ -46,8 +46,8 @@ public interface ErrorsJs {
             ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
     DiagnosticFactory3<KtElement, String, DeclarationDescriptor, DeclarationDescriptor> JS_NAME_CLASH_SYNTHETIC =
             DiagnosticFactory3.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
-    DiagnosticFactory0<KtElement> JS_NAME_ON_PRIMARY_CONSTRUCTOR_PROHIBITED = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory0<PsiElement> JS_NAME_ON_PRIMARY_CONSTRUCTOR_PROHIBITED = DiagnosticFactory0.create(ERROR);
-    DiagnosticFactory0<KtElement> JS_NAME_ON_ACCESSOR_AND_PROPERTY = DiagnosticFactory0.create(ERROR);
+    DiagnosticFactory0<PsiElement> JS_NAME_ON_ACCESSOR_AND_PROPERTY = DiagnosticFactory0.create(ERROR);
     DiagnosticFactory0<PsiElement> JS_NAME_IS_NOT_ON_ALL_ACCESSORS = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
     DiagnosticFactory0<PsiElement> JS_NAME_PROHIBITED_FOR_OVERRIDE = DiagnosticFactory0.create(ERROR);
     DiagnosticFactory0<PsiElement> JS_NAME_PROHIBITED_FOR_EXTENSION_PROPERTY = DiagnosticFactory0.create(ERROR);

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

@@ -22,9 +22,9 @@ import org.jetbrains.kotlin.js.PredefinedAnnotation
 import org.jetbrains.kotlin.js.translate.utils.AnnotationsUtils
 import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.resolve.BindingContext
-import org.jetbrains.kotlin.resolve.DeclarationChecker
-import org.jetbrains.kotlin.resolve.descriptorUtil.isExtension
 import org.jetbrains.kotlin.resolve.SimpleDeclarationChecker
+import org.jetbrains.kotlin.resolve.descriptorUtil.isExtension
+import org.jetbrains.kotlin.resolve.source.getPsi
 
 object JsNameChecker : SimpleDeclarationChecker {
     override fun check(declaration: KtDeclaration, descriptor: DeclarationDescriptor, diagnosticHolder: DiagnosticSink,
@@ -36,12 +36,10 @@ object JsNameChecker : SimpleDeclarationChecker {
             }
         }
 
-        if (AnnotationsUtils.getJsName(descriptor) == null) return
+        val jsName = AnnotationsUtils.getJsNameAnnotation(descriptor) ?: return
-        val jsNamePsi = AnnotationsUtils.getJsNameAnnotationPsi(bindingContext, declaration, descriptor) ?: return
+        val jsNamePsi = jsName.source.getPsi() ?: declaration
 
-        if (AnnotationsUtils.isNativeObject(descriptor) &&
+        if (AnnotationsUtils.getNameForAnnotatedObject(descriptor, PredefinedAnnotation.NATIVE) != null) {
-            AnnotationsUtils.getNameForAnnotatedObject(descriptor, PredefinedAnnotation.NATIVE) != null
-        ) {
             diagnosticHolder.report(ErrorsJs.JS_NAME_PROHIBITED_FOR_NAMED_NATIVE.on(jsNamePsi))
         }
 

js/js.frontend/src/org/jetbrains/kotlin/js/translate/utils/AnnotationsUtils.java

@@ -16,25 +16,20 @@
 
 package org.jetbrains.kotlin.js.translate.utils;
 
-import com.intellij.util.Function;
-import com.intellij.util.containers.ContainerUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
-import org.jetbrains.kotlin.descriptors.*;
+import org.jetbrains.kotlin.descriptors.ClassDescriptor;
+import org.jetbrains.kotlin.descriptors.DeclarationDescriptor;
+import org.jetbrains.kotlin.descriptors.PropertyAccessorDescriptor;
+import org.jetbrains.kotlin.descriptors.PropertyDescriptor;
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor;
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationWithTarget;
 import org.jetbrains.kotlin.descriptors.annotations.Annotations;
 import org.jetbrains.kotlin.js.PredefinedAnnotation;
 import org.jetbrains.kotlin.name.FqName;
-import org.jetbrains.kotlin.psi.KtAnnotated;
-import org.jetbrains.kotlin.psi.KtAnnotationEntry;
-import org.jetbrains.kotlin.resolve.BindingContext;
 import org.jetbrains.kotlin.resolve.DescriptorUtils;
 import org.jetbrains.kotlin.resolve.constants.ConstantValue;
 
-import java.util.List;
-import java.util.Set;
-
 public final class AnnotationsUtils {
     private static final String JS_NAME = "kotlin.js.JsName";
 
@@ -128,7 +123,7 @@ public final class AnnotationsUtils {
     }
 
     @Nullable
-    private static AnnotationDescriptor getJsNameAnnotation(@NotNull DeclarationDescriptor descriptor) {
+    public static AnnotationDescriptor getJsNameAnnotation(@NotNull DeclarationDescriptor descriptor) {
         AnnotationDescriptor annotation = getAnnotationByName(descriptor, new FqName(JS_NAME));
         if (annotation == null) return null;
 
@@ -141,22 +136,6 @@ public final class AnnotationsUtils {
         return annotation;
     }
 
-    @Nullable
-    public static KtAnnotationEntry getJsNameAnnotationPsi(@NotNull BindingContext context, @NotNull KtAnnotated annotated,
-            @NotNull DeclarationDescriptor descriptor) {
-        AnnotationDescriptor annotation = getJsNameAnnotation(descriptor);
-        if (annotation == null) return null;
-
-        for (KtAnnotationEntry entry : annotated.getAnnotationEntries()) {
-            AnnotationDescriptor annotationDescriptor = context.get(BindingContext.ANNOTATION, entry);
-            if (annotationDescriptor == annotation) {
-                return entry;
-            }
-        }
-
-        return null;
-    }
-
     public static boolean isPredefinedObject(@NotNull DeclarationDescriptor descriptor) {
         for (PredefinedAnnotation annotation : PredefinedAnnotation.values()) {
             if (hasAnnotationOrInsideAnnotatedClass(descriptor, annotation)) {

js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java

@@ -4801,6 +4801,18 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest {
             doTest(fileName);
         }
 
+        @TestMetadata("jsName.kt")
+        public void testJsName() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/jsName/jsName.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("jsNamePropertyAccessors.kt")
+        public void testJsNamePropertyAccessors() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/jsName/jsNamePropertyAccessors.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("overriddenMethod.kt")
         public void testOverriddenMethod() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/jsName/overriddenMethod.kt");
@@ -5293,18 +5305,6 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest {
             doTest(fileName);
         }
 
-        @TestMetadata("jsName.kt")
-        public void testJsName() throws Exception {
-            String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/native/jsName.kt");
-            doTest(fileName);
-        }
-
-        @TestMetadata("jsNamePropertyAccessors.kt")
-        public void testJsNamePropertyAccessors() throws Exception {
-            String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/native/jsNamePropertyAccessors.kt");
-            doTest(fileName);
-        }
-
         @TestMetadata("kt1519.kt")
         public void testKt1519() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/native/kt1519.kt");