diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/resolveTemporaryNames.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/resolveTemporaryNames.kt index 1e1fe51f85a..ca146a28a98 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/resolveTemporaryNames.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/resolveTemporaryNames.kt @@ -14,37 +14,13 @@ * limitations under the License. */ -/* - * 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.inline.clean import org.jetbrains.kotlin.js.backend.ast.* import org.jetbrains.kotlin.js.backend.ast.metadata.coroutineMetadata -import org.jetbrains.kotlin.js.inline.util.collectReferencedTemporaryNames fun JsNode.resolveTemporaryNames() { - val allNames = collectReferencedTemporaryNames(this) - - val scopeTree = ScopeCollector().let { - it.accept(this) - it.scopeTree - } - - val renamings = scopeTree.resolveNames(allNames) + val renamings = resolveNames() accept(object : RecursiveJsVisitor() { override fun visitElement(node: JsNode) { super.visitElement(node) @@ -66,52 +42,126 @@ fun JsNode.resolveTemporaryNames() { }) } -private fun Map>.resolveNames(knownNames: Set): Map { +private fun JsNode.resolveNames(): Map { + val rootScope = computeScopes().liftUsedNames() val replacements = mutableMapOf() - fun traverse(scope: JsScope) { - for (temporaryName in scope.temporaryNames.filter { it in knownNames }.sortedBy { it.ordinal }) { - replacements[temporaryName] = scope.declareFreshName(temporaryName.ident).apply { copyMetadataFrom(temporaryName) } + fun traverse(scope: Scope) { + // Don't clash with non-temporary names declared in current scope. It's for rare cases like `_` or `Kotlin` names, + // since most of local declarations are temporary. + val occupiedNames = scope.declaredNames.asSequence().filter { !it.isTemporary }.map { it.ident }.toMutableSet() + + // Don't clash with non-temporary names used in current scope. It's ok to clash with unused names. + // Don't clash with used temporary names from outer scopes that get their resolved names. For example, + // when function declares temporary name `foo` and inner function both declares temporary name `foo` and uses + // (directly or propagates to inner scope) outer `foo`, we should resolve distinct strings for these `foo` names. + // Outer `foo` resolves first, so when traversing inner scope, we should take it into account. + occupiedNames += scope.usedNames.asSequence().mapNotNull { if (!it.isTemporary) it.ident else replacements[it]?.ident } + + for (temporaryName in scope.declaredNames.asSequence().filter { it.isTemporary }) { + var resolvedName = temporaryName.ident + var suffix = 0 + while (resolvedName in JsDeclarationScope.RESERVED_WORDS || !occupiedNames.add(resolvedName)) { + resolvedName = "${temporaryName.ident}_${suffix++}" + } + replacements[temporaryName] = JsDynamicScope.declareName(resolvedName).apply { copyMetadataFrom(temporaryName) } + occupiedNames += resolvedName } - this[scope]!!.forEach(::traverse) + scope.children.forEach(::traverse) } - val roots = keys.toMutableSet() - values.forEach { roots -= it } + traverse(rootScope) - for (root in roots) { - traverse(root) - } + // Labels in JS are in separate scope from local variables. It's only important that nested labels don't clash. + // Adjacent labels in one function is OK + accept(object : RecursiveJsVisitor() { + var labels = mutableSetOf() + + override fun visitLabel(x: JsLabel) { + val addedNames = mutableSetOf() + if (x.name.isTemporary) { + var resolvedName = x.name.ident + var suffix = 0 + while (!labels.add(resolvedName)) { + resolvedName = "${x.name.ident}_${suffix++}" + } + replacements[x.name] = JsDynamicScope.declareName(resolvedName) + addedNames += resolvedName + } + super.visitLabel(x) + labels.removeAll(addedNames) + } + + override fun visitFunction(x: JsFunction) { + val oldLabels = labels + labels = mutableSetOf() + super.visitFunction(x) + labels = oldLabels + } + }) return replacements } -private class ScopeCollector : RecursiveJsVisitor() { - val scopeTree = mutableMapOf>() - - override fun visitCatch(x: JsCatch) { - recordScope(x.scope) - super.visitCatch(x) - } - - override fun visitFunction(x: JsFunction) { - recordScope(x.scope) - super.visitFunction(x) - } - override fun visitElement(node: JsNode) { - if (node is HasName) { - node.name?.let { recordScope(it.enclosing) } +// When name is used by inner scope, it's implicitly used by outer scopes +private fun Scope.liftUsedNames(): Scope { + fun traverse(scope: Scope) { + scope.children.forEach { child -> + scope.usedNames += scope.declaredNames + traverse(child) + scope.usedNames += child.usedNames.filter { !it.isTemporary } } - super.visitElement(node) } + traverse(this) + return this +} - private fun recordScope(scope: JsScope) { - if (scope !in scopeTree) { - scopeTree[scope] = mutableSetOf() - val parent = scope.parent - if (parent != null) { - recordScope(parent) - scopeTree[parent]!!.add(scope) +private fun JsNode.computeScopes(): Scope { + val rootScope = Scope() + accept(object : RecursiveJsVisitor() { + var currentScope: Scope = rootScope + + override fun visitFunction(x: JsFunction) { + x.name?.let { currentScope.declaredNames += it } + val oldScope = currentScope + currentScope = Scope().apply { + parent = currentScope + currentScope.children += this } + currentScope.declaredNames += x.parameters.map { it.name } + super.visitFunction(x) + currentScope = oldScope } - } + + override fun visitCatch(x: JsCatch) { + currentScope.declaredNames += x.parameter.name + super.visitCatch(x) + } + + override fun visit(x: JsVars.JsVar) { + currentScope.declaredNames += x.name + super.visit(x) + } + + override fun visitNameRef(nameRef: JsNameRef) { + if (nameRef.qualifier == null) { + val name = nameRef.name + currentScope.usedNames += name ?: JsDynamicScope.declareName(nameRef.ident) + } + + super.visitNameRef(nameRef) + } + + override fun visitBreak(x: JsBreak) {} + + override fun visitContinue(x: JsContinue) {} + }) + + return rootScope +} + +private class Scope { + var parent: Scope? = null + val declaredNames = mutableSetOf() + val usedNames = mutableSetOf() + val children = mutableSetOf() } \ No newline at end of file diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt index 3fd2390937b..8681be567c7 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt @@ -25,21 +25,6 @@ import org.jetbrains.kotlin.js.translate.utils.JsAstUtils fun collectFunctionReferencesInside(scope: JsNode): List = collectReferencedNames(scope).filter { it.staticRef is JsFunction } -fun collectReferencedTemporaryNames(scope: JsNode): Set { - val references = mutableSetOf() - - object : RecursiveJsVisitor() { - override fun visitElement(node: JsNode) { - if (node is HasName) { - node.name?.let { references += it } - } - super.visitElement(node) - } - }.accept(scope) - - return references -} - private fun collectReferencedNames(scope: JsNode): Set { val references = mutableSetOf() diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ast/NameResolutionTest.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/ast/NameResolutionTest.kt new file mode 100644 index 00000000000..bfbc868bb7f --- /dev/null +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ast/NameResolutionTest.kt @@ -0,0 +1,92 @@ +/* + * Copyright 2010-2017 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.ast + +import com.google.gwt.dev.js.rhino.CodePosition +import com.google.gwt.dev.js.rhino.ErrorReporter +import com.intellij.openapi.util.io.FileUtil +import org.jetbrains.kotlin.js.backend.ast.* +import org.jetbrains.kotlin.js.inline.clean.resolveTemporaryNames +import org.jetbrains.kotlin.js.parser.parse +import org.jetbrains.kotlin.js.test.BasicTest +import org.junit.Assert.assertEquals +import org.junit.Assert.fail +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TestName +import java.io.File + +class NameResolutionTest { + @Rule + @JvmField + var testName = TestName() + + @Test + fun simple() = doTest() + + @Test + fun reuseName() = doTest() + + @Test + fun globalName() = doTest() + + @Test + fun labels() = doTest() + + private fun doTest() { + val methodName = testName.methodName + val baseName = "${BasicTest.TEST_DATA_DIR_PATH}/js-name-resolution/" + val originalName = "$baseName/$methodName.original.js" + val expectedName = "$baseName/$methodName.expected.js" + + val originalCode = FileUtil.loadFile(File(originalName)) + val expectedCode = FileUtil.loadFile(File(expectedName)) + + val parserScope = JsFunctionScope(JsRootScope(JsProgram()), "") + val originalAst = JsGlobalBlock().apply { statements += parse(originalCode, errorReporter, parserScope) } + val expectedAst = JsGlobalBlock().apply { statements += parse(expectedCode, errorReporter, parserScope) } + + originalAst.accept(object : RecursiveJsVisitor() { + val cache = mutableMapOf() + + override fun visitElement(node: JsNode) { + super.visitElement(node) + if (node is HasName) { + node.name = node.name?.let { name -> + if (name.ident.startsWith("$")) { + cache.getOrPut(name) { JsDynamicScope.declareTemporaryName("x") } + } + else { + name + } + } + } + } + }) + originalAst.resolveTemporaryNames() + + assertEquals(expectedAst.toString(), originalAst.toString()) + } + + private val errorReporter = object : ErrorReporter { + override fun warning(message: String, startPosition: CodePosition, endPosition: CodePosition) { } + + override fun error(message: String, startPosition: CodePosition, endPosition: CodePosition) { + fail("Error parsing JS file: $message at $startPosition") + } + } +} diff --git a/js/js.translator/testData/js-name-resolution/globalName.expected.js b/js/js.translator/testData/js-name-resolution/globalName.expected.js new file mode 100644 index 00000000000..2055d704702 --- /dev/null +++ b/js/js.translator/testData/js-name-resolution/globalName.expected.js @@ -0,0 +1,10 @@ +var x_0 = 23; +var x_1 = 42; + +function foo() { + println(x_0); +} + +function bar() { + println(x + x_1); +} \ No newline at end of file diff --git a/js/js.translator/testData/js-name-resolution/globalName.original.js b/js/js.translator/testData/js-name-resolution/globalName.original.js new file mode 100644 index 00000000000..3041421022f --- /dev/null +++ b/js/js.translator/testData/js-name-resolution/globalName.original.js @@ -0,0 +1,10 @@ +var $a = 23; +var $b = 42; + +function foo() { + println($a); +} + +function bar() { + println(x + $b); +} \ No newline at end of file diff --git a/js/js.translator/testData/js-name-resolution/labels.expected.js b/js/js.translator/testData/js-name-resolution/labels.expected.js new file mode 100644 index 00000000000..e5d47a4e9cc --- /dev/null +++ b/js/js.translator/testData/js-name-resolution/labels.expected.js @@ -0,0 +1,28 @@ +function foo() { + x: { + x_0: { + console.log("1"); + console.log(function() { + x: { + console.log("2"); + } + }); + + if (condition()) { + break x; + } + else { + break x_0; + } + } + } + + x: { + x_0: { + console.log("1"); + } + x_0: { + console.log("1"); + } + } +} \ No newline at end of file diff --git a/js/js.translator/testData/js-name-resolution/labels.original.js b/js/js.translator/testData/js-name-resolution/labels.original.js new file mode 100644 index 00000000000..1755cdd213f --- /dev/null +++ b/js/js.translator/testData/js-name-resolution/labels.original.js @@ -0,0 +1,28 @@ +function foo() { + $a: { + $b: { + console.log("1"); + console.log(function() { + $aa: { + console.log("2"); + } + }); + + if (condition()) { + break $a; + } + else { + break $b; + } + } + } + + $c: { + $d: { + console.log("1"); + } + $e: { + console.log("1"); + } + } +} \ No newline at end of file diff --git a/js/js.translator/testData/js-name-resolution/reuseName.expected.js b/js/js.translator/testData/js-name-resolution/reuseName.expected.js new file mode 100644 index 00000000000..877caec9638 --- /dev/null +++ b/js/js.translator/testData/js-name-resolution/reuseName.expected.js @@ -0,0 +1,12 @@ +var x = 23; +var x_0 = 42; + +function foo() { + var x_0 = 99; + console.log(x + x_0); +} + +function bar() { + var x = 101; + console.log(x_0 + x); +} \ No newline at end of file diff --git a/js/js.translator/testData/js-name-resolution/reuseName.original.js b/js/js.translator/testData/js-name-resolution/reuseName.original.js new file mode 100644 index 00000000000..b77fc1c6cc7 --- /dev/null +++ b/js/js.translator/testData/js-name-resolution/reuseName.original.js @@ -0,0 +1,12 @@ +var $a = 23; +var $b = 42; + +function foo() { + var $c = 99; + console.log($a + $c); +} + +function bar() { + var $d = 101; + console.log($b + $d); +} \ No newline at end of file diff --git a/js/js.translator/testData/js-name-resolution/simple.expected.js b/js/js.translator/testData/js-name-resolution/simple.expected.js new file mode 100644 index 00000000000..93c2ebe17a9 --- /dev/null +++ b/js/js.translator/testData/js-name-resolution/simple.expected.js @@ -0,0 +1,7 @@ +var x = 23; + +function x_0() { + var x_0 = 42; + var x_1 = 99; + return x + x_0 + x_1; +} \ No newline at end of file diff --git a/js/js.translator/testData/js-name-resolution/simple.original.js b/js/js.translator/testData/js-name-resolution/simple.original.js new file mode 100644 index 00000000000..f7c19d47205 --- /dev/null +++ b/js/js.translator/testData/js-name-resolution/simple.original.js @@ -0,0 +1,7 @@ +var $a = 23; + +function $f() { + var $b = 42; + var $c = 99; + return $a + $b + $c; +} \ No newline at end of file