Attachment 112076 Details for Bug 142990 – Proposed fix and regression tests

[patch] Proposed fix and regression tests

142990.txt (text/plain), 18.23 KB, created by Jerome Lanneluc

on 2008-09-09 09:08:10 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jerome Lanneluc

Created: 2008-09-09 09:08:10 EDT

Size: 18.23 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core.tests.model
>Index: src/org/eclipse/jdt/core/tests/model/RenameTests.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/RenameTests.java,v
>retrieving revision 1.28
>diff -u -r1.28 RenameTests.java
>--- src/org/eclipse/jdt/core/tests/model/RenameTests.java	29 Aug 2008 09:50:32 -0000	1.28
>+++ src/org/eclipse/jdt/core/tests/model/RenameTests.java	9 Sep 2008 13:06:17 -0000
>@@ -13,6 +13,7 @@
> import junit.framework.Test;
> 
> import org.eclipse.core.resources.IFile;
>+import org.eclipse.core.resources.IResource;
> import org.eclipse.core.resources.IWorkspaceRunnable;
> 
> import org.eclipse.core.resources.IncrementalProjectBuilder;
>@@ -316,7 +317,7 @@
> /**
>  *
>  */
>-public void testRenameCU() throws CoreException {
>+public void testRenameCU1() throws CoreException {
> 	this.cu.rename("NewX.java", false, null);
> 	assertTrue("Original CU should not exist", !this.cu.exists());
> 
>@@ -337,7 +338,7 @@
> 		"			X.java[-]: {MOVED_TO(NewX.java [in <default> [in src [in P]]])}"
> 	);
> }
>-public void testRenameCUForce() throws CoreException {
>+public void testRenameCU2() throws CoreException {
> 	this.createFile(
> 		"/P/src/Y.java",
> 		"public class Y {\n" +
>@@ -371,6 +372,35 @@
> 	);
> }
> /*
>+ * Ensures that the correct scheduling rule is used when running ICompilationUnit.rename(...)
>+ * (regression test for https://bugs.eclipse.org/bugs/show_bug.cgi?id=142990 )
>+ */
>+public void testRenameCU3() throws Exception {
>+	IWorkspaceRunnable runnable = new IWorkspaceRunnable(){
>+		public void run(IProgressMonitor monitor) throws CoreException {
>+			RenameTests.this.cu.rename("NewX.java", false, null);
>+			assertTrue("Original CU should not exist", !RenameTests.this.cu.exists());
>+		}
>+	};
>+	getWorkspace().run(runnable, getFolder("/P/src"), IResource.NONE, null);
>+}
>+/*
>+ * Ensures that the correct scheduling rule is used when running ICompilationUnit.rename(...)
>+ * (regression test for https://bugs.eclipse.org/bugs/show_bug.cgi?id=142990 )
>+ */
>+public void testRenameCU4() throws Exception {
>+	createFolder("/P/src/p1");
>+	createFile("/P/src/p1/X.java", "package p1; public class X {}");
>+	IWorkspaceRunnable runnable = new IWorkspaceRunnable(){
>+		public void run(IProgressMonitor monitor) throws CoreException {
>+			ICompilationUnit x = getCompilationUnit("/P/src/p1/X.java");
>+			x.rename("NewX.java", false, null);
>+			assertTrue("Original CU should not exist", !x.exists());
>+		}
>+	};
>+	getWorkspace().run(runnable, getFolder("/P/src/p1"), IResource.NONE, null);
>+}
>+/*
>  * Ensures that renaming an empty package fragment doesn't make it disappear
>  * (regression test for bug 24505 Refactoring an empty package makes it disappears)
>  */
>Index: src/org/eclipse/jdt/core/tests/model/CopyMoveResourcesTests.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/CopyMoveResourcesTests.java,v
>retrieving revision 1.25
>diff -u -r1.25 CopyMoveResourcesTests.java
>--- src/org/eclipse/jdt/core/tests/model/CopyMoveResourcesTests.java	29 Aug 2008 09:50:32 -0000	1.25
>+++ src/org/eclipse/jdt/core/tests/model/CopyMoveResourcesTests.java	9 Sep 2008 13:06:17 -0000
>@@ -13,6 +13,8 @@
> import junit.framework.Test;
> 
> import org.eclipse.core.resources.IFile;
>+import org.eclipse.core.resources.IResource;
>+import org.eclipse.core.resources.IWorkspaceRunnable;
> import org.eclipse.core.runtime.CoreException;
> import org.eclipse.core.runtime.IProgressMonitor;
> import org.eclipse.core.runtime.QualifiedName;
>@@ -214,7 +216,7 @@
> /**
>  * Ensures that a CU can be copied to a different package.
>  */
>-public void testCopyCU() throws CoreException {
>+public void testCopyCU01() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -236,7 +238,7 @@
>  * This operation should fail as copying a CU and a CU member at the
>  * same time is not supported.
>  */
>-public void testCopyCUAndType() throws CoreException {
>+public void testCopyCU02() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -257,7 +259,7 @@
> /**
>  * Ensures that a CU can be copied to a different package, replacing an existing CU.
>  */
>-public void testCopyCUForce() throws CoreException {
>+public void testCopyCU03() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -281,7 +283,7 @@
> /**
>  * Ensures that a CU can be copied from a default package to a non-default package.
>  */
>-public void testCopyCUFromDefaultToNonDefault() throws CoreException {
>+public void testCopyCU04() throws CoreException {
> 	createFile(
> 		"/P/src/X.java",
> 		"public class X {\n" +
>@@ -301,7 +303,7 @@
>  * Ensures that a CU can be copied to a different package,
>  * and be renamed.
>  */
>-public void testCopyCURename() throws CoreException {
>+public void testCopyCU05() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -319,7 +321,7 @@
> /**
>  * Ensures that a read-only CU can be copied to a different package.
>  */
>-public void testCopyCUReadOnly() throws CoreException {
>+public void testCopyCU06() throws CoreException {
> 	if (!Util.isReadOnlySupported()) {
> 		// Do not test if file system does not support read-only attribute
> 		return;
>@@ -359,7 +361,7 @@
>  * Ensures that a CU can be copied to a different package,
>  * and be renamed, overwriting an existing CU
>  */
>-public void testCopyCURenameForce() throws CoreException {
>+public void testCopyCU07() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -383,7 +385,7 @@
> /**
>  * Ensures that a CU cannot be copied to a different package,over an existing CU when no force.
>  */
>-public void testCopyCUWithCollision() throws CoreException {
>+public void testCopyCU08() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -407,7 +409,7 @@
> /**
>  * Ensures that a CU cannot be copied to an invalid destination
>  */
>-public void testCopyCUWithInvalidDestination() throws CoreException {
>+public void testCopyCU09() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -422,7 +424,7 @@
> /**
>  * Ensures that a CU can be copied to a null container
>  */
>-public void testCopyCUWithNullContainer() throws CoreException {
>+public void testCopyCU10() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -443,7 +445,7 @@
>  * Ensures that a CU can be copied to along with its server properties.
>  * (Regression test for PR #1G56QT9)
>  */
>-public void testCopyCUWithServerProperties() throws CoreException {
>+public void testCopyCU11() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -470,6 +472,25 @@
> 		propertyValue
> 	);
> }
>+/*
>+ * Ensures that the correct scheduling rule is used when copying a CU
>+ * (regression test for https://bugs.eclipse.org/bugs/show_bug.cgi?id=142990 )
>+ */
>+public void testCopyCU12() throws Exception {
>+	createFolder("/P/src/p1");
>+	createFile("/P/src/p1/X.java", "package p1; public class X {}");
>+	this.createFolder("/P/src/p2");
>+
>+	IWorkspaceRunnable runnable = new IWorkspaceRunnable(){
>+		public void run(IProgressMonitor monitor) throws CoreException {
>+			ICompilationUnit cuSource = getCompilationUnit("/P/src/p1/X.java");
>+			IPackageFragment pkgDest = getPackage("/P/src/p2");
>+			cuSource.copy(pkgDest, null, null, false, null);
>+			assertTrue("/P/src/p2/X.java should exist", getCompilationUnit("/P/src/p2/X.java").exists());
>+		}
>+	};
>+	getWorkspace().run(runnable, getFolder("/P/src/p2"), IResource.NONE, null);
>+}
> /**
>  * Ensures that a package fragment can be copied to a different package fragment root.
>  */
>@@ -743,7 +764,7 @@
> /**
>  * Ensures that a CU can be moved to a different package.
>  */
>-public void testMoveCU() throws CoreException {
>+public void testMoveCU01() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -765,7 +786,7 @@
>  * This operation should fail as moving a CU and a CU member at the
>  * same time is not supported.
>  */
>-public void testMoveCUAndType() throws CoreException {
>+public void testMoveCU02() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -787,7 +808,7 @@
>  * Ensures that a CU can be moved to a different package, replacing an
>  * existing CU.
>  */
>-public void testMoveCUForce() throws CoreException {
>+public void testMoveCU03() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -812,7 +833,7 @@
>  * Ensures that a CU can be moved to a different package,
>  * be renamed
>  */
>-public void testMoveCURename() throws CoreException {
>+public void testMoveCU04() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -831,7 +852,7 @@
>  * Ensures that a CU can be moved to a different package,
>  * be renamed, overwriting an existing resource.
>  */
>-public void testMoveCURenameForce() throws CoreException {
>+public void testMoveCU05() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -856,7 +877,7 @@
>  * Ensures that a CU cannot be moved to a different package, replacing an
>  * existing CU when not forced.
>  */
>-public void testMoveCUWithCollision() throws CoreException {
>+public void testMoveCU06() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -880,7 +901,7 @@
> /**
>  * Ensures that a CU cannot be moved to an invalid destination.
>  */
>-public void testMoveCUWithInvalidDestination() throws CoreException {
>+public void testMoveCU07() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -895,7 +916,7 @@
> /**
>  * Ensures that a CU cannot be moved to a null container
>  */
>-public void testMoveCUWithNullContainer() throws CoreException {
>+public void testMoveCU08() throws CoreException {
> 	this.createFolder("/P/src/p1");
> 	this.createFile(
> 		"/P/src/p1/X.java",
>@@ -912,6 +933,25 @@
> 	}
> 	assertTrue("Should not be able to move a cu to a null container", false);
> }
>+/*
>+ * Ensures that the correct scheduling rule is used when moving a CU
>+ * (regression test for https://bugs.eclipse.org/bugs/show_bug.cgi?id=142990 )
>+ */
>+public void testMoveCU09() throws Exception {
>+	createFolder("/P/src/p1");
>+	createFile("/P/src/p1/X.java", "package p1; public class X {}");
>+	this.createFolder("/P/src/p2");
>+
>+	IWorkspaceRunnable runnable = new IWorkspaceRunnable(){
>+		public void run(IProgressMonitor monitor) throws CoreException {
>+			ICompilationUnit cuSource = getCompilationUnit("/P/src/p1/X.java");
>+			IPackageFragment pkgDest = getPackage("/P/src/p2");
>+			cuSource.move(pkgDest, null, null, false, null);
>+			assertFalse("/P/src/p1/X.java should no longer exist", cuSource.exists());
>+		}
>+	};
>+	getWorkspace().run(runnable, getFolder("/P/src"), IResource.NONE, null);
>+}
> /**
>  * Ensures that a package fragment can be moved to a different package fragment root.
>  */
>#P org.eclipse.jdt.core
>Index: model/org/eclipse/jdt/internal/core/JavaModelOperation.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaModelOperation.java,v
>retrieving revision 1.76
>diff -u -r1.76 JavaModelOperation.java
>--- model/org/eclipse/jdt/internal/core/JavaModelOperation.java	2 Jul 2008 13:52:58 -0000	1.76
>+++ model/org/eclipse/jdt/internal/core/JavaModelOperation.java	9 Sep 2008 13:06:18 -0000
>@@ -247,11 +247,17 @@
> 	/**
> 	 * Convenience method to copy resources
> 	 */
>-	protected void copyResources(IResource[] resources, IPath destinationPath) throws JavaModelException {
>+	protected void copyResources(IResource[] resources, IPath container) throws JavaModelException {
> 		IProgressMonitor subProgressMonitor = getSubProgressMonitor(resources.length);
>-		IWorkspace workspace = resources[0].getWorkspace();
>+		IWorkspaceRoot root =  ResourcesPlugin.getWorkspace().getRoot();
> 		try {
>-			workspace.copy(resources, destinationPath, false, subProgressMonitor);
>+			for (int i = 0, length = resources.length; i < length; i++) {
>+				IResource resource = resources[i];
>+				IPath destination = container.append(resource.getName());
>+				if (root.findMember(destination) == null) {
>+					resource.copy(destination, false, subProgressMonitor);
>+				}
>+			}
> 			setAttribute(HAS_MODIFIED_RESOURCE_ATTR, TRUE);
> 		} catch (CoreException e) {
> 			throw new JavaModelException(e);
>@@ -561,14 +567,20 @@
> 	/**
> 	 * Convenience method to move resources
> 	 */
>-	protected void moveResources(IResource[] resources, IPath destinationPath) throws JavaModelException {
>+	protected void moveResources(IResource[] resources, IPath container) throws JavaModelException {
> 		IProgressMonitor subProgressMonitor = null;
> 		if (this.progressMonitor != null) {
> 			subProgressMonitor = new SubProgressMonitor(this.progressMonitor, resources.length, SubProgressMonitor.PREPEND_MAIN_LABEL_TO_SUBTASK);
> 		}
>-		IWorkspace workspace = resources[0].getWorkspace();
>+		IWorkspaceRoot root = ResourcesPlugin.getWorkspace().getRoot();
> 		try {
>-			workspace.move(resources, destinationPath, false, subProgressMonitor);
>+			for (int i = 0, length = resources.length; i < length; i++) {
>+				IResource resource = resources[i];
>+				IPath destination = container.append(resource.getName());
>+				if (root.findMember(destination) == null) {
>+					resource.move(destination, false, subProgressMonitor);
>+				}
>+			}
> 			setAttribute(HAS_MODIFIED_RESOURCE_ATTR, TRUE);
> 		} catch (CoreException e) {
> 			throw new JavaModelException(e);
>Index: model/org/eclipse/jdt/internal/core/CopyResourceElementsOperation.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/CopyResourceElementsOperation.java,v
>retrieving revision 1.109
>diff -u -r1.109 CopyResourceElementsOperation.java
>--- model/org/eclipse/jdt/internal/core/CopyResourceElementsOperation.java	27 Jun 2008 16:03:51 -0000	1.109
>+++ model/org/eclipse/jdt/internal/core/CopyResourceElementsOperation.java	9 Sep 2008 13:06:18 -0000
>@@ -16,6 +16,8 @@
> import org.eclipse.core.runtime.CoreException;
> import org.eclipse.core.runtime.IPath;
> import org.eclipse.core.runtime.Path;
>+import org.eclipse.core.runtime.jobs.ISchedulingRule;
>+import org.eclipse.core.runtime.jobs.MultiRule;
> import org.eclipse.jdt.core.*;
> import org.eclipse.jdt.core.dom.AST;
> import org.eclipse.jdt.core.dom.ASTParser;
>@@ -73,13 +75,6 @@
> 	protected ASTParser parser;
> 	/**
> 	 * When executed, this operation will copy the given resources to the
>-	 * given container.
>-	 */
>-	public CopyResourceElementsOperation(IJavaElement[] resourcesToCopy, IJavaElement destContainer, boolean force) {
>-		this(resourcesToCopy, new IJavaElement[]{destContainer}, force);
>-	}
>-	/**
>-	 * When executed, this operation will copy the given resources to the
> 	 * given containers.  The resources and destination containers must be in
> 	 * the correct order. If there is > 1 destination, the number of destinations
> 	 * must be the same as the number of resources being copied/moved.
>@@ -192,6 +187,76 @@
> 	protected String getMainTaskName() {
> 		return Messages.operation_copyResourceProgress;
> 	}
>+	protected ISchedulingRule getSchedulingRule() {
>+		if (this.elementsToProcess == null)
>+			return null;
>+		int length = this.elementsToProcess.length;
>+		if (length == 1)
>+			return getSchedulingRule(this.elementsToProcess[0]);
>+		ISchedulingRule[] rules = new ISchedulingRule[length];
>+		int index = 0;
>+		for (int i = 0; i < length; i++) {
>+			ISchedulingRule rule = getSchedulingRule(this.elementsToProcess[i]);
>+			if (rule != null) {
>+				rules[index++] = rule;
>+			}
>+		}
>+		if (index != length)
>+			System.arraycopy(rules, 0, rules = new ISchedulingRule[index], 0, index);
>+		return new MultiRule(rules);
>+	}
>+	private ISchedulingRule getSchedulingRule(IJavaElement element) {
>+		if (element == null)
>+			return null;
>+		IResource sourceResource = getResource(element);
>+		IResource destContainer = getResource(getDestinationParent(element));
>+		if (!(destContainer instanceof IContainer)) {
>+			return null;
>+		}
>+		String newName;
>+		try {
>+			newName = getNewNameFor(element);
>+		} catch (JavaModelException e) {
>+			return null;
>+		}
>+		if (newName == null)
>+			newName = element.getElementName();
>+		IResource destResource;
>+		String sourceEncoding = null;
>+		if (sourceResource.getType() == IResource.FILE) {
>+			destResource = ((IContainer) destContainer).getFile(new Path(newName));
>+			try {
>+				sourceEncoding = ((IFile) sourceResource).getCharset(false);
>+			} catch (CoreException ce) {
>+				// use default encoding
>+			}
>+		} else {
>+			destResource = ((IContainer) destContainer).getFolder(new Path(newName));
>+		}
>+		IResourceRuleFactory factory = ResourcesPlugin.getWorkspace().getRuleFactory();
>+		ISchedulingRule rule;
>+		if (isMove()) {
>+			rule = factory.moveRule(sourceResource, destResource);
>+		} else {
>+			rule = factory.copyRule(sourceResource, destResource);
>+		}
>+		if (sourceEncoding != null) {
>+			rule = new MultiRule(new ISchedulingRule[] {rule, factory.charsetRule(destResource)});
>+		}
>+		return rule;
>+	}
>+	private IResource getResource(IJavaElement element) {
>+		if (element == null)
>+			return null;
>+		if (element.getElementType() == IJavaElement.PACKAGE_FRAGMENT) {
>+			String pkgName = element.getElementName();
>+			int firstDot = pkgName.indexOf('.');
>+			if (firstDot != -1) {
>+				element = ((IPackageFragmentRoot) element.getParent()).getPackageFragment(pkgName.substring(0, firstDot));
>+			}
>+		}
>+		return element.getResource();
>+	}
> 	/**
> 	 * Sets the deltas to register the changes resulting from this operation
> 	 * for this source element and its destination.

Actions: View | Diff

Attachments on bug 142990: 42250 | 112076