From e28aefb2a3c4018f2081625a7bb3ff40d62ae9eb Mon Sep 17 00:00:00 2001 From: Noopur Gupta Date: Wed, 14 May 2014 21:26:23 +0200 Subject: [PATCH] Bug 434693: [1.8][clean up] convert nested anonymous to lambdas results in code with conflicting variable names --- .../jdt/ui/tests/quickfix/CleanUpTest18.java | 111 ++++++++++++++++++ .../corext/fix/LambdaExpressionsFix.java | 34 ++++-- 2 files changed, 136 insertions(+), 9 deletions(-) diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest18.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest18.java index 62e5faa27a..2bc33b944a 100644 --- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest18.java +++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest18.java @@ -551,6 +551,117 @@ public void testConvertToLambdaAmbiguous03() throws Exception { assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { original }); } + public void testConvertToLambdaConflictingNames() throws Exception { + IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null); + StringBuffer buf= new StringBuffer(); + buf.append("package test1;\n"); + buf.append("\n"); + buf.append("interface FI {\n"); + buf.append(" void run(int x);\n"); + buf.append("}\n"); + buf.append("\n"); + buf.append("public class Test {\n"); + buf.append(" {\n"); + buf.append(" int e;\n"); + buf.append(" FI fi = new FI() {\n"); + buf.append(" @Override\n"); + buf.append(" public void run(int e) {\n"); + buf.append(" class C1 {\n"); + buf.append(" void init1() {\n"); + buf.append(" m(new FI() {\n"); + buf.append(" @Override\n"); + buf.append(" public void run(int e) {\n"); + buf.append(" FI fi = new FI() {\n"); + buf.append(" @Override\n"); + buf.append(" public void run(int e) {\n"); + buf.append(" FI fi = new FI() {\n"); + buf.append(" @Override\n"); + buf.append(" public void run(int e) {\n"); + buf.append(" return;\n"); + buf.append(" }\n"); + buf.append(" };\n"); + buf.append(" }\n"); + buf.append(" };\n"); + buf.append(" }\n"); + buf.append(" });\n"); + buf.append(" }\n"); + buf.append("\n"); + buf.append(" void init2() {\n"); + buf.append(" m(new FI() {\n"); + buf.append(" @Override\n"); + buf.append(" public void run(int e) {\n"); + buf.append(" new FI() {\n"); + buf.append(" @Override\n"); + buf.append(" public void run(int e3) {\n"); + buf.append(" FI fi = new FI() {\n"); + buf.append(" @Override\n"); + buf.append(" public void run(int e) {\n"); + buf.append(" return;\n"); + buf.append(" }\n"); + buf.append(" };\n"); + buf.append(" }\n"); + buf.append(" };\n"); + buf.append(" }\n"); + buf.append(" });\n"); + buf.append(" }\n"); + buf.append(" }\n"); + buf.append(" }\n"); + buf.append(" };\n"); + buf.append(" }\n"); + buf.append("\n"); + buf.append(" void m(FI fi) {\n"); + buf.append(" };\n"); + buf.append("}\n"); + String original= buf.toString(); + ICompilationUnit cu1= pack1.createCompilationUnit("Test.java", original, false, null); + + enable(CleanUpConstants.CONVERT_FUNCTIONAL_INTERFACES); + enable(CleanUpConstants.USE_LAMBDA); + + buf= new StringBuffer(); + buf.append("package test1;\n"); + buf.append("\n"); + buf.append("interface FI {\n"); + buf.append(" void run(int x);\n"); + buf.append("}\n"); + buf.append("\n"); + buf.append("public class Test {\n"); + buf.append(" {\n"); + buf.append(" int e;\n"); + buf.append(" FI fi = e1 -> {\n"); + buf.append(" class C1 {\n"); + buf.append(" void init1() {\n"); + buf.append(" m(e2 -> {\n"); + buf.append(" FI fi1 = e3 -> {\n"); + buf.append(" FI fi2 = e4 -> {\n"); + buf.append(" return;\n"); + buf.append(" };\n"); + buf.append(" };\n"); + buf.append(" });\n"); + buf.append(" }\n"); + buf.append("\n"); + buf.append(" void init2() {\n"); + buf.append(" m(e2 -> new FI() {\n"); + buf.append(" @Override\n"); + buf.append(" public void run(int e3) {\n"); + buf.append(" FI fi = e4 -> {\n"); + buf.append(" return;\n"); + buf.append(" };\n"); + buf.append(" }\n"); + buf.append(" });\n"); + buf.append(" }\n"); + buf.append(" }\n"); + buf.append(" };\n"); + buf.append(" }\n"); + buf.append("\n"); + buf.append(" void m(FI fi) {\n"); + buf.append(" };\n"); + buf.append("}\n"); + String expected1= buf.toString(); + + assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1 }, new String[] { expected1 }); + } + public void testConvertToAnonymousWithWildcards() throws Exception { IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null); StringBuffer buf= new StringBuffer(); diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFix.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFix.java index 39ee07a650..dec2ec4df1 100644 --- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFix.java +++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/LambdaExpressionsFix.java @@ -12,6 +12,8 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -199,8 +201,9 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModel mod ImportRemover importRemover= cuRewrite.getImportRemover(); AST ast= rewrite.getAST(); - for (Iterator iterator= fExpressions.iterator(); iterator.hasNext();) { - ClassInstanceCreation classInstanceCreation= iterator.next(); + HashMap> cicToNewNames= new HashMap>(); + for (int i= 0; i < fExpressions.size(); i++) { + ClassInstanceCreation classInstanceCreation= fExpressions.get(i); TextEditGroup group= createTextEditGroup(FixMessages.LambdaExpressionsFix_convert_to_lambda_expression, cuRewrite); AnonymousClassDeclaration anonymTypeDecl= classInstanceCreation.getAnonymousClassDeclaration(); @@ -210,7 +213,16 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModel mod if (!(object instanceof MethodDeclaration)) continue; MethodDeclaration methodDeclaration= (MethodDeclaration) object; - makeNamesUnique(methodDeclaration, rewrite, group); + HashSet excludedNames= new HashSet(); + if (i != 0) { + for (ClassInstanceCreation convertedCic : fExpressions.subList(0, i)) { + if (ASTNodes.isParent(classInstanceCreation, convertedCic)) { + excludedNames.addAll(cicToNewNames.get(convertedCic)); + } + } + } + HashSet newNames= makeNamesUnique(excludedNames, methodDeclaration, rewrite, group); + cicToNewNames.put(classInstanceCreation, new HashSet(newNames)); List methodParameters= methodDeclaration.parameters(); // use short form with inferred parameter types and without parentheses if possible @@ -260,8 +272,9 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModel mod } } - private void makeNamesUnique(MethodDeclaration methodDeclaration, ASTRewrite rewrite, TextEditGroup group) { - List excludedNames= ASTNodes.getVisibleLocalVariablesInScope(methodDeclaration); + private HashSet makeNamesUnique(HashSet excludedNames, MethodDeclaration methodDeclaration, ASTRewrite rewrite, TextEditGroup group) { + HashSet newNames= new HashSet(); + excludedNames.addAll(ASTNodes.getVisibleLocalVariablesInScope(methodDeclaration)); List simpleNamesInMethod= getNamesInMethod(methodDeclaration); List namesInMethod= new ArrayList(); for (SimpleName name : simpleNamesInMethod) { @@ -271,20 +284,23 @@ private void makeNamesUnique(MethodDeclaration methodDeclaration, ASTRewrite rew for (int i= 0; i < simpleNamesInMethod.size(); i++) { SimpleName name= simpleNamesInMethod.get(i); String identifier= namesInMethod.get(i); - List allNamesToExclude= getNamesToExclude(excludedNames, namesInMethod, i); + HashSet allNamesToExclude= getNamesToExclude(excludedNames, namesInMethod, i); if (allNamesToExclude.contains(identifier)) { String newIdentifier= createName(identifier, allNamesToExclude); excludedNames.add(newIdentifier); + newNames.add(newIdentifier); SimpleName[] references= LinkedNodeFinder.findByNode(name.getRoot(), name); for (SimpleName ref : references) { rewrite.set(ref, SimpleName.IDENTIFIER_PROPERTY, newIdentifier, group); } } } + + return newNames; } - private List getNamesToExclude(List excludedNames, List namesInMethod, int i) { - List allNamesToExclude= new ArrayList(excludedNames); + private HashSet getNamesToExclude(HashSet excludedNames, List namesInMethod, int i) { + HashSet allNamesToExclude= new HashSet(excludedNames); allNamesToExclude.addAll(namesInMethod.subList(0, i)); allNamesToExclude.addAll(namesInMethod.subList(i + 1, namesInMethod.size())); return allNamesToExclude; @@ -333,7 +349,7 @@ public boolean visit(VariableDeclaration node) { return namesCollector.fNames; } - private String createName(String candidate, List excludedNames) { + private String createName(String candidate, HashSet excludedNames) { int i= 1; String result= candidate; while (excludedNames.contains(result)) {