Generate unique names less aggressively.

editor/util/plugins/com.google.dart.java2dart/src/com/google/dart/java2dart/Context.java

 /*
  * Copyright (c) 2012, the Dart project authors.
  * 
  * Licensed under the Eclipse Public License v1.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.eclipse.org/legal/epl-v10.html
  * 
  * 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
@@ skipping 191 matching lines @@
               @Override
               public Void visitSimpleIdentifier(SimpleIdentifier node) {
                 if (node.getName().equals(parameterName)
                     && getNodeBinding(node) != parameterBinding) {
                   hasHiding.set(true);
                 }
                 return super.visitSimpleIdentifier(node);
               }
             });
             if (hasHiding.get()) {
-              String newName = generateUniqueName(parameterName);
+              Set<String> used = getSuperMembersNames(node);
+              String newName = generateUniqueParameterName(used, parameterName);
               renameIdentifier(parameter.getIdentifier(), newName);
             }
           }
         }
         return super.visitMethodDeclaration(node);
       }
+
+      private String generateUniqueParameterName(Set<String> used, String name) {

[Brian Wilkerson, 2013/04/18 00:00:32]

There's probably a good reason for doing it this way that I'm just not thinking
of, but couldn't we start with 'name' and only add an index if there's a
collision? Seems like there won't be collisions very often except with field
names, but then the field names would already have to be prefixed by "this." in
the Java, so as long as we preserve that we should be fine.

[scheglov, 2013/04/18 00:09:51]

In general this is good idea, but in  this case we call this method when we
already identified that parameter name conflicts with its method name.

  void set period(Token period2) {
    this._period = period2;
  }

+        int index = 2;
+        while (true) {
+          String newName = name + index;
+          if (!used.contains(newName)) {
+            return newName;
+          }
+          index++;
+        }
+      }
     });
   }
 
   public void ensureNoVariableNameReferenceFromInitializer(CompilationUnit unit) {
     unit.accept(new RecursiveASTVisitor<Void>() {
+      private Set<String> hierarchyNames;
+      private Set<String> methodNames;
       private String currentVariableName = null;
       private boolean hasNameReference = false;
 
       @Override
+      public Void visitClassDeclaration(ClassDeclaration node) {
+        hierarchyNames = null;
+        try {
+          return super.visitClassDeclaration(node);
+        } finally {
+          hierarchyNames = null;
+        }
+      }
+
+      @Override
+      public Void visitMethodDeclaration(MethodDeclaration node) {
+        methodNames = null;
+        try {
+          return super.visitMethodDeclaration(node);
+        } finally {
+          methodNames = null;
+        }
+      }
+
+      @Override
       public Void visitSimpleIdentifier(SimpleIdentifier node) {
         hasNameReference |= node.getName().equals(currentVariableName);
         return super.visitSimpleIdentifier(node);
       }
 
       @Override
       public Void visitVariableDeclaration(VariableDeclaration node) {
         String oldVariableName = currentVariableName;
         try {
           currentVariableName = node.getName().getName();
           hasNameReference = false;
           Expression initializer = node.getInitializer();
           if (initializer != null) {
             initializer.accept(this);
           }
           if (hasNameReference || forbiddenNames.contains(currentVariableName)) {
-            String newName = generateUniqueName(currentVariableName);
+            ensureHierarchyNames(node);
+            ensureMethodNames(node);
+            String newName = generateUniqueVariableName(currentVariableName);
             renameIdentifier(node.getName(), newName);
           }
         } finally {
           currentVariableName = oldVariableName;
         }
         return null;
       }
+
+      private void ensureHierarchyNames(ASTNode node) {
+        if (hierarchyNames != null) {
+          return;
+        }
+        hierarchyNames = getSuperMembersNames(node);
+      }
+
+      private void ensureMethodNames(ASTNode node) {
+        methodNames = Sets.newHashSet();
+        MethodDeclaration method = node.getAncestor(MethodDeclaration.class);
+        if (method != null) {
+          method.accept(new RecursiveASTVisitor<Void>() {
+            @Override
+            public Void visitVariableDeclaration(VariableDeclaration node) {
+              methodNames.add(node.getName().getName());
+              return super.visitVariableDeclaration(node);
+            }
+          });
+        }
+      }
+
+      /**
+       * @return the new name for variable which does not conflict with name of any member in super
+       *         classes - {@link #hierarchyNames}.
+       */
+      private String generateUniqueVariableName(String name) {
+        int index = 2;
+        while (true) {
+          String newName = name + index;
+          if (!hierarchyNames.contains(newName) && !methodNames.contains(newName)
+              && !forbiddenNames.contains(newName)) {
+            methodNames.add(newName);
+            return newName;
+          }
+          index++;
+        }
+      }
     });
   }
 
   public void ensureUniqueClassMemberNames(CompilationUnit unit) {
     unit.accept(new RecursiveASTVisitor<Void>() {
       private final Set<ClassMember> untouchableMethods = Sets.newHashSet();
       private final Map<String, ClassMember> usedClassMembers = Maps.newHashMap();
 
       @Override
       public Void visitClassDeclaration(ClassDeclaration node) {
@@ skipping 452 matching lines @@
               variable.setInitializer(initializer);
             }
           }
         }
         return super.visitFieldDeclaration(node);
       }
     });
   }
 
   /**
-   * @return the globally unique name, based on the given one.
+   * @return the name of member declared in enclosing {@link ClassDeclaration} and its super
+   *         classes.
    */
