Fix Useless layout detector, and add HTML summary

First, fix the logic in the useless layout detector to properly
consider the background attributes.

Second, add a summary section at the top of the HTML report with
hyperlinks to each issue section.

Change-Id: I7e9e2d81babf977751fc3ca64e61fcc8434a0792

4 files changed, 138 insertions, 40 deletions

diff --git a/lint/cli/src/com/android/tools/lint/HtmlReporter.java b/lint/cli/src/com/android/tools/lint/HtmlReporter.java
index 86b9d71..eb794dd 100644
--- a/lint/cli/src/com/android/tools/lint/HtmlReporter.java
+++ b/lint/cli/src/com/android/tools/lint/HtmlReporter.java
@@ -118,16 +118,31 @@ class HtmlReporter extends Reporter {
                 currentList.add(warning);
             }
 
-            mWriter.write(String.format("Check performed at %1$s.", new Date().toString()));
-            mWriter.write("<br/>"); //$NON-NLS-1$
-            mWriter.write(String.format("%1$d errors and %2$d warnings found.", errorCount,
-                    warningCount));
-            mWriter.write("<br/>"); //$NON-NLS-1$
+            mWriter.write(String.format("Check performed at %1$s.",
+                    new Date().toString()));
+            mWriter.write("<br/>");                                       //$NON-NLS-1$
+            mWriter.write(String.format("%1$d errors and %2$d warnings found:",
+                    errorCount, warningCount));
+            mWriter.write("<br/>");                                       //$NON-NLS-1$
+
+            // Write issue id summary
+            mWriter.write("<ul>\n");                                     //$NON-NLS-1$
+            for (List<Warning> warnings : related) {
+                mWriter.write("<li> <a href=\"#"                         //$NON-NLS-1$
+                        + warnings.get(0).issue.getId()
+                        +"\">");                                         //$NON-NLS-1$
+                mWriter.write(String.format("%1$3d %2$s",                //$NON-NLS-1$
+                        warnings.size(), warnings.get(0).issue.getId()));
+                mWriter.write("</a>\n");                                 //$NON-NLS-1$
+            }
+            mWriter.write("</ul>\n");                                    //$NON-NLS-1$
+            mWriter.write("<br/>");                                      //$NON-NLS-1$
 
             for (List<Warning> warnings : related) {
                 Warning first = warnings.get(0);
                 Issue issue = first.issue;
 
+                mWriter.write("<a name=\"" + issue.getId() + "\">\n");  //$NON-NLS-1$ //$NON-NLS-2$
                 mWriter.write("<div class=\"issue\">\n");                //$NON-NLS-1$
 
                 // Explain this issue
@@ -205,7 +220,7 @@ class HtmlReporter extends Reporter {
                     mWriter.write("<a href=\"");                         //$NON-NLS-1$
                     mWriter.write(issue.getMoreInfo());
                     mWriter.write("\">");                                //$NON-NLS-1$
-                    mWriter.write("More Info");
+                    mWriter.write(issue.getMoreInfo());
                     mWriter.write("</a></div>\n");                       //$NON-NLS-1$
                 }
 
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/UselessViewDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/UselessViewDetector.java
index b2c4d07..a585023 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/UselessViewDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/UselessViewDetector.java
@@ -149,25 +149,34 @@ public class UselessViewDetector extends LayoutDetector {
             return;
         }
 
+        // - A parent can be removed if it doesn't have a background
+        // - A parent can be removed if has a background *and* the child does not have a
+        //   background (in which case, just move the background over to the child, remove
+        //   the parent)
+        // - If both child and parent have a background, the parent cannot be removed (a
+        //   background can be translucent, have transparent padding, etc.)
         boolean nodeHasBackground = element.hasAttributeNS(ANDROID_URI, ATTR_BACKGROUND);
         boolean parentHasBackground = parent.hasAttributeNS(ANDROID_URI, ATTR_BACKGROUND);
-        // TODO: The logic on this has background stuff is a bit unclear to me; this is
-        // a literal translation of the Groovy code in layoutopt
-        // TODO: Get clarification on what the criteria are.
-        if (nodeHasBackground || parentHasBackground ||
-                (!nodeHasBackground && !parentHasBackground)) {
-            boolean hasId = element.hasAttributeNS(ANDROID_URI, ATTR_ID);
-            Location location = context.getLocation(element);
-            String tag = element.getTagName();
-            String format;
-            if (hasId) {
-                format = "This %1$s layout or its %2$s parent is possibly useless";
-            } else {
-                format = "This %1$s layout or its %2$s parent is useless";
-            }
-            String message = String.format(format, tag, parentTag);
-            context.toolContext.report(context, USELESS_PARENT, location, message, null);
+        if (nodeHasBackground && parentHasBackground) {
+            // Can't remove because both define a background, and they might both be
+            // visible (e.g. through transparency or padding).
+            return;
+        }
+
+        boolean hasId = element.hasAttributeNS(ANDROID_URI, ATTR_ID);
+        Location location = context.getLocation(element);
+        String tag = element.getTagName();
+        String format;
+        if (hasId) {
+            format = "This %1$s layout or its %2$s parent is possibly useless";
+        } else {
+            format = "This %1$s layout or its %2$s parent is useless";
+        }
+        if (nodeHasBackground || parentHasBackground) {
+            format += "; transfer the background attribute to the other view";
         }
+        String message = String.format(format, tag, parentTag);
+        context.toolContext.report(context, USELESS_PARENT, location, message, null);
     }
 
     // This is the old UselessView check from layoutopt
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UselessViewDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UselessViewDetectorTest.java
index 6e84205..68e729b 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UselessViewDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/UselessViewDetectorTest.java
@@ -25,12 +25,16 @@ public class UselessViewDetectorTest extends AbstractCheckTest {
         return new UselessViewDetector();
     }
 
-    public void testUseless1() throws Exception {
+    public void testUseless() throws Exception {
         assertEquals(
-                "useless.xml:9: Warning: This LinearLayout layout or its FrameLayout parent " +
-                        "is useless",
-                lint("res/layout/useless.xml"));
+            "useless.xml:13: Warning: This LinearLayout layout or its FrameLayout parent " +
+                    "is useless\n" +
+            "useless.xml:47: Warning: This LinearLayout layout or its FrameLayout parent " +
+                "is useless; transfer the background attribute to the other view\n" +
+            "useless.xml:65: Warning: This LinearLayout layout or its FrameLayout parent " +
+                "is useless; transfer the background attribute to the other view\n" +
+            "useless.xml:85: Warning: This FrameLayout view is useless (no children, " +
+                "no background, no id)",
+            lint("res/layout/useless.xml"));
     }
-
-    // TODO: Test the other case as well
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/useless.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/useless.xml
index b6d5ee7..c317235 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/useless.xml
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/useless.xml
@@ -1,19 +1,89 @@
 <?xml version="1.0" encoding="utf-8"?>
+<merge xmlns:android="http://schemas.android.com/apk/res/android"
+    android:layout_width="match_parent"
+    android:layout_height="match_parent" >
 
-<FrameLayout
-    xmlns:android="http://schemas.android.com/apk/res/android"
+    <!-- Neither parent nor child define background: delete is okay -->
 
-    android:layout_width="match_parent"
-    android:layout_height="match_parent">
+    <FrameLayout
+        android:id="@+id/LinearLayout"
+        android:layout_width="match_parent"
+        android:layout_height="match_parent" >
+
+        <LinearLayout
+            android:layout_width="match_parent"
+            android:layout_height="match_parent" >
+
+            <TextView
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content" />
+        </LinearLayout>
+    </FrameLayout>
+
+    <!-- Both define background: cannot be deleted -->
+
+    <FrameLayout
+        android:layout_width="match_parent"
+        android:layout_height="match_parent"
+        android:background="@drawable/bg" >
+
+        <LinearLayout
+            android:layout_width="match_parent"
+            android:layout_height="match_parent"
+            android:background="@drawable/bg" >
+
+            <TextView
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content" />
+        </LinearLayout>
+    </FrameLayout>
+
+    <!-- Only child defines background: delete is okay -->
+
+    <FrameLayout
+        android:layout_width="match_parent"
+        android:layout_height="match_parent" >
+
+        <LinearLayout
+            android:layout_width="match_parent"
+            android:layout_height="match_parent"
+            android:background="@drawable/bg" >
+
+            <TextView
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content" />
+        </LinearLayout>
+    </FrameLayout>
+
+    <!-- Only parent defines background: delete is okay -->
+
+    <FrameLayout
+        android:layout_width="match_parent"
+        android:layout_height="match_parent"
+        android:background="@drawable/bg" >
+
+        <LinearLayout
+            android:layout_width="match_parent"
+            android:layout_height="match_parent" >
+
+            <TextView
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content" />
+        </LinearLayout>
+    </FrameLayout>
 
-	<LinearLayout
-		android:layout_width="match_parent"
-	    android:layout_height="match_parent">
+    <!-- Leaf cannot be deleted because it has a background -->
 
-		<TextView
-			android:layout_width="wrap_content"
-		    android:layout_height="wrap_content" />
+    <FrameLayout
+        android:layout_width="match_parent"
+        android:layout_height="match_parent"
+        android:background="@drawable/bg" >
+    </FrameLayout>
 
-	</LinearLayout>
+    <!-- Useless leaf -->
 
-</FrameLayout>
+    <FrameLayout
+        android:layout_width="match_parent"
+        android:layout_height="match_parent" >
+    </FrameLayout>
+</merge>