39612: Question Mark causes Eclipse Graphical Layout Editor to Freak Out

Handle string values starting with ? and @ even if they do not correspond
to actual theme or resource URLs.

Also fix the code which handles processing strings read from XML files;
apply unescaping rules (for unicode, newlines and tabs, removing quotes,
etc).

Also make the style warning include the full resource URI (it was only
logging the stripped URI).

Change-Id: I9b9a87ac4841faeacd1d94a43fa091702e60f4d8

6 files changed, 12 insertions, 147 deletions

diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/ContextPullParser.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/ContextPullParser.java
index 62821ae..c77c853 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/ContextPullParser.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/ContextPullParser.java
@@ -34,7 +34,7 @@ import static com.android.ide.eclipse.adt.internal.editors.layout.gle2.LayoutMet
 import com.android.SdkConstants;
 import com.android.ide.common.rendering.api.ILayoutPullParser;
 import com.android.ide.common.rendering.api.IProjectCallback;
-import com.android.ide.eclipse.adt.AdtUtils;
+import com.android.ide.common.resources.ValueResourceParser;
 import com.android.ide.eclipse.adt.internal.editors.layout.gle2.LayoutMetadata;
 import com.google.common.collect.Maps;
 
@@ -158,10 +158,8 @@ public class ContextPullParser extends KXmlParser implements ILayoutPullParser {
             return VALUE_FILL_PARENT;
         }
 
-        // Handle unicode escapes
-        if (value != null && value.indexOf('\\') != -1) {
-            value = AdtUtils.replaceUnicodeEscapes(value);
-        }
+        // Handle unicode escapes etc
+        value = ValueResourceParser.unescapeResourceString(value, false, false);
 
         return value;
     }
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/UiElementPullParser.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/UiElementPullParser.java
index c89a81b..e8e0d79 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/UiElementPullParser.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/UiElementPullParser.java
@@ -36,7 +36,7 @@ import static com.android.SdkConstants.VIEW_INCLUDE;
 
 import com.android.ide.common.rendering.api.ILayoutPullParser;
 import com.android.ide.common.rendering.api.ViewInfo;
-import com.android.ide.eclipse.adt.AdtUtils;
+import com.android.ide.common.resources.ValueResourceParser;
 import com.android.ide.eclipse.adt.internal.editors.layout.descriptors.LayoutDescriptors;
 import com.android.ide.eclipse.adt.internal.editors.layout.descriptors.ViewElementDescriptor;
 import com.android.ide.eclipse.adt.internal.editors.layout.gle2.FragmentMenu;
@@ -398,10 +398,8 @@ public class UiElementPullParser extends BasePullParser {
                     return VALUE_FILL_PARENT;
                 }
 
-                // Handle unicode escapes
-                if (value.indexOf('\\') != -1) {
-                    value = AdtUtils.replaceUnicodeEscapes(value);
-                }
+                // Handle unicode escapes etc
+                value = ValueResourceParser.unescapeResourceString(value, false, false);
 
                 return value;
             }
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoring.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoring.java
index e9d386e..7d0f926 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoring.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoring.java
@@ -16,12 +16,11 @@
 
 package com.android.ide.eclipse.adt.internal.refactorings.extractstring;
 
-import static com.android.SdkConstants.AMP_ENTITY;
-import static com.android.SdkConstants.LT_ENTITY;
 import static com.android.SdkConstants.QUOT_ENTITY;
 import static com.android.SdkConstants.STRING_PREFIX;
 
 import com.android.SdkConstants;
+import com.android.ide.common.resources.ValueResourceParser;
 import com.android.ide.common.xml.ManifestData;
 import com.android.ide.eclipse.adt.AdtConstants;
 import com.android.ide.eclipse.adt.internal.editors.AndroidXmlEditor;
