Don't report missing layout_width/height if set in theme

Change-Id: I48ecfea6304abedb942161bf2fec1d9867ddf0c6

4 files changed, 115 insertions, 3 deletions

diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/RequiredAttributeDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/RequiredAttributeDetector.java
index af49d6c..e31d8a5 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/RequiredAttributeDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/RequiredAttributeDetector.java
@@ -124,6 +124,12 @@ public class RequiredAttributeDetector extends LayoutDetector implements Detecto
      * where the height is <b>not</b> set on the include */
     private @Nullable Set<String> mNotIncludedHeights;
 
+    /** Whether the width was set in a theme definition */
+    private boolean mSetWidthInTheme;
+
+    /** Whether the height was set in a theme definition */
+    private boolean mSetHeightInTheme;
+
     /** Constructs a new {@link RequiredAttributeDetector} */
     public RequiredAttributeDetector() {
     }
@@ -148,6 +154,8 @@ public class RequiredAttributeDetector extends LayoutDetector implements Detecto
         // not known.
         //
         if (context.getPhase() == 1) {
+            checkSizeSetInTheme();
+
             context.requestRepeat(this, Scope.RESOURCE_FILE_SCOPE);
         }
     }
@@ -227,6 +235,65 @@ public class RequiredAttributeDetector extends LayoutDetector implements Detecto
         return false;
     }
 
+    private void checkSizeSetInTheme() {
+        // Look through the styles and determine whether each style is a theme
+        if (mStyleParents == null) {
+            return;
+        }
+
+        Map<String, Boolean> isTheme = Maps.newHashMap();
+        for (String style : mStyleParents.keySet()) {
+            if (isTheme(stripStylePrefix(style), isTheme, 0)) {
+                mSetWidthInTheme = true;
+                mSetHeightInTheme = true;
+                break;
+            }
+        }
+    }
+
+    private boolean isTheme(String style, Map<String, Boolean> isTheme, int depth) {
+        if (depth == 30) {
+            // Cycle between local and framework attribute style missed
+            // by the fact that we're stripping the distinction between framework
+            // and local styles here
+            return false;
+        }
+
+        assert !style.startsWith(STYLE_RESOURCE_PREFIX)
+                && !style.startsWith(ANDROID_STYLE_RESOURCE_PREFIX);
+
+        Boolean known = isTheme.get(style);
+        if (known != null) {
+            return known;
+        }
+
+        if (style.contains("Theme")) { //$NON-NLS-1$
+            isTheme.put(style, true);
+            return true;
+        }
+
+        if (mStyleParents != null) {
+            String parentStyle = mStyleParents.get(style);
+            if (parentStyle != null) {
+                parentStyle = stripStylePrefix(parentStyle);
+                if (isTheme(parentStyle, isTheme, depth + 1)) {
+                    isTheme.put(style, true);
+                    return true;
+                }
+            }
+        }
+
+        int index = style.lastIndexOf('.');
+        if (index > 0) {
+            String parentStyle = style.substring(0, index);
+            boolean result = isTheme(parentStyle, isTheme, depth + 1);
+            isTheme.put(style, result);
+            return result;
+        }
+
+        return false;
+    }
+
     private static boolean hasLayoutVariations(File file) {
         File parent = file.getParentFile();
         if (parent == null) {
@@ -305,7 +372,6 @@ public class RequiredAttributeDetector extends LayoutDetector implements Detecto
                     }
                     String styleName = ((Element) element.getParentNode()).getAttribute(ATTR_NAME);
                     mWidthStyles.add(styleName);
-
                 }
                 if (name.endsWith(ATTR_LAYOUT_HEIGHT) &&
                         name.equals(ANDROID_NS_NAME_PREFIX + ATTR_LAYOUT_HEIGHT)) {
@@ -314,7 +380,6 @@ public class RequiredAttributeDetector extends LayoutDetector implements Detecto
                     }
                     String styleName = ((Element) element.getParentNode()).getAttribute(ATTR_NAME);
                     mHeightStyles.add(styleName);
-
                 }
             }
         } else if (folderType == LAYOUT) {
@@ -333,6 +398,15 @@ public class RequiredAttributeDetector extends LayoutDetector implements Detecto
                 assert phase == 2; // Check everything using style data and include data
                 boolean hasWidth = element.hasAttributeNS(ANDROID_URI, ATTR_LAYOUT_WIDTH);
                 boolean hasHeight = element.hasAttributeNS(ANDROID_URI, ATTR_LAYOUT_HEIGHT);
+
+                if (mSetWidthInTheme) {
+                    hasWidth = true;
+                }
+
+                if (mSetHeightInTheme) {
+                    hasHeight = true;
+                }
+
                 if (hasWidth && hasHeight) {
                     return;
                 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/RequiredAttributeDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/RequiredAttributeDetectorTest.java
index 1e3b660..9b60532 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/RequiredAttributeDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/RequiredAttributeDetectorTest.java
@@ -58,7 +58,6 @@ public class RequiredAttributeDetectorTest extends AbstractCheckTest {
                     "res/layout/size2.xml",
                     "res/layout/sizeincluded.xml",
                     "res/values/sizestyles.xml"
-                    //"res/layout/sizeincludedmerge"
                     ));
     }
 
@@ -102,4 +101,25 @@ public class RequiredAttributeDetectorTest extends AbstractCheckTest {
             ));
     }
 
+    public void testThemeStyles() throws Exception {
+        // Check that we don't complain about cases where the size is defined in a theme
+        assertEquals(
+            "No warnings.",
+
+            lintProject(
+                    "res/layout/size.xml",
+                    "res/values/themes.xml"
+            ));
+    }
+
+    public void testThemeStyles2() throws Exception {
+        // Check that we don't complain about cases where the size is defined in a theme
+        assertEquals(
+            "No warnings.",
+
+            lintProject(
+                    "res/layout/size.xml",
+                    "res/values/themes2.xml"
+            ));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/themes.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/themes.xml
new file mode 100644
index 0000000..eba1c3c
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/themes.xml
@@ -0,0 +1,8 @@
+<resources xmlns:android="http://schemas.android.com/apk/res/android">
+
+    <style name="MyTheme" parent="@android:style/Theme">
+        <item name="android:layout_width">wrap_content</item>
+        <item name="android:layout_height">wrap_content</item>
+    </style>
+
+</resources>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/themes2.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/themes2.xml
new file mode 100644
index 0000000..b587923
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/values/themes2.xml
@@ -0,0 +1,10 @@
+<resources xmlns:android="http://schemas.android.com/apk/res/android">
+
+    <style name="MyStyle" parent="@style/Theme.Holo.Light"></style>
+    <style name="MyStyle.Big"></style>
+    <style name="MyOtherStyle" parent="@style/MyStyle.Big">
+        <item name="android:layout_width">wrap_content</item>
+        <item name="android:layout_height">wrap_content</item>
+    </style>
+
+</resources>