From a2b40acc98d739b15ccabdd7073664404437cf64 Mon Sep 17 00:00:00 2001 From: Artem Kobzar Date: Wed, 4 May 2022 11:22:57 +0000 Subject: [PATCH] feat: add WARNING on usage top-level exportable declarations with non-consumable identifiers. --- .../export/nonConsumableIdentifiers.kt | 42 ++++++++++++++ .../export/nonConsumableIdentifiers.txt | 57 +++++++++++++++++++ .../DiagnosticsTestWithJsStdLibGenerated.java | 6 ++ .../kotlin/js/backend/ast/jsScopes.kt | 19 +------ .../kotlin/js/common/IdentifierPolicy.kt | 20 ++++++- .../diagnostics/DefaultErrorMessagesJs.kt | 2 + .../js/resolve/diagnostics/ErrorsJs.java | 2 + .../diagnostics/JsExportDeclarationChecker.kt | 40 ++++++++++++- .../js/translate/utils/AnnotationsUtils.java | 4 +- 9 files changed, 170 insertions(+), 22 deletions(-) create mode 100644 compiler/testData/diagnostics/testsWithJsStdLib/export/nonConsumableIdentifiers.kt create mode 100644 compiler/testData/diagnostics/testsWithJsStdLib/export/nonConsumableIdentifiers.txt diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/export/nonConsumableIdentifiers.kt b/compiler/testData/diagnostics/testsWithJsStdLib/export/nonConsumableIdentifiers.kt new file mode 100644 index 00000000000..cf8f37399d5 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithJsStdLib/export/nonConsumableIdentifiers.kt @@ -0,0 +1,42 @@ +// !OPT_IN: kotlin.js.ExperimentalJsExport +// !RENDER_DIAGNOSTICS_MESSAGES + +package foo + +@JsExport +fun delete() {} + +@JsExport +val instanceof = 4 + +@JsExport +class eval + +@JsExport +@JsName("await") +fun foo() {} + +@JsExport +@JsName("this") +val bar = 4 + +@JsExport +@JsName("super") +class Baz + +@JsExport +class Test { + fun instanceof() {} + + @JsName("eval") + fun test() {} +} + +@JsExport +object NaN + +@JsExport +enum class Nums { + Infinity, + undefined +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/testsWithJsStdLib/export/nonConsumableIdentifiers.txt b/compiler/testData/diagnostics/testsWithJsStdLib/export/nonConsumableIdentifiers.txt new file mode 100644 index 00000000000..f5322bdd401 --- /dev/null +++ b/compiler/testData/diagnostics/testsWithJsStdLib/export/nonConsumableIdentifiers.txt @@ -0,0 +1,57 @@ +package + +package foo { + @kotlin.js.JsExport @kotlin.js.JsName(name = "this") public val bar: kotlin.Int = 4 + @kotlin.js.JsExport public val instanceof: kotlin.Int = 4 + @kotlin.js.JsExport public fun delete(): kotlin.Unit + @kotlin.js.JsExport @kotlin.js.JsName(name = "await") public fun foo(): kotlin.Unit + + @kotlin.js.JsExport @kotlin.js.JsName(name = "super") public final class Baz { + public constructor Baz() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + + @kotlin.js.JsExport public object NaN { + private constructor NaN() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + + @kotlin.js.JsExport public final enum class Nums : kotlin.Enum { + enum entry Infinity + + enum entry undefined + + private constructor Nums() + public final override /*1*/ /*fake_override*/ val name: kotlin.String + public final override /*1*/ /*fake_override*/ val ordinal: kotlin.Int + protected final override /*1*/ /*fake_override*/ fun clone(): kotlin.Any + public final override /*1*/ /*fake_override*/ fun compareTo(/*0*/ other: foo.Nums): kotlin.Int + public final override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + + // Static members + public final /*synthesized*/ fun valueOf(/*0*/ value: kotlin.String): foo.Nums + public final /*synthesized*/ fun values(): kotlin.Array + } + + @kotlin.js.JsExport public final class Test { + public constructor Test() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final fun instanceof(): kotlin.Unit + @kotlin.js.JsName(name = "eval") public final fun test(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } + + @kotlin.js.JsExport public final class eval { + public constructor eval() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String + } +} \ No newline at end of file diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticsTestWithJsStdLibGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticsTestWithJsStdLibGenerated.java index 33b96653d64..688caa73bb0 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticsTestWithJsStdLibGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticsTestWithJsStdLibGenerated.java @@ -401,6 +401,12 @@ public class DiagnosticsTestWithJsStdLibGenerated extends AbstractDiagnosticsTes runTest("compiler/testData/diagnostics/testsWithJsStdLib/export/jsExportOnNestedDeclarations.kt"); } + @Test + @TestMetadata("nonConsumableIdentifiers.kt") + public void testNonConsumableIdentifiers() throws Exception { + runTest("compiler/testData/diagnostics/testsWithJsStdLib/export/nonConsumableIdentifiers.kt"); + } + @Test @TestMetadata("secondaryConstructorWithoutJsName.kt") public void testSecondaryConstructorWithoutJsName() throws Exception { diff --git a/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/jsScopes.kt b/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/jsScopes.kt index 6c0738631cb..30fcc37a777 100644 --- a/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/jsScopes.kt +++ b/js/js.ast/src/org/jetbrains/kotlin/js/backend/ast/jsScopes.kt @@ -16,6 +16,7 @@ package org.jetbrains.kotlin.js.backend.ast +import org.jetbrains.kotlin.js.common.RESERVED_KEYWORDS import java.util.* class JsObjectScope(parent: JsScope, description: String) : JsScope(parent, description) @@ -70,23 +71,7 @@ open class JsDeclarationScope(parent: JsScope, description: String, useParentSco } companion object { - val RESERVED_WORDS: Set = setOf( - // keywords - "await", "break", "case", "catch", "continue", "debugger", "default", "delete", "do", "else", "finally", "for", "function", "if", - "in", "instanceof", "new", "return", "switch", "this", "throw", "try", "typeof", "var", "void", "while", "with", - - // future reserved words - "class", "const", "enum", "export", "extends", "import", "super", - - // as future reserved words in strict mode - "implements", "interface", "let", "package", "private", "protected", "public", "static", "yield", - - // additional reserved words - "null", "true", "false", - - // disallowed as variable names in strict mode - "eval", "arguments", - + val RESERVED_WORDS: Set = RESERVED_KEYWORDS + setOf( // global identifiers usually declared in a typical JS interpreter "NaN", "isNaN", "Infinity", "undefined", "Error", "Object", "Math", "String", "Number", "Boolean", "Date", "Array", "RegExp", "JSON", diff --git a/js/js.ast/src/org/jetbrains/kotlin/js/common/IdentifierPolicy.kt b/js/js.ast/src/org/jetbrains/kotlin/js/common/IdentifierPolicy.kt index d4f78f968de..f16afd30b89 100644 --- a/js/js.ast/src/org/jetbrains/kotlin/js/common/IdentifierPolicy.kt +++ b/js/js.ast/src/org/jetbrains/kotlin/js/common/IdentifierPolicy.kt @@ -49,4 +49,22 @@ fun String.isValidES5Identifier(): Boolean { if (!get(idx).isES5IdentifierPart()) return false } return true -} \ No newline at end of file +} + +val RESERVED_KEYWORDS: Set = setOf( + // keywords + "await", "break", "case", "catch", "continue", "debugger", "default", "delete", "do", "else", "finally", "for", "function", "if", + "in", "instanceof", "new", "return", "switch", "this", "throw", "try", "typeof", "var", "void", "while", "with", + + // future reserved words + "class", "const", "enum", "export", "extends", "import", "super", + + // as future reserved words in strict mode + "implements", "interface", "let", "package", "private", "protected", "public", "static", "yield", + + // additional reserved words + "null", "true", "false", + + // disallowed as variable names in strict mode + "eval", "arguments", +) \ No newline at end of file diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt index b922ab2d8a0..e6fdefbbe2f 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt +++ b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt @@ -106,6 +106,8 @@ private val DIAGNOSTIC_FACTORY_TO_RENDERER by lazy { put(ErrorsJs.WRONG_EXPORTED_DECLARATION, "Declaration of such kind ({0}) can''t be exported to JS", STRING) put(ErrorsJs.NON_EXPORTABLE_TYPE, "Exported declaration uses non-exportable {0} type: {1}", STRING, RENDER_TYPE) + put(ErrorsJs.NON_CONSUMABLE_EXPORTED_IDENTIFIER, "Exported declaration contains non-consumable identifier '${0}', that can't be represented inside TS definitions and ESM", STRING) + this } } 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 900c72f43e8..4c2fa71f124 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 @@ -110,6 +110,8 @@ public interface ErrorsJs { DiagnosticFactory1 WRONG_EXPORTED_DECLARATION = DiagnosticFactory1.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT); DiagnosticFactory2 NON_EXPORTABLE_TYPE = DiagnosticFactory2.create(WARNING, DECLARATION_SIGNATURE_OR_DEFAULT); + DiagnosticFactory1 NON_CONSUMABLE_EXPORTED_IDENTIFIER = DiagnosticFactory1.create(WARNING, DEFAULT); + @SuppressWarnings("UnusedDeclaration") Object _initializer = new Object() { { diff --git a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsExportDeclarationChecker.kt b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsExportDeclarationChecker.kt index ea4a0f36ba2..ccef49ef065 100644 --- a/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsExportDeclarationChecker.kt +++ b/js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsExportDeclarationChecker.kt @@ -5,15 +5,19 @@ package org.jetbrains.kotlin.js.resolve.diagnostics +import com.intellij.psi.PsiElement import org.jetbrains.kotlin.builtins.KotlinBuiltIns -import org.jetbrains.kotlin.builtins.StandardNames import org.jetbrains.kotlin.builtins.isFunctionType import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.ClassKind.* -import org.jetbrains.kotlin.js.resolve.diagnostics.JsExportDeclarationChecker.isExportable +import org.jetbrains.kotlin.js.common.RESERVED_KEYWORDS +import org.jetbrains.kotlin.js.naming.NameSuggestion import org.jetbrains.kotlin.js.translate.utils.AnnotationsUtils +import org.jetbrains.kotlin.psi.KtAnnotationEntry import org.jetbrains.kotlin.psi.KtDeclaration +import org.jetbrains.kotlin.psi.KtNamedDeclaration import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.BindingTrace import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils import org.jetbrains.kotlin.resolve.checkers.DeclarationChecker import org.jetbrains.kotlin.resolve.checkers.DeclarationCheckerContext @@ -22,6 +26,7 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.isExtensionProperty import org.jetbrains.kotlin.resolve.descriptorUtil.isInsideInterface import org.jetbrains.kotlin.resolve.inline.isInlineWithReified import org.jetbrains.kotlin.resolve.isInlineClass +import org.jetbrains.kotlin.resolve.source.getPsi import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.isDynamic import org.jetbrains.kotlin.types.typeUtil.* @@ -61,6 +66,8 @@ object JsExportDeclarationChecker : DeclarationChecker { reportWrongExportedDeclaration("expect") } + validateDeclarationOnConsumableName(declaration, descriptor, trace) + when (descriptor) { is FunctionDescriptor -> { for (typeParameter in descriptor.typeParameters) { @@ -201,4 +208,33 @@ object JsExportDeclarationChecker : DeclarationChecker { return descriptor.isEffectivelyExternal() || AnnotationsUtils.isExportedObject(descriptor, bindingContext) } + + private fun validateDeclarationOnConsumableName( + declaration: KtDeclaration, + declarationDescriptor: DeclarationDescriptor, + trace: BindingTrace + ) { + if (!declarationDescriptor.isTopLevelInPackage() || declarationDescriptor.name.isSpecial) return + + val name = declarationDescriptor.getKotlinOrJsName() + + if (name !in RESERVED_KEYWORDS && NameSuggestion.sanitizeName(name) == name) return + + val reportTarget = declarationDescriptor.getJsNameArgument() ?: declaration.getIdentifier() + + trace.report(ErrorsJs.NON_CONSUMABLE_EXPORTED_IDENTIFIER.on(reportTarget, name)) + } + + private fun DeclarationDescriptor.getKotlinOrJsName(): String { + return AnnotationsUtils.getJsName(this) ?: name.identifier + } + + private fun KtDeclaration.getIdentifier(): PsiElement { + return (this as KtNamedDeclaration).nameIdentifier!! + } + + private fun DeclarationDescriptor.getJsNameArgument(): PsiElement? { + val jsNameAnnotation = AnnotationsUtils.getJsNameAnnotation(this) ?: return null + return (jsNameAnnotation.source.getPsi() as KtAnnotationEntry).valueArgumentList?.arguments?.first() + } } 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 06799818754..a51374df310 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 @@ -39,7 +39,7 @@ import java.util.List; import static org.jetbrains.kotlin.resolve.descriptorUtil.DescriptorUtilsKt.isEffectivelyExternal; public final class AnnotationsUtils { - private static final String JS_NAME = "kotlin.js.JsName"; + public static final FqName JS_NAME = new FqName("kotlin.js.JsName"); private static final FqName JS_EXPORT = new FqName("kotlin.js.JsExport"); public static final FqName JS_MODULE_ANNOTATION = new FqName("kotlin.js.JsModule"); private static final FqName JS_NON_MODULE_ANNOTATION = new FqName("kotlin.js.JsNonModule"); @@ -172,7 +172,7 @@ public final class AnnotationsUtils { @Nullable public static AnnotationDescriptor getJsNameAnnotation(@NotNull DeclarationDescriptor descriptor) { - return descriptor.getAnnotations().findAnnotation(new FqName(JS_NAME)); + return descriptor.getAnnotations().findAnnotation(JS_NAME); } @Nullable