@@ -1218,7 +1217,7 @@ public class ExtractStringRefactoring extends Refactoring {
         IStructuredModel smodel = null;
 
         // Single and double quotes must be escaped in the <string>value</string> declaration
-        tokenString = escapeString(tokenString);
+        tokenString = ValueResourceParser.escapeResourceString(tokenString);
 
         try {
             IStructuredDocument sdoc = null;
@@ -1450,79 +1449,6 @@ public class ExtractStringRefactoring extends Refactoring {
     }
 
     /**
-     * Escape a string value to be placed in a string resource file such that it complies with
-     * the escaping rules described here:
-     *   http://developer.android.com/guide/topics/resources/string-resource.html
-     * More examples of the escaping rules can be found here:
-     *   http://androidcookbook.com/Recipe.seam?recipeId=2219&recipeFrom=ViewTOC
-     * This method assumes that the String is not escaped already.
-     *
-     * Rules:
-     * <ul>
-     * <li>Double quotes are needed if string starts or ends with at least one space.
-     * <li>{@code @, ?} at beginning of string have to be escaped with a backslash.
-     * <li>{@code ', ", \} have to be escaped with a backslash.
-     * <li>{@code <, >, &} have to be replaced by their predefined xml entity.
-     * <li>{@code \n, \t} have to be replaced by a backslash and the appropriate character.
-     * </ul>
-     * @param s the string to be escaped
-     * @return the escaped string as it would appear in the XML text in a values file
-     */
-    public static String escapeString(String s) {
-        int n = s.length();
-        if (n == 0) {
-            return "";
-        }
-
-        StringBuilder sb = new StringBuilder(s.length() * 2);
-        boolean hasSpace = s.charAt(0) == ' ' || s.charAt(n - 1) == ' ';
-
-        if (hasSpace) {
-            sb.append('"');
-        } else if (s.charAt(0) == '@' || s.charAt(0) == '?') {
-            sb.append('\\');
-        }
-
-        for (int i = 0; i < n; ++i) {
-            char c = s.charAt(i);
-            switch (c) {
-                case '\'':
-                    if (!hasSpace) {
-                        sb.append('\\');
-                    }
-                    sb.append(c);
-                    break;
-                case '"':
-                case '\\':
-                    sb.append('\\');
-                    sb.append(c);
-                    break;
-                case '<':
-                    sb.append(LT_ENTITY);
-                    break;
-                case '&':
-                    sb.append(AMP_ENTITY);
-                    break;
-                case '\n':
-                    sb.append("\\n"); //$NON-NLS-1$
-                    break;
-                case '\t':
-                    sb.append("\\t"); //$NON-NLS-1$
-                    break;
-                default:
-                    sb.append(c);
-                    break;
-            }
-        }
-
-        if (hasSpace) {
-            sb.append('"');
-        }
-
-        return sb.toString();
-    }
-
-    /**
      * Computes the changes to be made to the source Android XML file and
      * returns a list of {@link Change}.
      * <p/>
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/newproject/NewProjectCreator.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/newproject/NewProjectCreator.java
index 245d84e..19a7101 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/newproject/NewProjectCreator.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/newproject/NewProjectCreator.java
@@ -23,6 +23,7 @@ import static org.eclipse.core.resources.IResource.DEPTH_ZERO;
 import com.android.SdkConstants;
 import com.android.annotations.NonNull;
 import com.android.annotations.Nullable;
+import com.android.ide.common.resources.ValueResourceParser;
 import com.android.ide.common.xml.ManifestData;
 import com.android.ide.eclipse.adt.AdtConstants;
 import com.android.ide.eclipse.adt.AdtPlugin;
@@ -34,7 +35,6 @@ import com.android.ide.eclipse.adt.internal.preferences.AdtPrefs;
 import com.android.ide.eclipse.adt.internal.project.AndroidNature;
 import com.android.ide.eclipse.adt.internal.project.BaseProjectHelper;
 import com.android.ide.eclipse.adt.internal.project.ProjectHelper;
-import com.android.ide.eclipse.adt.internal.refactorings.extractstring.ExtractStringRefactoring;
 import com.android.ide.eclipse.adt.internal.sdk.ProjectState;
 import com.android.ide.eclipse.adt.internal.sdk.Sdk;
 import com.android.ide.eclipse.adt.internal.wizards.newproject.NewProjectWizardState.Mode;
@@ -1058,7 +1058,7 @@ public class NewProjectCreator  {
                 String value = strings.get(key);
 
                 // Escape values if necessary
-                value = ExtractStringRefactoring.escapeString(value);
+                value = ValueResourceParser.escapeResourceString(value);
 
                 // place them in the template
                 String stringDef = stringTemplate.replace(PARAM_STRING_NAME, key);
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/templates/FmEscapeXmlStringMethod.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/templates/FmEscapeXmlStringMethod.java
index 7e5866e..ffcfa3e 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/templates/FmEscapeXmlStringMethod.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/wizards/templates/FmEscapeXmlStringMethod.java
@@ -15,7 +15,7 @@
  */
 package com.android.ide.eclipse.adt.internal.wizards.templates;
 
-import com.android.ide.eclipse.adt.internal.refactorings.extractstring.ExtractStringRefactoring;
+import com.android.ide.common.resources.ValueResourceParser;
 
 import freemarker.template.SimpleScalar;
 import freemarker.template.TemplateMethodModel;
@@ -38,6 +38,6 @@ public class FmEscapeXmlStringMethod implements TemplateMethodModel {
             throw new TemplateModelException("Wrong arguments");
         }
         String string = args.get(0).toString();
-        return new SimpleScalar(ExtractStringRefactoring.escapeString(string));
+        return new SimpleScalar(ValueResourceParser.escapeResourceString(string));
     }
 }
