diff options
author | Tor Norbye <tnorbye@google.com> | 2012-12-04 20:13:32 -0800 |
---|---|---|
committer | Tor Norbye <tnorbye@google.com> | 2012-12-05 08:39:00 -0800 |
commit | 7e3452665f6672c8a42710b799a1f69e26e3d98d (patch) | |
tree | 4bd0d8edb00a390d41be8ef70b37ced6644f021c | |
parent | 154ef78e9c8fc661149909e1b13186da114245dd (diff) | |
download | sdk-7e3452665f6672c8a42710b799a1f69e26e3d98d.zip sdk-7e3452665f6672c8a42710b799a1f69e26e3d98d.tar.gz sdk-7e3452665f6672c8a42710b799a1f69e26e3d98d.tar.bz2 |
Fix Rename Application Package such that it doesn't touch bin/
Fix the rename application package refactoring such that it only
modifies AndroidManifest.xml, not for example a copy in bin/.
Also added a diagnostic information message to the plain Java
package refactoring, shown when refactoring the application
package, explaining that the refactoring will not actually change
the manifest package, and explaining how to invoke the dedicated
refactoring to change it.
Also add unit tests for this refactoring.
Change-Id: I13054e033b9ee44210ca341f00c633d73a4418b4
7 files changed, 261 insertions, 21 deletions
diff --git a/eclipse/dictionary.txt b/eclipse/dictionary.txt index 7d6f931..f8d6550 100644 --- a/eclipse/dictionary.txt +++ b/eclipse/dictionary.txt @@ -233,6 +233,7 @@ recompilation rect redo refactor +refactored refactoring refactorings regex diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/core/AndroidPackageRenameParticipant.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/core/AndroidPackageRenameParticipant.java index 67f7d44..1455af6 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/core/AndroidPackageRenameParticipant.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/core/AndroidPackageRenameParticipant.java @@ -32,6 +32,7 @@ import com.android.annotations.NonNull; import com.android.ide.common.xml.ManifestData; import com.android.ide.eclipse.adt.AdtConstants; import com.android.ide.eclipse.adt.AdtPlugin; +import com.android.ide.eclipse.adt.internal.editors.layout.gle2.DomUtilities; import com.android.ide.eclipse.adt.internal.project.AndroidManifestHelper; import com.android.resources.ResourceFolderType; import com.android.utils.SdkUtils; @@ -51,10 +52,14 @@ import org.eclipse.jdt.core.JavaModelException; import org.eclipse.jdt.internal.corext.refactoring.changes.RenamePackageChange; import org.eclipse.jdt.internal.corext.refactoring.rename.RenameCompilationUnitProcessor; import org.eclipse.jdt.internal.corext.refactoring.rename.RenameTypeProcessor; +import org.eclipse.jface.text.IRegion; +import org.eclipse.jface.text.Region; import org.eclipse.ltk.core.refactoring.Change; import org.eclipse.ltk.core.refactoring.CompositeChange; +import org.eclipse.ltk.core.refactoring.FileStatusContext; import org.eclipse.ltk.core.refactoring.NullChange; import org.eclipse.ltk.core.refactoring.RefactoringStatus; +import org.eclipse.ltk.core.refactoring.RefactoringStatusContext; import org.eclipse.ltk.core.refactoring.TextFileChange; import org.eclipse.ltk.core.refactoring.participants.CheckConditionsContext; import org.eclipse.ltk.core.refactoring.participants.RefactoringProcessor; @@ -65,9 +70,11 @@ import org.eclipse.text.edits.TextEdit; import org.eclipse.wst.sse.core.StructuredModelManager; import org.eclipse.wst.sse.core.internal.provisional.IModelManager; import org.eclipse.wst.sse.core.internal.provisional.IStructuredModel; +import org.eclipse.wst.sse.core.internal.provisional.IndexedRegion; import org.eclipse.wst.sse.core.internal.provisional.text.IStructuredDocument; import org.eclipse.wst.xml.core.internal.provisional.document.IDOMModel; import org.w3c.dom.Attr; +import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NamedNodeMap; import org.w3c.dom.Node; @@ -107,6 +114,44 @@ public class AndroidPackageRenameParticipant extends RenameParticipant { @Override public RefactoringStatus checkConditions(IProgressMonitor pm, CheckConditionsContext context) throws OperationCanceledException { + if (mAppPackage.equals(mOldPackage) && !mRefactoringAppPackage) { + IRegion region = null; + Document document = DomUtilities.getDocument(mManifestFile); + if (document != null && document.getDocumentElement() != null) { + Attr attribute = document.getDocumentElement().getAttributeNode(ATTR_PACKAGE); + if (attribute instanceof IndexedRegion) { + IndexedRegion ir = (IndexedRegion) attribute; + int start = ir.getStartOffset(); + region = new Region(start, ir.getEndOffset() - start); + } + } + if (region == null) { + region = new Region(0, 0); + } + // There's no line wrapping in the error dialog, so split up the message into + // individually digestible pieces of information + RefactoringStatusContext ctx = new FileStatusContext(mManifestFile, region); + RefactoringStatus status = RefactoringStatus.createInfoStatus( + "You are refactoring the same package as your application's " + + "package (specified in the manifest).\n", ctx); + status.addInfo( + "Note that this refactoring does NOT also update your " + + "application package.", ctx); + status.addInfo("The application package defines your application's identity.", ctx); + status.addInfo( + "If you change it, then it is considered to be a different application.", ctx); + status.addInfo("(Users of the previous version cannot update to the new version.)", + ctx); + status.addInfo( + "The application package, and the package containing the code, can differ.", + ctx); + status.addInfo( + "To really change application package, " + + "choose \"Android Tools\" > \"Rename Application Package.\" " + + "from the project context menu.", ctx); + return status; + } + return new RefactoringStatus(); } diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/renamepackage/ApplicationPackageNameRefactoring.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/renamepackage/ApplicationPackageNameRefactoring.java index ec40a6e..95ae529 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/renamepackage/ApplicationPackageNameRefactoring.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/renamepackage/ApplicationPackageNameRefactoring.java @@ -436,10 +436,13 @@ class ApplicationPackageNameRefactoring extends Refactoring { } else if (SdkConstants.EXT_XML.equals(file.getFileExtension())) { if (SdkConstants.FN_ANDROID_MANIFEST_XML.equals(file.getName())) { - - TextFileChange manifest_change = editAndroidManifest(file); - mChanges.add(manifest_change); - + // Ensure that this is the root manifest, not some other copy + // (such as the one in bin/) + IPath path = file.getFullPath(); + if (path.segmentCount() == 2) { + TextFileChange manifest_change = editAndroidManifest(file); + mChanges.add(manifest_change); + } } else { // Currently we only support Android resource XML files, diff --git a/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/AndroidPackageRenameParticipantTest.java b/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/AndroidPackageRenameParticipantTest.java index a074146..bff6a0b 100644 --- a/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/AndroidPackageRenameParticipantTest.java +++ b/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/AndroidPackageRenameParticipantTest.java @@ -61,7 +61,8 @@ public class AndroidPackageRenameParticipantTest extends RefactoringTestBase { " + android:name=\"my.pkg.name.MainActivity\"\n" + " @@ -25 +25\n" + " - android:name=\".MainActivity2\"\n" + - " + android:name=\"my.pkg.name.MainActivity2\""); + " + android:name=\"my.pkg.name.MainActivity2\"", + true); } public void testRefactor1_noreferences() throws Exception { @@ -73,7 +74,8 @@ public class AndroidPackageRenameParticipantTest extends RefactoringTestBase { "CHANGES:\n" + "-------\n" + - "* Rename package 'com.example.refactoringtest' to 'my.pkg.name'"); + "* Rename package 'com.example.refactoringtest' to 'my.pkg.name'", + false); } public void testRefactor2() throws Exception { @@ -118,7 +120,8 @@ public class AndroidPackageRenameParticipantTest extends RefactoringTestBase { " + android:name=\"my.pkg.name.MainActivity\"\n" + " @@ -25 +25\n" + " - android:name=\".MainActivity2\"\n" + - " + android:name=\"my.pkg.name.MainActivity2\""); + " + android:name=\"my.pkg.name.MainActivity2\"", + true); } public void testRefactor2_renamesub() throws Exception { @@ -175,7 +178,8 @@ public class AndroidPackageRenameParticipantTest extends RefactoringTestBase { "* customviews.xml - /testRefactor2_renamesub/res/layout-land/customviews.xml\n" + " @@ -15 +15\n" + " - <com.example.refactoringtest.subpackage.CustomView2\n" + - " + <my.pkg.name.subpackage.CustomView2"); + " + <my.pkg.name.subpackage.CustomView2", + true); } public void testRefactor2_renamesub_norefs() throws Exception { @@ -188,7 +192,8 @@ public class AndroidPackageRenameParticipantTest extends RefactoringTestBase { "CHANGES:\n" + "-------\n" + - "* Rename package 'com.example.refactoringtest' and subpackages to 'my.pkg.name'"); + "* Rename package 'com.example.refactoringtest' and subpackages to 'my.pkg.name'", + false); } @@ -199,9 +204,13 @@ public class AndroidPackageRenameParticipantTest extends RefactoringTestBase { boolean renameSubpackages, boolean updateReferences, @NonNull String newName, - @NonNull String expected) throws Exception { + @NonNull String expected, + boolean expectedAppPackageRenameWarning) throws Exception { IProject project = createProject(testData); - renamePackage(project, renameSubpackages, updateReferences, newName, expected); + String expectedWarnings = expectedAppPackageRenameWarning ? + EXPECTED_WARNINGS_TEMPLATE.replace("PROJECTNAME", project.getName()) : null; + renamePackage(project, renameSubpackages, updateReferences, newName, expected, + expectedWarnings); } protected void renamePackage( @@ -209,7 +218,8 @@ public class AndroidPackageRenameParticipantTest extends RefactoringTestBase { boolean renameSubpackages, boolean updateReferences, @NonNull String newName, - @NonNull String expected) throws Exception { + @NonNull String expected, + @NonNull String expectedWarnings) throws Exception { ManifestInfo info = ManifestInfo.get(project); String currentPackage = info.getPackage(); assertNotNull(currentPackage); @@ -222,7 +232,7 @@ public class AndroidPackageRenameParticipantTest extends RefactoringTestBase { assertNotNull(processor); RenameRefactoring refactoring = new RenameRefactoring(processor); - checkRefactoring(refactoring, expected); + checkRefactoring(refactoring, expected, expectedWarnings); } private static IPackageFragment getPackageFragment(IProject project, String pkg) @@ -240,4 +250,44 @@ public class AndroidPackageRenameParticipantTest extends RefactoringTestBase { } return pkgFragment; } + + private static String EXPECTED_WARNINGS_TEMPLATE = + "<INFO\n" + + "\t\n" + + "INFO: You are refactoring the same package as your application's package (specified in the manifest).\n" + + "\n" + + "Context: L/PROJECTNAME/AndroidManifest.xml\n" + + "code: none\n" + + "Data: null\n" + + "\t\n" + + "INFO: Note that this refactoring does NOT also update your application package.\n" + + "Context: L/PROJECTNAME/AndroidManifest.xml\n" + + "code: none\n" + + "Data: null\n" + + "\t\n" + + "INFO: The application package defines your application's identity.\n" + + "Context: L/PROJECTNAME/AndroidManifest.xml\n" + + "code: none\n" + + "Data: null\n" + + "\t\n" + + "INFO: If you change it, then it is considered to be a different application.\n" + + "Context: L/PROJECTNAME/AndroidManifest.xml\n" + + "code: none\n" + + "Data: null\n" + + "\t\n" + + "INFO: (Users of the previous version cannot update to the new version.)\n" + + "Context: L/PROJECTNAME/AndroidManifest.xml\n" + + "code: none\n" + + "Data: null\n" + + "\t\n" + + "INFO: The application package, and the package containing the code, can differ.\n" + + "Context: L/PROJECTNAME/AndroidManifest.xml\n" + + "code: none\n" + + "Data: null\n" + + "\t\n" + + "INFO: To really change application package, choose \"Android Tools\" > \"Rename Application Package.\" from the project context menu.\n" + + "Context: L/PROJECTNAME/AndroidManifest.xml\n" + + "code: none\n" + + "Data: null\n" + + ">"; } diff --git a/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/RefactoringTestBase.java b/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/RefactoringTestBase.java index 44fb522..8b7b817 100644 --- a/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/RefactoringTestBase.java +++ b/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/RefactoringTestBase.java @@ -51,12 +51,19 @@ import java.util.List; @SuppressWarnings({"javadoc","restriction"}) public abstract class RefactoringTestBase extends AdtProjectTest { - protected void checkRefactoring(Refactoring refactoring, String expected) - throws Exception { + protected void checkRefactoring(Refactoring refactoring, String expected) throws Exception { + checkRefactoring(refactoring, expected, null); + } + + protected void checkRefactoring(Refactoring refactoring, String expected, + @Nullable String expectedWarnings) throws Exception { RefactoringStatus status = refactoring.checkAllConditions(new NullProgressMonitor()); assertNotNull(status); + if (expectedWarnings == null) { + expectedWarnings = "<OK\n>"; + } + assertEquals(status.toString().trim(), expectedWarnings.trim()); if (!status.isOK()) { - assertEquals(status.toString(), expected); return; } assertTrue(status.toString(), status.isOK()); @@ -634,5 +641,10 @@ public abstract class RefactoringTestBase extends AdtProjectTest { "res/values/styles.xml", // file 3 SAMPLE_STYLES, + + // Just a gen file, should not be refactored + "bin/AndroidManifest.xml", + SAMPLE_MANIFEST, + }; } diff --git a/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/RenameResourceParticipantTest.java b/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/RenameResourceParticipantTest.java index 3a6859c..d3459b8 100644 --- a/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/RenameResourceParticipantTest.java +++ b/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/core/RenameResourceParticipantTest.java @@ -230,7 +230,8 @@ public class RenameResourceParticipantTest extends RefactoringTestBase { "\n" + "* Rename 'testRefactor7/res/layout-land/activity_main.xml' to 'newlayout.xml'\n" + "\n" + - "* Rename 'testRefactor7/res/layout/activity_main.xml' to 'newlayout.xml'"); + "* Rename 'testRefactor7/res/layout/activity_main.xml' to 'newlayout.xml'", + null); } public void testRefactor8() throws Exception { @@ -270,7 +271,8 @@ public class RenameResourceParticipantTest extends RefactoringTestBase { "* R.java - /testRefactor8/gen/com/example/refactoringtest/R.java\n" + " @@ -23 +23\n" + " - public static final int activity_main=0x7f030000;\n" + - " + public static final int newlauncher=0x7f030000;"); + " + public static final int newlauncher=0x7f030000;", + null); } public void testInvalidName() throws Exception { @@ -280,6 +282,7 @@ public class RenameResourceParticipantTest extends RefactoringTestBase { true /*updateReferences*/, "Newlauncher", + "", "<ERROR\n" + "\t\n" + "ERROR: File-based resource names must start with a lowercase letter.\n" + @@ -338,8 +341,18 @@ public class RenameResourceParticipantTest extends RefactoringTestBase { boolean updateReferences, @NonNull String newName, @NonNull String expected) throws Exception { + renameResource(testData, resource, updateReferences, newName, expected, null); + } + + protected void renameResource( + @NonNull Object[] testData, + @NonNull Object resource, + boolean updateReferences, + @NonNull String newName, + @NonNull String expected, + @NonNull String expectedWarnings) throws Exception { IProject project = createProject(testData); - renameResource(project, resource, updateReferences, newName, expected); + renameResource(project, resource, updateReferences, newName, expected, expectedWarnings); } protected void renameResource( @@ -347,7 +360,8 @@ public class RenameResourceParticipantTest extends RefactoringTestBase { @NonNull Object resource, boolean updateReferences, @NonNull String newName, - @NonNull String expected) throws Exception { + @NonNull String expected, + @NonNull String expectedWarnings) throws Exception { RenameProcessor processor = null; if (resource instanceof String) { String url = (String) resource; @@ -382,6 +396,6 @@ public class RenameResourceParticipantTest extends RefactoringTestBase { assertNotNull(processor); RenameRefactoring refactoring = new RenameRefactoring(processor); - checkRefactoring(refactoring, expected); + checkRefactoring(refactoring, expected, expectedWarnings); } }
\ No newline at end of file diff --git a/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/renamepackage/ApplicationPackageNameRefactoringTest.java b/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/renamepackage/ApplicationPackageNameRefactoringTest.java new file mode 100644 index 0000000..742aa31 --- /dev/null +++ b/eclipse/plugins/com.android.ide.eclipse.tests/src/com/android/ide/eclipse/adt/internal/refactorings/renamepackage/ApplicationPackageNameRefactoringTest.java @@ -0,0 +1,115 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Eclipse Public License, Version 1.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/org/documents/epl-v10.php + * + * 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 com.android.ide.eclipse.adt.internal.refactorings.renamepackage; + +import com.android.annotations.NonNull; +import com.android.ide.eclipse.adt.internal.editors.manifest.ManifestInfo; +import com.android.ide.eclipse.adt.internal.refactorings.core.RefactoringTestBase; + +import org.eclipse.core.resources.IProject; +import org.eclipse.jdt.core.dom.AST; +import org.eclipse.jdt.core.dom.Name; + +@SuppressWarnings("javadoc") +public class ApplicationPackageNameRefactoringTest extends RefactoringTestBase { + public void testRefactor1() throws Exception { + renamePackage( + TEST_PROJECT, + "my.pkg.name", + + "CHANGES:\n" + + "-------\n" + + "* Refactoring Application package name\n" + + "\n" + + " * MainActivity2.java - /testRefactor1/src/com/example/refactoringtest/MainActivity2.java\n" + + " @@ -7 +7\n" + + " + import my.pkg.name.R;\n" + + "\n" + + "\n" + + " * MainActivity.java - /testRefactor1/src/com/example/refactoringtest/MainActivity.java\n" + + " @@ -7 +7\n" + + " + import my.pkg.name.R;\n" + + "\n" + + "\n" + + " * Make Manifest edits - /testRefactor1/AndroidManifest.xml\n" + + " @@ -3 +3\n" + + " - package=\"com.example.refactoringtest\"\n" + + " + package=\"my.pkg.name\"\n" + + " @@ -25 +25\n" + + " - android:name=\".MainActivity2\"\n" + + " + android:name=\"com.example.refactoringtest.MainActivity2\""); + } + + public void testRefactor2() throws Exception { + // Tests custom view handling + renamePackage( + TEST_PROJECT2, + "my.pkg.name", + + "CHANGES:\n" + + "-------\n" + + "* Refactoring Application package name\n" + + "\n" + + " * MyFragment.java - /testRefactor2/src/com/example/refactoringtest/MyFragment.java\n" + + " @@ -3 +3\n" + + " + import my.pkg.name.R;\n" + + "\n" + + "\n" + + " * MainActivity.java - /testRefactor2/src/com/example/refactoringtest/MainActivity.java\n" + + " @@ -7 +7\n" + + " + import my.pkg.name.R;\n" + + "\n" + + "\n" + + " * CustomView1.java - /testRefactor2/src/com/example/refactoringtest/CustomView1.java\n" + + " @@ -5 +5\n" + + " + import my.pkg.name.R;\n" + + "\n" + + "\n" + + " * Make Manifest edits - /testRefactor2/AndroidManifest.xml\n" + + " @@ -3 +3\n" + + " - package=\"com.example.refactoringtest\"\n" + + " + package=\"my.pkg.name\"\n" + + " @@ -25 +25\n" + + " - android:name=\".MainActivity2\"\n" + + " + android:name=\"com.example.refactoringtest.MainActivity2\""); + } + + // ---- Test infrastructure ---- + + protected void renamePackage( + @NonNull Object[] testData, + @NonNull String newName, + @NonNull String expected) throws Exception { + IProject project = createProject(testData); + renamePackage(project, newName, expected); + } + + protected void renamePackage( + @NonNull IProject project, + @NonNull String newName, + @NonNull String expected) throws Exception { + ManifestInfo info = ManifestInfo.get(project); + String currentPackage = info.getPackage(); + assertNotNull(currentPackage); + + final AST astValidator = AST.newAST(AST.JLS3); + Name oldPackageName = astValidator.newName(currentPackage); + Name newPackageName = astValidator.newName(newName); + ApplicationPackageNameRefactoring refactoring = + new ApplicationPackageNameRefactoring(project, oldPackageName, newPackageName); + checkRefactoring(refactoring, expected); + } +} |