-  private String generateUniqueName(String name) {
-    if (usedNames.contains(name) || forbiddenNames.contains(name)) {
-      int index = 2;
-      while (true) {
-        String newName = name + index;
-        if (!usedNames.contains(newName) && !forbiddenNames.contains(newName)) {
-          usedNames.add(newName);
-          return newName;
+  private Set<String> getSuperMembersNames(ASTNode node) {
+    Set<String> hierarchyNames = Sets.newHashSet();
+    ClassDeclaration classDeclaration = node.getAncestor(ClassDeclaration.class);
+    org.eclipse.jdt.core.dom.ITypeBinding binding = getNodeTypeBinding(classDeclaration);
+    if (binding != null) {
+      binding = binding.getSuperclass();
+      while (binding != null) {
+        for (org.eclipse.jdt.core.dom.IVariableBinding field : binding.getDeclaredFields()) {
+          hierarchyNames.add(field.getName());
         }
-        index++;
+        for (org.eclipse.jdt.core.dom.IMethodBinding method : binding.getDeclaredMethods()) {
+          hierarchyNames.add(method.getName());
+        }
+        binding = binding.getSuperclass();
       }
     }
-    return name;
+    return hierarchyNames;
   }
 
   /**
    * @return the Java AST of the given Java {@link File} in context of {@link #sourceFolders}.
    */
   private Map<File, CompilationUnit> parseJavaFiles(final List<File> javaFiles) throws Exception {
     String paths[] = new String[javaFiles.size()];
     final Map<String, File> pathToFile = Maps.newHashMap();
     for (int i = 0; i < javaFiles.size(); i++) {
       File javaFile = javaFiles.get(i);
@@ skipping 118 matching lines @@
         List<CompilationUnitMember> fileMembers = fileToMembers.get(javaFile);
         if (fileMembers == null) {
           fileMembers = Lists.newArrayList();
           fileToMembers.put(javaFile, fileMembers);
         }
         fileMembers.addAll(dartDeclarations);
       }
     }
   }
 
-  // XXX
   private void unwrapVarArgIfAlreadyArray(CompilationUnit unit) {
     unit.accept(new RecursiveASTVisitor<Void>() {
       @Override
       public Void visitMethodInvocation(MethodInvocation node) {
         Object binding = nodeToBinding.get(node);
         if (binding instanceof IMethodBinding) {
           IMethodBinding methodBinding = (IMethodBinding) binding;
           if (methodBinding.isVarargs()) {
             List<Expression> args = node.getArgumentList().getArguments();
             if (!args.isEmpty() && args.get(args.size() - 1) instanceof ListLiteral) {
               ListLiteral listLiteral = (ListLiteral) args.get(args.size() - 1);
               List<Expression> elements = listLiteral.getElements();
               if (elements.size() == 1) {
                 Expression element = elements.get(0);
                 if (nodeToTypeBinding.get(element) instanceof ITypeBinding) {
                   ITypeBinding elementTypeBinding = nodeToTypeBinding.get(element);
                   if (elementTypeBinding.isArray()) {
                     args.set(args.size() - 1, element);
                   }
                 }
               }
             }
           }
         }
         return super.visitMethodInvocation(node);
       }
     });
   }
 }