From aa5d15cff7f3ae887c117d5c9ce9b7bd6a3be736 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Tue, 23 Aug 2016 15:32:39 +0300 Subject: [PATCH] KT-2752: minor fixes after code review --- .../kotlin/backend/common/CodegenUtil.kt | 12 ------- .../codegen/ImplementationBodyCodegen.java | 2 +- .../kotlin/resolve/DescriptorUtils.kt | 6 ++-- .../js/descriptorUtils/descriptorUtils.kt | 11 ------- .../kotlin/js/naming/NameSuggestion.kt | 20 ++++++------ .../js/resolve/diagnostics/ErrorsJs.java | 4 +-- .../js/resolve/diagnostics/JsNameChecker.kt | 12 +++---- .../js/translate/utils/AnnotationsUtils.java | 31 +++---------------- .../js/test/semantics/BoxJsTestGenerated.java | 24 +++++++------- .../testData/box/{native => jsName}/jsName.js | 0 .../testData/box/{native => jsName}/jsName.kt | 0 .../jsNamePropertyAccessors.js | 0 .../jsNamePropertyAccessors.kt | 0 13 files changed, 36 insertions(+), 86 deletions(-) rename js/js.translator/testData/box/{native => jsName}/jsName.js (100%) rename js/js.translator/testData/box/{native => jsName}/jsName.kt (100%) rename js/js.translator/testData/box/{native => jsName}/jsNamePropertyAccessors.js (100%) rename js/js.translator/testData/box/{native => jsName}/jsNamePropertyAccessors.kt (100%) diff --git a/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt b/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt index 80d27fac127..6d649450bf9 100644 --- a/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt +++ b/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 diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java index 53105217b91..2be22f52a21 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java +++ b/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() { @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(), diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt b/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt index 618b4f385cd..bd66e92b00d 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt +++ b/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.TypeProjection -import org.jetbrains.kotlin.types.TypeSubstitutor -import org.jetbrains.kotlin.types.Variance +import org.jetbrains.kotlin.types.* +import org.jetbrains.kotlin.types.checker.KotlinTypeChecker import org.jetbrains.kotlin.types.typeUtil.isAnyOrNullableAny import org.jetbrains.kotlin.utils.DFS import org.jetbrains.kotlin.utils.SmartList diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/descriptorUtils/descriptorUtils.kt b/js/js.frontend/src/org/jetbrains/kotlin/js/descriptorUtils/descriptorUtils.kt index 2616e7a3298..0011f6e3c7c 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/descriptorUtils/descriptorUtils.kt +++ b/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) -} diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/naming/NameSuggestion.kt b/js/js.frontend/src/org/jetbrains/kotlin/js/naming/NameSuggestion.kt index bb5a84b8510..de136f8f1e6 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/naming/NameSuggestion.kt +++ b/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 { + private fun getMangledName(baseName: String, descriptor: DeclarationDescriptor): Pair { // 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 || - containingDeclaration.modality == Modality.OPEN || containingDeclaration.modality == Modality.ABSTRACT - ) { + if (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 "" } diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java index 22a3bfdabca..f49d3433872 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java +++ b/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 JS_NAME_CLASH_SYNTHETIC = DiagnosticFactory3.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT); - DiagnosticFactory0 JS_NAME_ON_PRIMARY_CONSTRUCTOR_PROHIBITED = DiagnosticFactory0.create(ERROR); - DiagnosticFactory0 JS_NAME_ON_ACCESSOR_AND_PROPERTY = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 JS_NAME_ON_PRIMARY_CONSTRUCTOR_PROHIBITED = DiagnosticFactory0.create(ERROR); + DiagnosticFactory0 JS_NAME_ON_ACCESSOR_AND_PROPERTY = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 JS_NAME_IS_NOT_ON_ALL_ACCESSORS = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT); DiagnosticFactory0 JS_NAME_PROHIBITED_FOR_OVERRIDE = DiagnosticFactory0.create(ERROR); DiagnosticFactory0 JS_NAME_PROHIBITED_FOR_EXTENSION_PROPERTY = DiagnosticFactory0.create(ERROR); diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsNameChecker.kt b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsNameChecker.kt index 0c4abc55b53..55247255df0 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsNameChecker.kt +++ b/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 jsNamePsi = AnnotationsUtils.getJsNameAnnotationPsi(bindingContext, declaration, descriptor) ?: return + val jsName = AnnotationsUtils.getJsNameAnnotation(descriptor) ?: return + val jsNamePsi = jsName.source.getPsi() ?: declaration - if (AnnotationsUtils.isNativeObject(descriptor) && - AnnotationsUtils.getNameForAnnotatedObject(descriptor, PredefinedAnnotation.NATIVE) != null - ) { + if (AnnotationsUtils.getNameForAnnotatedObject(descriptor, PredefinedAnnotation.NATIVE) != null) { diagnosticHolder.report(ErrorsJs.JS_NAME_PROHIBITED_FOR_NAMED_NATIVE.on(jsNamePsi)) } diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/translate/utils/AnnotationsUtils.java b/js/js.frontend/src/org/jetbrains/kotlin/js/translate/utils/AnnotationsUtils.java index cb7f3efbcd4..a899d972789 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/translate/utils/AnnotationsUtils.java +++ b/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)) { diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java index df2c6c06a99..da7397dc0ed 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java +++ b/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"); diff --git a/js/js.translator/testData/box/native/jsName.js b/js/js.translator/testData/box/jsName/jsName.js similarity index 100% rename from js/js.translator/testData/box/native/jsName.js rename to js/js.translator/testData/box/jsName/jsName.js diff --git a/js/js.translator/testData/box/native/jsName.kt b/js/js.translator/testData/box/jsName/jsName.kt similarity index 100% rename from js/js.translator/testData/box/native/jsName.kt rename to js/js.translator/testData/box/jsName/jsName.kt diff --git a/js/js.translator/testData/box/native/jsNamePropertyAccessors.js b/js/js.translator/testData/box/jsName/jsNamePropertyAccessors.js similarity index 100% rename from js/js.translator/testData/box/native/jsNamePropertyAccessors.js rename to js/js.translator/testData/box/jsName/jsNamePropertyAccessors.js diff --git a/js/js.translator/testData/box/native/jsNamePropertyAccessors.kt b/js/js.translator/testData/box/jsName/jsNamePropertyAccessors.kt similarity index 100% rename from js/js.translator/testData/box/native/jsNamePropertyAccessors.kt rename to js/js.translator/testData/box/jsName/jsNamePropertyAccessors.kt