\ No newline at end of file
diff --git a/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoringTest.java b/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoringTest.java
deleted file mode 100644
index f7d1d4b..0000000
--- a/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoringTest.java
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * Copyright (C) 2011 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.extractstring;
-
-import junit.framework.TestCase;
-
-public class ExtractStringRefactoringTest extends TestCase {
-
-    public void testEscapeStringShouldEscapeXmlSpecialCharacters() throws Exception {
-        assertEquals("&lt;", escape("<")); //$NON-NLS-1$ //$NON-NLS-2$
-        assertEquals("&amp;", escape("&")); //$NON-NLS-1$ //$NON-NLS-2$
-    }
-
-    public void testEscapeStringShouldEscapeQuotes() throws Exception {
-        assertEquals("\\'", escape("'")); //$NON-NLS-1$ //$NON-NLS-2$
-        assertEquals("\\\"", escape("\"")); //$NON-NLS-1$ //$NON-NLS-2$
-        assertEquals("\" ' \"", escape(" ' ")); //$NON-NLS-1$ //$NON-NLS-2$
-    }
-
-    public void testEscapeStringShouldPreserveWhitespace() throws Exception {
-        assertEquals("\"at end  \"", escape("at end  ")); //$NON-NLS-1$ //$NON-NLS-2$
-        assertEquals("\"  at begin\"", escape("  at begin")); //$NON-NLS-1$ //$NON-NLS-2$
-    }
-
-    public void testEscapeStringShouldEscapeAtSignAndQuestionMarkOnlyAtBeginning()
-            throws Exception {
-        assertEquals("\\@text", escape("@text")); //$NON-NLS-1$ //$NON-NLS-2$
-        assertEquals("a@text", escape("a@text")); //$NON-NLS-1$ //$NON-NLS-2$
-        assertEquals("\\?text", escape("?text")); //$NON-NLS-1$ //$NON-NLS-2$
-        assertEquals("a?text", escape("a?text")); //$NON-NLS-1$ //$NON-NLS-2$
-        assertEquals("\" ?text\"", escape(" ?text")); //$NON-NLS-1$ //$NON-NLS-2$
-    }
-
-    public void testEscapeStringShouldEscapeJavaEscapeSequences() throws Exception {
-        assertEquals("\\n", escape("\n")); //$NON-NLS-1$ //$NON-NLS-2$
-        assertEquals("\\t", escape("\t")); //$NON-NLS-1$ //$NON-NLS-2$
-        assertEquals("\\\\", escape("\\")); //$NON-NLS-1$ //$NON-NLS-2$
-    }
-
-    private static String escape(String unescaped) {
-        return ExtractStringRefactoring.escapeString(unescaped);
-    }
-}