JS backend: increment counter for fresh names

js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/JsScope.java

@@ -11,6 +11,8 @@ import org.jetbrains.annotations.Nullable;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import static com.google.dart.compiler.backend.js.ast.AstPackage.JsObjectScope;
 
@@ -48,6 +50,8 @@ public abstract class JsScope {
     protected int tempIndex = 0;
     private final String scopeId;
 
+    private static final Pattern FRESH_NAME_SUFFIX = Pattern.compile("[\\$_]\\d+$");
+
     public JsScope(JsScope parent, @NotNull String description, @Nullable String scopeId) {
         assert (parent != null);
         this.scopeId = scopeId;
@@ -89,12 +93,10 @@ public abstract class JsScope {
      */
     @NotNull
     public JsName declareFreshName(@NotNull String suggestedName) {
-        String name = suggestedName;
-        int counter = 0;
-        while (hasOwnName(name)) {
-            name = suggestedName + '_' + counter++;
-        }
-        return doCreateName(name);
+        assert !suggestedName.isEmpty();
+        String ident = getFreshIdent(suggestedName);
+        assert !hasOwnName(ident);
+        return doCreateName(ident);
     }
 
     private String getNextTempName() {
@@ -181,4 +183,37 @@ public abstract class JsScope {
     protected JsName findOwnName(@NotNull String ident) {
         return names.get(ident);
     }
+
+    /**
+     * During inlining names can be refreshed multiple times,
+     * so "a" becomes "a_0", then becomes "a_0_0"
+     * in case a_0 has been declared in calling scope.
+     *
+     * That's ugly. To resolve it, we rename
+     * clashing names with "[_$]\\d+" suffix,
+     * incrementing last number.
+     *
+     * Fresh name for "a0" should still be "a0_0".
+     */
+    @NotNull
+    private String getFreshIdent(@NotNull String suggestedIdent) {
+        char sep = '_';
+        String baseName = suggestedIdent;
+        int counter = 0;
+
+        Matcher matcher = FRESH_NAME_SUFFIX.matcher(suggestedIdent);
+        if (matcher.find()) {
+            String group = matcher.group();
+            baseName = matcher.replaceAll("");
+            sep = group.charAt(0);
+            counter = Integer.valueOf(group.substring(1));
+        }
+
+        String freshName = suggestedIdent;
+        while (hasOwnName(freshName)) {
+            freshName = baseName + sep + counter++;
+        }
+
+        return freshName;
+    }
 }

js/js.tests/test/org/jetbrains/k2js/test/ast/JsScopeTest.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2010-2014 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.k2js.test.ast;
+
+import com.google.dart.compiler.backend.js.ast.JsName;
+import com.google.dart.compiler.backend.js.ast.JsScope;
+import junit.framework.TestCase;
+import org.jetbrains.annotations.NotNull;
+
+public final class JsScopeTest extends TestCase {
+    private JsScope scope;
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+        scope = new JsScope("Test scope") {};
+    }
+
+    public void testDeclareFreshName() throws Exception {
+        declareFreshNameAndAssertEquals("a", "a");
+        declareFreshNameAndAssertEquals("a", "a_0");
+        declareFreshNameAndAssertEquals("a", "a_1");
+
+        declareFreshNameAndAssertEquals("a_1", "a_2");
+        declareFreshNameAndAssertEquals("a_3", "a_3");
+
+        declareFreshNameAndAssertEquals("a_1_1", "a_1_1");
+        declareFreshNameAndAssertEquals("a_1_1", "a_1_2");
+
+        declareFreshNameAndAssertEquals("tmp$0", "tmp$0");
+        declareFreshNameAndAssertEquals("tmp$0", "tmp$1");
+
+        declareFreshNameAndAssertEquals("a0", "a0");
+        declareFreshNameAndAssertEquals("a0", "a0_0");
+        declareFreshNameAndAssertEquals("a0_0", "a0_1");
+    }
+
+    private void declareFreshNameAndAssertEquals(@NotNull String suggested, @NotNull String expected) {
+        JsName actual = scope.declareFreshName(suggested);
+        assertEquals(expected, actual.getIdent());
+    }
+}