From 0a240b2a3a91e89df64e87f01920544e0880e0ae Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Tue, 13 Sep 2016 12:44:31 +0300 Subject: [PATCH] KT-12976: add code to generated JS modules that detects wrong module order and produces human-friendly error message. Fix #KT-12976 --- .../inline/clean/removeDefaultInitializers.kt | 8 +-- .../jetbrains/kotlin/js/test/BasicBoxTest.kt | 2 +- .../js/test/semantics/MultiModuleOrderTest.kt | 57 +++++++++++++++++++ .../kotlin/js/translate/context/Namer.java | 10 ---- .../general/ModuleWrapperTranslation.kt | 32 +++++++++-- .../testData/multiModuleOrder/cases/plain.kt | 14 +++++ .../testData/multiModuleOrder/cases/umd.kt | 16 ++++++ 7 files changed, 120 insertions(+), 19 deletions(-) create mode 100644 js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/MultiModuleOrderTest.kt create mode 100644 js/js.translator/testData/multiModuleOrder/cases/plain.kt create mode 100644 js/js.translator/testData/multiModuleOrder/cases/umd.kt diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeDefaultInitializers.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeDefaultInitializers.kt index 7d7ce07a382..0a30651653c 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeDefaultInitializers.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeDefaultInitializers.kt @@ -22,7 +22,7 @@ import com.google.dart.compiler.backend.js.ast.metadata.hasDefaultValue import org.jetbrains.kotlin.js.inline.util.toIdentitySet import org.jetbrains.kotlin.js.inline.util.zipWithDefault import org.jetbrains.kotlin.js.translate.context.Namer -import org.jetbrains.kotlin.js.translate.context.Namer.isUndefined +import org.jetbrains.kotlin.js.translate.utils.JsAstUtils import org.jetbrains.kotlin.js.translate.utils.JsAstUtils.flattenStatement /** @@ -72,11 +72,11 @@ private fun getNameFromInitializer(isInitializedExpr: JsBinaryOperation): JsName val arg2 = isInitializedExpr.arg2 val op = isInitializedExpr.operator - if (arg1 == null || arg2 == null || op == null) { + if (arg1 == null || arg2 == null) { return null } - if (op == JsBinaryOperator.REF_EQ && isUndefined(arg2)) { + if (op == JsBinaryOperator.REF_EQ && JsAstUtils.isUndefinedExpression(arg2)) { return (arg1 as? JsNameRef)?.name } @@ -115,7 +115,7 @@ private fun getDefaultParamsNames( val argsParams = args.zipWithDefault(params, Namer.getUndefinedExpression()) val relevantParams = argsParams.asSequence() .filter { it.second.hasDefaultValue } - .filter { initialized == !isUndefined(it.first) } + .filter { initialized == !JsAstUtils.isUndefinedExpression(it.first) } val names = relevantParams.map { it.second.name } return names.toIdentitySet() diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt index f09fdbcf88e..37604010559 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt @@ -160,7 +160,7 @@ abstract class BasicBoxTest( return sb.toString() } - private fun getOutputDir(file: File): File { + protected fun getOutputDir(file: File): File { val stopFile = File(pathToTestDir) return generateSequence(file.parentFile) { it.parentFile } .takeWhile { it != stopFile } diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/MultiModuleOrderTest.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/MultiModuleOrderTest.kt new file mode 100644 index 00000000000..cecbf8887b2 --- /dev/null +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/MultiModuleOrderTest.kt @@ -0,0 +1,57 @@ +/* + * Copyright 2010-2016 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.js.test.semantics + +import org.jetbrains.kotlin.js.test.BasicBoxTest +import org.jetbrains.kotlin.js.test.rhino.RhinoResultChecker +import org.jetbrains.kotlin.js.test.rhino.RhinoUtils +import org.mozilla.javascript.JavaScriptException +import java.io.File + +class MultiModuleOrderTest : BasicBoxTest("$TEST_DATA_DIR_PATH/multiModuleOrder/cases/", "$TEST_DATA_DIR_PATH/multiModuleOrder/out/") { + fun testPlain() { + runTest("plain") + } + + fun testUmd() { + runTest("umd") + } + + fun runTest(name: String) { + val fullPath = "$TEST_DATA_DIR_PATH/multiModuleOrder/cases/$name.kt" + doTest(fullPath) + checkWrongOrderReported(fullPath, name) + } + + private fun checkWrongOrderReported(path: String, name: String) { + val parentDir = getOutputDir(File(path)) + val mainJsFile = File(parentDir, "$name-main_v5.js").path + val libJsFile = File(parentDir, "$name-lib_v5.js").path + try { + RhinoUtils.runRhinoTest(listOf(mainJsFile, libJsFile), RhinoResultChecker { context, scope -> + // don't check anything, expect exception from function + }) + } + catch (e: JavaScriptException) { + val message = e.message!! + assertTrue("Exception message should contain reference to dependency (lib)", "'lib'" in message) + assertTrue("Exception message should contain reference to module that failed to load (main)", "'main'" in message) + return + } + fail("Exception should have been thrown due to wrong order of modules") + } +} \ No newline at end of file diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java index fd00b89a9fe..9752bdde1d3 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java @@ -112,16 +112,6 @@ public final class Namer { public static final String ENUM_NAME_FIELD = "name$"; public static final String ENUM_ORDINAL_FIELD = "ordinal$"; - public static boolean isUndefined(@NotNull JsExpression expr) { - if (expr instanceof JsPrefixOperation) { - JsUnaryOperator op = ((JsPrefixOperation) expr).getOperator(); - - return op == JsUnaryOperator.VOID; - } - - return false; - } - @NotNull public static String getFunctionTag(@NotNull CallableDescriptor functionDescriptor) { String moduleName = getModuleName(functionDescriptor); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/ModuleWrapperTranslation.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/ModuleWrapperTranslation.kt index 8021320192e..88d6702387b 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/ModuleWrapperTranslation.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/general/ModuleWrapperTranslation.kt @@ -63,9 +63,14 @@ object ModuleWrapperTranslation { else { JsNameRef(scope.declareName(moduleId), rootName.makeRef()) } - val plainExpr = JsAstUtils.assignment(lhs, plainInvocation) - val selector = JsAstUtils.newJsIf(amdTest, amdBody, JsAstUtils.newJsIf(commonJsTest, commonJsBody, plainExpr.makeStmt())) + val plainBlock = JsBlock() + for (importedModule in importedModules) { + plainBlock.statements += addModuleValidation(moduleId, program, importedModule) + } + plainBlock.statements += JsAstUtils.assignment(lhs, plainInvocation).makeStmt() + + val selector = JsAstUtils.newJsIf(amdTest, amdBody, JsAstUtils.newJsIf(commonJsTest, commonJsBody, plainBlock)) adapterBody.statements += selector return listOf(JsInvocation(adapter, JsLiteral.THIS, function).makeStmt()) @@ -102,15 +107,34 @@ object ModuleWrapperTranslation { importedModules: List, program: JsProgram ): List { val invocation = makePlainInvocation(moduleId, function, importedModules, program) + val statements = mutableListOf() - val statement = if (Namer.requiresEscaping(moduleId)) { + for (importedModule in importedModules) { + statements += addModuleValidation(moduleId, program, importedModule) + } + + statements += if (Namer.requiresEscaping(moduleId)) { JsAstUtils.assignment(makePlainModuleRef(moduleId, program), invocation).makeStmt() } else { JsAstUtils.newVar(program.rootScope.declareName(moduleId), invocation) } - return listOf(statement) + return statements + } + + private fun addModuleValidation( + currentModuleId: String, + program: JsProgram, + moduleName: String + ): JsStatement { + val moduleRef = makePlainModuleRef(moduleName, program) + val moduleExistsCond = JsAstUtils.typeOfIs(moduleRef, program.getStringLiteral("undefined")) + val moduleNotFoundMessage = program.getStringLiteral( + "Error loading module '" + currentModuleId + "'. Its dependency '" + moduleName + "' was not found. " + + "Please, check whether '" + moduleName + "' is loaded prior to '" + currentModuleId + "'.") + val moduleNotFoundThrow = JsThrow(JsNew(JsNameRef("Error"), listOf(moduleNotFoundMessage))) + return JsIf(moduleExistsCond, JsBlock(moduleNotFoundThrow)) } private fun makePlainInvocation( diff --git a/js/js.translator/testData/multiModuleOrder/cases/plain.kt b/js/js.translator/testData/multiModuleOrder/cases/plain.kt new file mode 100644 index 00000000000..5d88270e501 --- /dev/null +++ b/js/js.translator/testData/multiModuleOrder/cases/plain.kt @@ -0,0 +1,14 @@ +// MODULE: lib +// FILE: lib.kt +package lib + +fun bar() = "OK" + + +// MODULE: main(lib) +// FILE: main.kt +package foo + +import lib.bar + +fun box() = bar() \ No newline at end of file diff --git a/js/js.translator/testData/multiModuleOrder/cases/umd.kt b/js/js.translator/testData/multiModuleOrder/cases/umd.kt new file mode 100644 index 00000000000..122f82c1067 --- /dev/null +++ b/js/js.translator/testData/multiModuleOrder/cases/umd.kt @@ -0,0 +1,16 @@ +// MODULE: lib +// FILE: lib.kt +// MODULE_KIND: UMD +package lib + +fun bar() = "OK" + + +// MODULE: main(lib) +// FILE: main.kt +// MODULE_KIND: UMD +package foo + +import lib.bar + +fun box() = bar() \ No newline at end of file