Tweak resource name validator, move code around

Incorporate some feedback from
https://review.source.android.com//#change,21341
Disallow uppercase chars in file-based resource names

Change-Id: I078b30be64e1aeb984f26d5c8b2976aa0083fcf4

8 files changed, 89 insertions, 34 deletions

diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/ExtractIncludeRefactoring.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/ExtractIncludeRefactoring.java
index 9cf307d..21779dc 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/ExtractIncludeRefactoring.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/ExtractIncludeRefactoring.java
@@ -38,7 +38,7 @@ import com.android.ide.eclipse.adt.internal.editors.layout.descriptors.LayoutDes
 import com.android.ide.eclipse.adt.internal.editors.layout.gle2.CanvasViewInfo;
 import com.android.ide.eclipse.adt.internal.editors.layout.gle2.DomUtilities;
 import com.android.ide.eclipse.adt.internal.editors.layout.uimodel.UiViewElementNode;
-import com.android.ide.eclipse.adt.internal.wizards.newxmlfile.ResourceNameValidator;
+import com.android.ide.eclipse.adt.internal.resources.ResourceNameValidator;
 import com.android.util.Pair;
 
 import org.eclipse.core.resources.IContainer;
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/ExtractIncludeWizard.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/ExtractIncludeWizard.java
index 0e3314e..a0d6c5a 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/ExtractIncludeWizard.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/ExtractIncludeWizard.java
@@ -16,7 +16,7 @@
 
 package com.android.ide.eclipse.adt.internal.editors.layout.refactoring;
 
-import com.android.ide.eclipse.adt.internal.wizards.newxmlfile.ResourceNameValidator;
+import com.android.ide.eclipse.adt.internal.resources.ResourceNameValidator;
 import com.android.resources.ResourceType;
 
 import org.eclipse.core.resources.IProject;
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/WrapInWizard.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/WrapInWizard.java
index ee746c3..8119392 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/WrapInWizard.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/WrapInWizard.java
@@ -20,9 +20,9 @@ import static com.android.ide.common.layout.LayoutConstants.FQCN_LINEAR_LAYOUT;
 
 import com.android.ide.eclipse.adt.internal.editors.layout.descriptors.ViewElementDescriptor;
 import com.android.ide.eclipse.adt.internal.editors.layout.gre.ViewMetadataRepository;
+import com.android.ide.eclipse.adt.internal.resources.ResourceNameValidator;
 import com.android.ide.eclipse.adt.internal.sdk.AndroidTargetData;
 import com.android.ide.eclipse.adt.internal.sdk.Sdk;
-import com.android.ide.eclipse.adt.internal.wizards.newxmlfile.ResourceNameValidator;
 import com.android.resources.ResourceType;
 import com.android.sdklib.IAndroidTarget;
 import com.android.util.Pair;
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/xml/Hyperlinks.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/xml/Hyperlinks.java
index 5e662bf..3bfe469 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/xml/Hyperlinks.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/xml/Hyperlinks.java
@@ -41,8 +41,8 @@ import com.android.ide.eclipse.adt.internal.editors.layout.LayoutEditor;
 import com.android.ide.eclipse.adt.internal.editors.layout.gle2.GraphicalEditorPart;
 import com.android.ide.eclipse.adt.internal.editors.manifest.ManifestEditor;
 import com.android.ide.eclipse.adt.internal.editors.resources.descriptors.ResourcesDescriptors;
+import com.android.ide.eclipse.adt.internal.resources.ResourceNameValidator;
 import com.android.ide.eclipse.adt.internal.resources.configurations.FolderConfiguration;
-import com.android.ide.eclipse.adt.internal.resources.manager.FolderTypeRelationship;
 import com.android.ide.eclipse.adt.internal.resources.manager.ProjectResources;
 import com.android.ide.eclipse.adt.internal.resources.manager.ResourceFile;
 import com.android.ide.eclipse.adt.internal.resources.manager.ResourceFolder;
