Merge "Add "Extra Text" lint detector."

6 files changed, 247 insertions, 3 deletions

diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/EclipseLintClient.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/EclipseLintClient.java
index 310d689..d105867 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/EclipseLintClient.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/lint/EclipseLintClient.java
@@ -587,6 +587,22 @@ public class EclipseLintClient extends LintClient implements IDomParser {
                 int column = -1;
                 int offset = mRegion.getStartOffset();
 
+                if (mRegion instanceof org.w3c.dom.Text && mDocument != null) {
+                    // For text nodes, skip whitespace prefix, if any
+                    for (int i = offset;
+                            i < mRegion.getEndOffset() && i < mDocument.getLength(); i++) {
+                        try {
+                            char c = mDocument.getChar(i);
+                            if (!Character.isWhitespace(c)) {
+                                offset = i;
+                                break;
+                            }
+                        } catch (BadLocationException e) {
+                            break;
+                        }
+                    }
+                }
+
                 if (mDocument != null && offset < mDocument.getLength()) {
                     line = mDocument.getLineOfOffset(offset);
                     column = -1;
diff --git a/lint/cli/src/com/android/tools/lint/PositionXmlParser.java b/lint/cli/src/com/android/tools/lint/PositionXmlParser.java
index 7efdd57..b4af282 100644
--- a/lint/cli/src/com/android/tools/lint/PositionXmlParser.java
+++ b/lint/cli/src/com/android/tools/lint/PositionXmlParser.java
@@ -28,6 +28,7 @@ import org.w3c.dom.Attr;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
+import org.w3c.dom.Text;
 import org.xml.sax.Attributes;
 import org.xml.sax.InputSource;
 import org.xml.sax.Locator;
@@ -139,12 +140,13 @@ public class PositionXmlParser implements IDomParser {
                         if (t == '\n') {
                             line++;
                             column = 0;
+                        } else {
+                            column++;
                         }
-                        column++;
                     }
 
                     OffsetPosition attributePosition = new OffsetPosition(line, column, index);
-                    // Also set end range for retrieval in getEndPosition
+                    // Also set end range for retrieval in getLocation
                     attributePosition.next = new OffsetPosition(line, column, matcher.end());
                     return attributePosition;
                 } else {
@@ -152,6 +154,81 @@ public class PositionXmlParser implements IDomParser {
                     return pos;
                 }
             }
+        } else if (node instanceof Text) {
+            // Position of parent element, if any
+            OffsetPosition pos = null;
+            if (node.getPreviousSibling() != null) {
+                pos = (OffsetPosition) node.getPreviousSibling().getUserData(POS_KEY);
+            }
+            if (pos == null) {
+                pos = (OffsetPosition) node.getParentNode().getUserData(POS_KEY);
+            }
+            if (pos != null) {
+                // Attempt to point forward to the actual text node
+                int startOffset = pos.getOffset();
+                int endOffset = pos.next.getOffset();
+                int line = pos.getLine();
+                int column = pos.getColumn();
+
+                // Find attribute in the text
+                String contents = (String) node.getOwnerDocument().getUserData(CONTENT_KEY);
+                if (contents == null || contents.length() < endOffset) {
+                    return null;
+                }
+
+                boolean inAttribute = false;
+                for (int offset = startOffset; offset <= endOffset; offset++) {
+                    char c = contents.charAt(offset);
+                    if (c == '>' && !inAttribute) {
+                        // Found the end of the element open tag: this is where the
+                        // text begins.
+
+                        // Skip >
+                        offset++;
+
+                        // Skip text whitespace prefix, if the text node contains non-whitespace
+                        // characters
+                        String text = node.getNodeValue();
+                        int textIndex = 0;
+                        int textLength = text.length();
+                        int newLine = line;
+                        int newColumn = column;
+                        for (; textIndex < text.length(); textIndex++) {
+                            char t = text.charAt(textIndex);
+                            if (t == '\n') {
+                                newLine++;
+                                newColumn = 0;
+                            } else {
+                                newColumn++;
+                            }
+                            if (!Character.isWhitespace(t)) {
+                                break;
+                            }
+                        }
+                        if (textIndex == textLength) {
+                            textIndex = 0; // Whitespace node
+                        } else {
+                            line = newLine;
+                            column = newColumn;
+                        }
+
+                        OffsetPosition attributePosition = new OffsetPosition(line, column,
+                                offset);
+                        // Also set end range for retrieval in getLocation
+                        attributePosition.next = new OffsetPosition(line, column,
+                                offset + textLength);
+                        return attributePosition;
+                    } else if (c == '"') {
+                        inAttribute = !inAttribute;
+                    } else if (c == '\n') {
+                        line++;
+                        column = -1; // pre-subtract column added below
+                    }
+                    column++;
+                }
+
+                return pos;
+            }
         }
 
         return (OffsetPosition) node.getUserData(POS_KEY);
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
index 689ad89..5d31523 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/BuiltinIssueRegistry.java
@@ -51,7 +51,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
     private static final List<Issue> sIssues;
 
     static {
-        List<Issue> issues = new ArrayList<Issue>();
+        List<Issue> issues = new ArrayList<Issue>(60);
 
         issues.add(AccessibilityDetector.ISSUE);
         issues.add(MathDetector.ISSUE);
@@ -86,6 +86,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(TextFieldDetector.ISSUE);
         issues.add(UnusedResourceDetector.ISSUE);
         issues.add(UnusedResourceDetector.ISSUE_IDS);
+        issues.add(ExtraTextDetector.ISSUE);
         issues.add(ArraySizeDetector.INCONSISTENT);
         issues.add(ManifestOrderDetector.ISSUE);
         issues.add(SecurityDetector.EXPORTED_SERVICE);
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ExtraTextDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ExtraTextDetector.java
new file mode 100644
index 0000000..cf5d2e6
--- /dev/null
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ExtraTextDetector.java
@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2011 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.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.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.tools.lint.checks;
+
+import com.android.resources.ResourceFolderType;
+import com.android.tools.lint.detector.api.Category;
+import com.android.tools.lint.detector.api.Issue;
+import com.android.tools.lint.detector.api.ResourceXmlDetector;
+import com.android.tools.lint.detector.api.Scope;
+import com.android.tools.lint.detector.api.Severity;
+import com.android.tools.lint.detector.api.Speed;
+import com.android.tools.lint.detector.api.XmlContext;
+
+import org.w3c.dom.Document;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+
+/**
+ * Check which looks for invalid resources. Aapt already performs some validation,
+ * such as making sure that resource references point to resources that exist, but this
+ * detector looks for additional issues.
+ */
+public class ExtraTextDetector extends ResourceXmlDetector {
+    private boolean mFoundText;
+
+    /** The main issue discovered by this detector */
+    public static final Issue ISSUE = Issue.create(
+            "ExtraText", //$NON-NLS-1$
+            "Looks for extraneous text in layout files",
+            "Layout resource files should only contain elements and attributes. Any XML " +
+            "text content found in the file is likely accidental (and potentially " +
+            "dangerous if the text resembles XML and the developer believes the text " +
+            "to be functional)",
+            Category.CORRECTNESS,
+            3,
+            Severity.WARNING,
+            ExtraTextDetector.class,
+            Scope.RESOURCE_FILE_SCOPE);
+
+    /** Constructs a new detector */
+    public ExtraTextDetector() {
+    }
+
+    @Override
+    public boolean appliesTo(ResourceFolderType folderType) {
+        return folderType == ResourceFolderType.LAYOUT
+                || folderType == ResourceFolderType.MENU
+                || folderType == ResourceFolderType.ANIMATOR
+                || folderType == ResourceFolderType.ANIMATOR
+                || folderType == ResourceFolderType.DRAWABLE
+                || folderType == ResourceFolderType.COLOR;
+    }
+
+    @Override
+    public Speed getSpeed() {
+        return Speed.FAST;
+    }
+
+    @Override
+    public void visitDocument(XmlContext context, Document document) {
+        mFoundText = false;
+        visitNode(context, document);
+    }
+
+    private void visitNode(XmlContext context, Node node) {
+        short nodeType = node.getNodeType();
+        if (nodeType == Node.TEXT_NODE && !mFoundText) {
+            String text = node.getNodeValue();
+            for (int i = 0, n = text.length(); i < n; i++) {
+                char c = text.charAt(i);
+                if (!Character.isWhitespace(c)) {
+                    String snippet = text.trim();
+                    int maxLength = 100;
+                    if (snippet.length() > maxLength) {
+                        snippet = snippet.substring(0, maxLength) + "...";
+                    }
+                    context.report(ISSUE, context.getLocation(node),
+                            String.format("Unexpected text found in layout file: \"%1$s\"",
+                                    snippet), null);
+                    mFoundText = true;
+                    break;
+                }
+            }
+        }
+
+        // Visit children
+        NodeList childNodes = node.getChildNodes();
+        for (int i = 0, n = childNodes.getLength(); i < n; i++) {
+            Node child = childNodes.item(i);
+            visitNode(context, child);
+        }
+    }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ExtraTextDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ExtraTextDetectorTest.java
new file mode 100644
index 0000000..35c38ac
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ExtraTextDetectorTest.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2011 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.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.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.tools.lint.checks;
+
+import com.android.tools.lint.detector.api.Detector;
+
+@SuppressWarnings("javadoc")
+public class ExtraTextDetectorTest extends AbstractCheckTest {
+    @Override
+    protected Detector getDetector() {
+        return new ExtraTextDetector();
+    }
+
+    public void testBroken() throws Exception {
+        assertEquals(
+                "broken.xml:5: Warning: Unexpected text found in layout file: \"ImageButton " +
+                "android:id=\"@+id/android_logo2\" android:layout_width=\"wrap_content\"" +
+                " android:layout_heigh...\"",
+                lintProject("res/layout/broken.xml"));
+    }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/broken.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/broken.xml
new file mode 100644
index 0000000..5dd9d2d
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/broken.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" android:id="@+id/newlinear" android:orientation="vertical" android:layout_width="match_parent" android:layout_height="match_parent">
+    <Button android:text="Button" android:id="@+id/button1" android:layout_width="wrap_content" android:layout_height="wrap_content"></Button>
+    <ImageView android:id="@+id/android_logo" android:layout_width="wrap_content" android:layout_height="wrap_content" android:src="@drawable/android_button" android:focusable="false" android:clickable="false" android:layout_weight="1.0" />
+    ImageButton android:id="@+id/android_logo2" android:layout_width="wrap_content" android:layout_height="wrap_content" android:src="@drawable/android_button" android:focusable="false" android:clickable="false" android:layout_weight="1.0" />
+    <Button android:text="Button" android:id="@+id/button2" android:layout_width="wrap_content" android:layout_height="wrap_content"></Button>
+    <Button android:id="@+android:id/summary" android:contentDescription="@string/label" />
+</LinearLayout>