Attachment 243874 Details for Bug 151668 – Patch proposal v2

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Patch proposal v2

patch-151668-v2.diff (text/plain), 15.73 KB, created by Yves Joan

on 2014-06-03 06:21:37 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yves Joan

Created: 2014-06-03 06:21:37 EDT

Size: 15.73 KB

patch

obsolete

>diff --git a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ccp/CopyTest.java b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ccp/CopyTest.java
>index 43d2548..67e7c13 100644
>--- a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ccp/CopyTest.java	
>+++ b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ccp/CopyTest.java	
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2000, 2012 IBM Corporation and others.
>+ * Copyright (c) 2000, 2014 IBM Corporation and others.
>  * All rights reserved. This program and the accompanying materials
>  * are made available under the terms of the Eclipse Public License v1.0
>  * which accompanies this distribution, and is available at
>@@ -7,6 +7,7 @@
>  *
>  * Contributors:
>  *     IBM Corporation - initial API and implementation
>+ *     Yves Joan <yves.joan@oracle.com> - [reorg] Copy action should NOT add 'copy of' prefix - https://bugs.eclipse.org/bugs/show_bug.cgi?id=151668
>  *******************************************************************************/
> package org.eclipse.jdt.ui.tests.refactoring.ccp;
> 
>@@ -173,12 +174,16 @@ public class CopyTest extends RefactoringTest {
> 		private static final String NEW_FILE_NAME= "UnusedName.gif";
> 		private static final String NEW_FOLDER_NAME= "UnusedName";
> 		private static final String NEW_CU_NAME= "UnusedName";
>+		private String fCuInitialSuggestedName= "unset";
>+		private String fResourceInitialSuggestedName= "unset";
> 
> 		public INewNameQuery createNewCompilationUnitNameQuery(ICompilationUnit cu, String s) {
>+			setCuInitialSuggestedName(s);
> 			return createStaticQuery(NEW_CU_NAME);
> 		}
> 
> 		public INewNameQuery createNewResourceNameQuery(IResource res, String s) {
>+			setResourceInitialSuggestedName(s);
> 			if (res instanceof IFile)
> 				return createStaticQuery(NEW_FILE_NAME);
> 			else
>@@ -204,6 +209,22 @@ public class CopyTest extends RefactoringTest {
> 		public INewNameQuery createNewPackageFragmentRootNameQuery(IPackageFragmentRoot root, String initialSuggestedName) {
> 			return createStaticQuery(NEW_PACKAGE_FRAGMENT_ROOT_NAME);
> 		}
>+
>+		public String getCuInitialSuggestedName() {
>+			return fCuInitialSuggestedName;
>+		}
>+
>+		private void setCuInitialSuggestedName(String cuInitialSuggestedName) {
>+			this.fCuInitialSuggestedName= cuInitialSuggestedName;
>+		}
>+
>+		public String getResourceInitialSuggestedName() {
>+			return fResourceInitialSuggestedName;
>+		}
>+
>+		public void setResourceInitialSuggestedName(String resourceInitialSuggestedName) {
>+			fResourceInitialSuggestedName= resourceInitialSuggestedName;
>+		}
> 	}
> 
> 	private static class MockCancelNameQueries implements INewNameQueries{
>@@ -1481,6 +1502,7 @@ public class CopyTest extends RefactoringTest {
> 		ParticipantTesting.testCopy(handles, new CopyArguments[] {
> 				new CopyArguments(destination, log)
> 		});
>+		assertEquals("a2.txt", ((MockNewNameQueries)queries).getResourceInitialSuggestedName());
> 	}
> 
> 	public void testCopy_File_to_Same_Folder_Cancel() throws Exception {
>@@ -1541,6 +1563,36 @@ public class CopyTest extends RefactoringTest {
> 		assertTrue("new file does not exist after copying", newFile.exists());
> 	}
> 
>+	public void testCopy_File_to_Itself_2() throws Exception {
>+		IFolder parentFolder= (IFolder)getPackageP().getResource();
>+		String fileName= "A.java";
>+		IFile file= parentFolder.getFile(fileName);
>+		file.create(getStream("123"), true, null);
>+		String conflictingFileName= "A2.java";
>+		IFile conflictingFile= parentFolder.getFile(conflictingFileName);
>+		conflictingFile.create(getStream("456"), true, null);
>+
>+		INewNameQueries queries= new MockNewNameQueries();
>+
>+		IJavaElement[] javaElements= {};
>+		IResource[] resources= { file };
>+		JavaCopyProcessor ref= verifyEnabled(resources, javaElements, queries, createReorgQueries());
>+
>+		Object destination= file;
>+		verifyValidDestination(ref, destination);
>+
>+		assertTrue("source file does not exist before copying", file.exists());
>+
>+		RefactoringStatus status= performRefactoring(ref, false);
>+		assertEquals(null, status);
>+
>+		assertTrue("source file does not exist after copying", file.exists());
>+
>+		IFile newFile= parentFolder.getFile(MockNewNameQueries.NEW_FILE_NAME);
>+		assertTrue("new file does not exist after copying", newFile.exists());
>+		assertEquals("A3.java", ((MockNewNameQueries)queries).getResourceInitialSuggestedName());
>+	}
>+
> 	public void testCopy_File_to_AnotherFile() throws Exception {
> 		IFolder parentFolder= (IFolder) getPackageP().getResource();
> 		String fileName= "a.txt";
>@@ -1919,6 +1971,7 @@ public class CopyTest extends RefactoringTest {
> 
> 		ICompilationUnit newCu= getPackageP().getCompilationUnit(MockNewNameQueries.NEW_CU_NAME + ".java");
> 		assertTrue("new file does not exist after copying", newCu.exists());
>+		assertEquals("A2", ((MockNewNameQueries)queries).getCuInitialSuggestedName());
> 	}
> 
> 	public void testCopy_Cu_to_OtherPackage() throws Exception {
>@@ -2322,7 +2375,47 @@ public class CopyTest extends RefactoringTest {
> 		folder.create(true, true, null);
> 		IJavaElement[] javaElements= {};
> 		IResource[] resources= {folder};
>-		JavaCopyProcessor ref= verifyEnabled(resources, javaElements, new MockNewNameQueries(), createReorgQueries());
>+		INewNameQueries queries= new MockNewNameQueries();
>+
>+		JavaCopyProcessor ref= verifyEnabled(resources, javaElements, queries, createReorgQueries());
>+		String[] handles= ParticipantTesting.createHandles(folder);
>+
>+		Object destination= superFolder;
>+		verifyValidDestination(ref, destination);
>+
>+		assertTrue("source does not exist before copying", folder.exists());
>+
>+		RefactoringStatus status= performRefactoring(ref, false);
>+		assertEquals(null, status);
>+
>+		assertTrue("source does not exist after copying", folder.exists());
>+		IFolder newFolder= superFolder.getFolder(MockNewNameQueries.NEW_FOLDER_NAME);
>+		assertTrue("copied folder does not exist after copying", newFolder.exists());
>+		ReorgExecutionLog log= new ReorgExecutionLog();
>+		log.markAsProcessed(folder);
>+		log.setNewName(folder, MockNewNameQueries.NEW_FOLDER_NAME);
>+		ParticipantTesting.testCopy(handles, new CopyArguments[] {
>+				new CopyArguments(destination, log)
>+		});
>+		assertEquals(folderName + "2", ((MockNewNameQueries)queries).getResourceInitialSuggestedName());
>+
>+	}
>+
>+	public void testCopy_folder_to_same_container_2() throws Exception {
>+		ParticipantTesting.reset();
>+		IProject superFolder= RefactoringTestSetup.getProject().getProject();
>+		String folderName= "folder.name.with.segments";
>+		IFolder folder= superFolder.getFolder(folderName);
>+		folder.create(true, true, null);
>+		IJavaElement[] javaElements= {};
>+		IResource[] resources= { folder };
>+		INewNameQueries queries= new MockNewNameQueries();
>+
>+		String secondFolderName= "folder.name.with.segments2";
>+		IFolder secondFolder= superFolder.getFolder(secondFolderName);
>+		secondFolder.create(true, true, null);
>+
>+		JavaCopyProcessor ref= verifyEnabled(resources, javaElements, queries, createReorgQueries());
> 		String[] handles= ParticipantTesting.createHandles(folder);
> 
> 		Object destination= superFolder;
>@@ -2342,6 +2435,7 @@ public class CopyTest extends RefactoringTest {
> 		ParticipantTesting.testCopy(handles, new CopyArguments[] {
> 				new CopyArguments(destination, log)
> 		});
>+		assertEquals(folderName + "3", ((MockNewNameQueries)queries).getResourceInitialSuggestedName());
> 
> 	}
> 
>diff --git a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/RefactoringCoreMessages.java b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/RefactoringCoreMessages.java
>index 64b339a..0871718 100644
>--- a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/RefactoringCoreMessages.java	
>+++ b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/RefactoringCoreMessages.java	
>@@ -9,6 +9,7 @@
>  *     IBM Corporation - initial API and implementation
>  *     Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] Extract method and continue https://bugs.eclipse.org/bugs/show_bug.cgi?id=48056
>  *     Samrat Dhillon <samrat.dhillon@gmail.com> - [introduce factory] Introduce Factory on an abstract class adds a statement to create an instance of that class - https://bugs.eclipse.org/bugs/show_bug.cgi?id=395016
>+ *     Yves Joan <yves.joan@oracle.com> - [reorg] Copy action should NOT add 'copy of' prefix - https://bugs.eclipse.org/bugs/show_bug.cgi?id=151668
>  *******************************************************************************/
> package org.eclipse.jdt.internal.corext.refactoring;
> 
>@@ -276,18 +277,10 @@ public final class RefactoringCoreMessages extends NLS {
> 
> 	public static String CopyPackageFragmentRootChange_copy;
> 
>-	public static String CopyRefactoring_cu_copyOf1;
>-
>-	public static String CopyRefactoring_cu_copyOfMore;
>-
> 	public static String CopyRefactoring_package_copyOf1;
> 
> 	public static String CopyRefactoring_package_copyOfMore;
> 
>-	public static String CopyRefactoring_resource_copyOf1;
>-
>-	public static String CopyRefactoring_resource_copyOfMore;
>-
> 	public static String CopyRefactoring_update_ref;
> 
> 	public static String CopyResourceString_copy;
>diff --git a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/refactoring.properties b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/refactoring.properties
>index d28219a..ac8576e 100644
>--- a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/refactoring.properties	
>+++ b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/refactoring.properties	
>@@ -10,6 +10,7 @@
> #     Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] Extract method and continue https://bugs.eclipse.org/bugs/show_bug.cgi?id=48056
> #     Benjamin Muskalla <bmuskalla@eclipsesource.com> - [extract method] Name ambiguous return value in error message - https://bugs.eclipse.org/bugs/show_bug.cgi?id=50607
> #	  Samrat Dhillon <samrat.dhillon@gmail.com> - [introduce factory] Introduce Factory on an abstract class adds a statement to create an instance of that class - https://bugs.eclipse.org/bugs/show_bug.cgi?id=395016
>+#     Yves Joan <yves.joan@oracle.com> - [reorg] Copy action should NOT add 'copy of' prefix - https://bugs.eclipse.org/bugs/show_bug.cgi?id=151668
> ###############################################################################
> # NLS properties for the Refactoring Core
> 
>@@ -579,10 +580,6 @@ MoveCuUpdateCreator_update_imports=Update imports
> MoveCuUpdateCreator_searching=Searching for references to types in ''{0}''
> MoveCuUpdateCreator_update_references=Update references
> 
>-CopyRefactoring_cu_copyOf1=CopyOf{0}
>-CopyRefactoring_cu_copyOfMore=Copy_{0}_of_{1}
>-CopyRefactoring_resource_copyOf1=Copy of {0}
>-CopyRefactoring_resource_copyOfMore=Copy ({0}) of {1}
> CopyRefactoring_package_copyOf1={0}.copy
> CopyRefactoring_package_copyOfMore={1}.copy{0}
> 
>diff --git a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/reorg/ReorgPolicyFactory.java b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/reorg/ReorgPolicyFactory.java
>index 618686b..f274119 100644
>--- a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/reorg/ReorgPolicyFactory.java	
>+++ b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/reorg/ReorgPolicyFactory.java	
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2000, 2013 IBM Corporation and others.
>+ * Copyright (c) 2000, 2014 IBM Corporation and others.
>  * All rights reserved. This program and the accompanying materials
>  * are made available under the terms of the Eclipse Public License v1.0
>  * which accompanies this distribution, and is available at
>@@ -8,6 +8,7 @@
>  * Contributors:
>  *     IBM Corporation - initial API and implementation
>  *     Carsten Pfeiffer <carsten.pfeiffer@gebit.de> - [ccp] ReorgPolicies' canEnable() methods return true too often - https://bugs.eclipse.org/bugs/show_bug.cgi?id=303698
>+ *     Yves Joan <yves.joan@oracle.com> - [reorg] Copy action should NOT add 'copy of' prefix - https://bugs.eclipse.org/bugs/show_bug.cgi?id=151668
>  *******************************************************************************/
> package org.eclipse.jdt.internal.corext.refactoring.reorg;
> 
>@@ -22,6 +23,8 @@ import java.util.List;
> import java.util.Map;
> import java.util.Set;
> import java.util.StringTokenizer;
>+import java.util.regex.Matcher;
>+import java.util.regex.Pattern;
> 
> import org.eclipse.core.runtime.Assert;
> import org.eclipse.core.runtime.CoreException;
>@@ -2557,23 +2560,42 @@ public final class ReorgPolicyFactory {
> 
> 		private final Set<String> fAutoGeneratedNewNames= new HashSet<String>(2);
> 
>+		private String computeNewName(String str, int resourceType) {
>+        	final int lastIndexOfDot= str.lastIndexOf('.');
>+            String fileExtension= ""; //$NON-NLS-1$
>+            String fileNameNoExtension= str;
>+			if (lastIndexOfDot != -1 && resourceType == IResource.FILE) {
>+            	fileExtension= str.substring(lastIndexOfDot);
>+            	fileNameNoExtension= str.substring(0, lastIndexOfDot);
>+            }
>+            final Pattern p= Pattern.compile("[0-9]+$"); //$NON-NLS-1$
>+            final Matcher m= p.matcher(fileNameNoExtension);
>+            if (m.find()) {
>+                // String ends with a number: increment it by 1
>+                final Integer newNumber= new Integer(Integer.parseInt(m.group()) + 1);
>+                final String numberStr= m.replaceFirst(newNumber.toString());
>+                return numberStr + fileExtension;
>+            } else {
>+                return fileNameNoExtension + "2" + fileExtension; //$NON-NLS-1$
>+            }
>+        }
>+
> 		public String createNewName(ICompilationUnit cu, IPackageFragment destination) {
> 			if (isNewNameOk(destination, cu.getElementName()))
> 				return null;
> 			if (!ReorgUtils.isParentInWorkspaceOrOnDisk(cu, destination))
> 				return null;
>-			int i= 1;
>+			
>+			int resourceType= cu.getResource().getType();
>+			String newName= computeNewName(cu.getElementName(), resourceType);
>+			
> 			while (true) {
>-				String newName;
>-				if (i == 1)
>-					newName= Messages.format(RefactoringCoreMessages.CopyRefactoring_cu_copyOf1, cu.getElementName()); // Don't use BasicElementLabels! No RTL!
>-				else
>-					newName= Messages.format(RefactoringCoreMessages.CopyRefactoring_cu_copyOfMore, new String[] { String.valueOf(i), cu.getElementName()}); // Don't use BasicElementLabels! No RTL!
> 				if (isNewNameOk(destination, newName) && !fAutoGeneratedNewNames.contains(newName)) {
> 					fAutoGeneratedNewNames.add(newName);
> 					return JavaCore.removeJavaLikeExtension(newName);
>+				} else {
>+					newName= computeNewName(newName, resourceType);
> 				}
>-				i++;
> 			}
> 		}
> 
>@@ -2602,18 +2624,17 @@ public final class ReorgPolicyFactory {
> 				return null;
> 			if (!ReorgUtils.isParentInWorkspaceOrOnDisk(res, destination))
> 				return null;
>-			int i= 1;
>+			
>+			int resourceType= res.getType();
>+			String newName= computeNewName(res.getName(), resourceType);
>+			
> 			while (true) {
>-				String newName;
>-				if (i == 1)
>-					newName= Messages.format(RefactoringCoreMessages.CopyRefactoring_resource_copyOf1, res.getName()); // Don't use BasicElementLabels! No RTL!
>-				else
>-					newName= Messages.format(RefactoringCoreMessages.CopyRefactoring_resource_copyOfMore, new String[] { String.valueOf(i), res.getName()}); // Don't use BasicElementLabels! No RTL!
> 				if (isNewNameOk(destination, newName) && !fAutoGeneratedNewNames.contains(newName)) {
> 					fAutoGeneratedNewNames.add(newName);
> 					return newName;
>+				} else {
>+					newName= computeNewName(newName, resourceType);
> 				}
>-				i++;
> 			}
> 		}
> 	}

Actions: View | Diff

Attachments on bug 151668: 243164 | 243874 | 244110