@@ -488,24 +488,6 @@ public class Hyperlinks {
     }
 
     /**
-     * Is this a resource that can be defined in any file within the "values" folder?
-     *
-     * @param type the resource type to check
-     * @return true if the given resource type can be represented as a value under the
-     *         values/ folder
-     */
-    public static boolean isValueResource(ResourceType type) {
-        ResourceFolderType[] folderTypes = FolderTypeRelationship.getRelatedFolders(type);
-        for (ResourceFolderType folderType : folderTypes) {
-            if (folderType == ResourceFolderType.VALUES) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
-    /**
      * Returns the XML tag containing an element description for value items of the given
      * resource type
      *
@@ -1181,8 +1163,7 @@ public class Hyperlinks {
                 });
 
                 // Is this something found in a values/ folder?
-                boolean valueResource = isValueResource(type);
-                //boolean fileResource = isFileResource(type);
+                boolean valueResource = ResourceNameValidator.isValueBasedResourceType(type);
 
                 for (ResourceFile file : matches) {
                     String folderName = file.getFolder().getFolder().getName();
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/newxmlfile/ResourceNameValidator.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/resources/ResourceNameValidator.java
index a6c59e6..2ef0beb 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/newxmlfile/ResourceNameValidator.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/resources/ResourceNameValidator.java
@@ -14,13 +14,13 @@
  * limitations under the License.
  */
 
-package com.android.ide.eclipse.adt.internal.wizards.newxmlfile;
+package com.android.ide.eclipse.adt.internal.resources;
 
 import static com.android.ide.eclipse.adt.AndroidConstants.DOT_XML;
 
 import com.android.ide.eclipse.adt.AdtPlugin;
 import com.android.ide.eclipse.adt.AndroidConstants;
-import com.android.ide.eclipse.adt.internal.editors.xml.Hyperlinks;
+import com.android.ide.eclipse.adt.internal.resources.manager.FolderTypeRelationship;
 import com.android.ide.eclipse.adt.internal.resources.manager.ProjectResourceItem;
 import com.android.ide.eclipse.adt.internal.resources.manager.ProjectResources;
 import com.android.ide.eclipse.adt.internal.resources.manager.ResourceFolderType;
@@ -91,7 +91,7 @@ public class ResourceNameValidator implements IInputValidator {
                 // "%s: Invalid file name: must contain only [a-z0-9_.]","
                 for (int i = 0, n = newText.length(); i < n; i++) {
                     char c = newText.charAt(i);
-                    if (!(c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '_') {
+                    if (!((c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '_')) {
                         return String.format(
                           "File-based resource names must contain only lowercase a-z, 0-9, or _.");
                     }
@@ -141,7 +141,7 @@ public class ResourceNameValidator implements IInputValidator {
      */
     public static ResourceNameValidator create(boolean allowXmlExtension, Set<String> existing,
             ResourceType type) {
-        boolean isFileType = !Hyperlinks.isValueResource(type);
+        boolean isFileType = isFileBasedResourceType(type);
         return new ResourceNameValidator(allowXmlExtension, existing, isFileType);
     }
 
@@ -164,7 +164,54 @@ public class ResourceNameValidator implements IInputValidator {
             existing.add(resource.getName());
         }
 
-        boolean isFileType = !Hyperlinks.isValueResource(type);
+        boolean isFileType = isFileBasedResourceType(type);
         return new ResourceNameValidator(allowXmlExtension, existing, isFileType);
     }
+
+    /**
+     * Is this a resource that is defined in a file named by the resource plus the XML
+     * extension?
+     * <p>
+     * Some resource types can be defined <b>both</b> as a separate XML file as well as
+     * defined within a value XML file along with other properties. This method will
+     * return true for these resource types as well. In other words, a ResourceType can
+     * return true for both {@link #isValueBasedResourceType} and
+     * {@link #isFileBasedResourceType}.
+     *
+     * @param type the resource type to check
+     * @return true if the given resource type is stored in a file named by the resource
+     */
+    public static boolean isFileBasedResourceType(ResourceType type) {
+        ResourceFolderType[] folderTypes = FolderTypeRelationship.getRelatedFolders(type);
+        for (ResourceFolderType folderType : folderTypes) {
+            if (folderType != ResourceFolderType.VALUES) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Is this a resource that can be defined in any file within the "values" folder?
+     * <p>
+     * Some resource types can be defined <b>both</b> as a separate XML file as well
+     * as defined within a value XML file. This method will return true for these types
+     * as well. In other words, a ResourceType can return true for both
+     * {@link #isValueBasedResourceType} and {@link #isFileBasedResourceType}.
+     *
+     * @param type the resource type to check
+     * @return true if the given resource type can be represented as a value under the
+     *         values/ folder
+     */
+    public static boolean isValueBasedResourceType(ResourceType type) {
+        ResourceFolderType[] folderTypes = FolderTypeRelationship.getRelatedFolders(type);
+        for (ResourceFolderType folderType : folderTypes) {
+            if (folderType == ResourceFolderType.VALUES) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/ui/ResourceChooser.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/ui/ResourceChooser.java
index 78853f5..e27ea5b 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/ui/ResourceChooser.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/ui/ResourceChooser.java
@@ -31,9 +31,9 @@ import com.android.ide.eclipse.adt.internal.refactorings.extractstring.ExtractSt
 import com.android.ide.eclipse.adt.internal.resources.IResourceRepository;
 import com.android.ide.eclipse.adt.internal.resources.ResourceHelper;
 import com.android.ide.eclipse.adt.internal.resources.ResourceItem;
+import com.android.ide.eclipse.adt.internal.resources.ResourceNameValidator;
 import com.android.ide.eclipse.adt.internal.sdk.AndroidTargetData;
 import com.android.ide.eclipse.adt.internal.sdk.Sdk;
-import com.android.ide.eclipse.adt.internal.wizards.newxmlfile.ResourceNameValidator;
 import com.android.resources.ResourceType;
 
 import org.eclipse.core.resources.IFile;
@@ -208,8 +208,8 @@ public class ResourceChooser extends AbstractElementListSelectionDialog {
         String title = String.format("New %1$s...", mResourceType.getDisplayName());
         mNewButton.setText(title);
 
-        // We only support adding new strings right now
-        mNewButton.setEnabled(Hyperlinks.isValueResource(mResourceType));
+        // We only support adding new values right now
+        mNewButton.setEnabled(ResourceNameValidator.isValueBasedResourceType(mResourceType));
 
         mNewButton.addSelectionListener(new SelectionAdapter() {
             @Override
@@ -219,7 +219,7 @@ public class ResourceChooser extends AbstractElementListSelectionDialog {
                 if (mResourceType == ResourceType.STRING) {
                     createNewString();
                 } else {
-                    assert Hyperlinks.isValueResource(mResourceType);
+                    assert ResourceNameValidator.isValueBasedResourceType(mResourceType);
                     String newName = createNewValue(mResourceType);
                     if (newName != null) {
                         // Recompute the "current resource" to select the new id
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/newxmlfile/NewXmlFileCreationPage.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/newxmlfile/NewXmlFileCreationPage.java
index 9021951..aea01b9 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/newxmlfile/NewXmlFileCreationPage.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/newxmlfile/NewXmlFileCreationPage.java
@@ -27,6 +27,7 @@ import com.android.ide.eclipse.adt.internal.editors.resources.descriptors.Resour
 import com.android.ide.eclipse.adt.internal.project.BaseProjectHelper;
 import com.android.ide.eclipse.adt.internal.project.ProjectChooserHelper;
 import com.android.ide.eclipse.adt.internal.project.BaseProjectHelper.IProjectFilter;
+import com.android.ide.eclipse.adt.internal.resources.ResourceNameValidator;
 import com.android.ide.eclipse.adt.internal.resources.configurations.FolderConfiguration;
 import com.android.ide.eclipse.adt.internal.resources.configurations.ResourceQualifier;
 import com.android.ide.eclipse.adt.internal.resources.manager.ResourceFolderType;
diff --git a/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/wizards/newxmlfile/ResourceNameValidatorTest.java b/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/resources/ResourceNameValidatorTest.java
index 4c4b292..9c2a0c7 100644
--- a/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/wizards/newxmlfile/ResourceNameValidatorTest.java
+++ b/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/resources/ResourceNameValidatorTest.java
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package com.android.ide.eclipse.adt.internal.wizards.newxmlfile;
+package com.android.ide.eclipse.adt.internal.resources;
 
 import com.android.ide.eclipse.adt.internal.resources.manager.ResourceFolderType;
 import com.android.resources.ResourceType;
@@ -45,6 +45,32 @@ public class ResourceNameValidatorTest extends TestCase {
         // Only lowercase chars allowed in file-based resource names
         assertTrue(ResourceNameValidator.create(true, ResourceFolderType.LAYOUT)
                 .isValid("Foo123_$") != null);
+        assertTrue(ResourceNameValidator.create(true, ResourceFolderType.LAYOUT)
+                .isValid("foo123_") == null);
+    }
+
+    public void testIsFileBasedResourceType() throws Exception {
+        assertTrue(ResourceNameValidator.isFileBasedResourceType(ResourceType.ANIMATOR));
+        assertTrue(ResourceNameValidator.isFileBasedResourceType(ResourceType.LAYOUT));
+
+        assertFalse(ResourceNameValidator.isFileBasedResourceType(ResourceType.STRING));
+        assertFalse(ResourceNameValidator.isFileBasedResourceType(ResourceType.DIMEN));
+        assertFalse(ResourceNameValidator.isFileBasedResourceType(ResourceType.ID));
+
+        // Both:
+        assertTrue(ResourceNameValidator.isFileBasedResourceType(ResourceType.DRAWABLE));
+        assertTrue(ResourceNameValidator.isFileBasedResourceType(ResourceType.COLOR));
+    }
+
+    public void testIsValueBasedResourceType() throws Exception {
+        assertTrue(ResourceNameValidator.isValueBasedResourceType(ResourceType.STRING));
+        assertTrue(ResourceNameValidator.isValueBasedResourceType(ResourceType.DIMEN));
+        assertTrue(ResourceNameValidator.isValueBasedResourceType(ResourceType.ID));
+
+        assertFalse(ResourceNameValidator.isValueBasedResourceType(ResourceType.LAYOUT));
 
+        // These can be both:
+        assertTrue(ResourceNameValidator.isValueBasedResourceType(ResourceType.DRAWABLE));
+        assertTrue(ResourceNameValidator.isValueBasedResourceType(ResourceType.COLOR));
     }
 }