Make view cast detector handle multiple types

Change-Id: I375ebc368a0173e78fd8aae1c6fc658c4ec5416d

4 files changed, 121 insertions, 25 deletions

diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewTypeDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewTypeDetector.java
index 61498f4..46d806e 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewTypeDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ViewTypeDetector.java
@@ -37,10 +37,12 @@ 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 com.google.common.base.Joiner;
 
 import org.w3c.dom.Attr;
 
 import java.io.File;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
@@ -68,7 +70,7 @@ public class ViewTypeDetector extends ResourceXmlDetector implements Detector.Ja
             ViewTypeDetector.class,
             EnumSet.of(Scope.ALL_RESOURCE_FILES, Scope.ALL_JAVA_FILES));
 
-    private Map<String, String> mIdToViewTag = new HashMap<String, String>(50);
+    private Map<String, Object> mIdToViewTag = new HashMap<String, Object>(50);
 
     @Override
     public @NonNull Speed getSpeed() {
@@ -94,13 +96,6 @@ public class ViewTypeDetector extends ResourceXmlDetector implements Detector.Ja
         return Collections.singletonList(ATTR_ID);
     }
 
-    /** Special marker value which means that an id is used for multiple different view types;
-     * in this case we should figure out which ids are reachable from different activities
-     * and do a more fine grained analysis to report casting problems, but for now we just
-     * mark these id's to be ignored instead
-     */
-    private static final String IGNORE = "#ignore#";
-
     @Override
     public void visitAttribute(@NonNull XmlContext context, @NonNull Attr attribute) {
         String view = attribute.getOwnerElement().getTagName();
@@ -117,11 +112,25 @@ public class ViewTypeDetector extends ResourceXmlDetector implements Detector.Ja
                 view = attribute.getOwnerElement().getAttribute(ATTR_CLASS);
             }
 
-            String existing = mIdToViewTag.get(id);
-            if (existing != null && !existing.equals(view)) {
-                view = IGNORE;
+            Object existing = mIdToViewTag.get(id);
+            if (existing == null) {
+                mIdToViewTag.put(id, view);
+            } else if (existing instanceof String) {
+                String existingString = (String) existing;
+                if (!existingString.equals(view)) {
+                    // Convert to list
+                    List<String> list = new ArrayList<String>(2);
+                    list.add((String) existing);
+                    list.add(view);
+                    mIdToViewTag.put(id, list);
+                }
+            } else if (existing instanceof List<?>) {
+                @SuppressWarnings("unchecked")
+                List<String> list = (List<String>) existing;
+                if (!list.contains(view)) {
+                    list.add(view);
+                }
             }
-            mIdToViewTag.put(id, view);
         }
     }
 
@@ -148,9 +157,14 @@ public class ViewTypeDetector extends ResourceXmlDetector implements Detector.Ja
                     String resource = first.toString();
                     if (resource.startsWith("R.id.")) { //$NON-NLS-1$
                         String id = ((Select) first).astIdentifier().astValue();
-                        String layoutType = mIdToViewTag.get(id);
-                        if (layoutType != null) {
-                            checkCompatible(context, castType, layoutType, cast);
+                        Object types = mIdToViewTag.get(id);
+                        if (types instanceof String) {
+                            String layoutType = (String) types;
+                            checkCompatible(context, castType, layoutType, null, cast);
+                        } else if (types instanceof List<?>) {
+                            @SuppressWarnings("unchecked")
+                            List<String> layoutTypes = (List<String>) types;
+                            checkCompatible(context, castType, null, layoutTypes, cast);
                         }
                     }
                 }
@@ -160,15 +174,35 @@ public class ViewTypeDetector extends ResourceXmlDetector implements Detector.Ja
 
     /** Check if the view and cast type are compatible */
     private void checkCompatible(JavaContext context, String castType, String layoutType,
-            Cast node) {
-        if (layoutType != null && !layoutType.equals(IGNORE) && !layoutType.equals(castType)) {
-            if (!context.getSdkInfo().isSubViewOf(castType, layoutType)) {
-                String message = String.format(
-                        "Unexpected cast to %1$s: layout tag was %2$s",
-                        castType, layoutType);
-                context.report(ISSUE, node, context.parser.getLocation(context, node), message,
-                        null);
+            List<String> layoutTypes, Cast node) {
+        assert layoutType == null || layoutTypes == null; // Should only specify one or the other
+        boolean compatible = true;
+        if (layoutType != null) {
+            if (!layoutType.equals(castType)
+                    && !context.getSdkInfo().isSubViewOf(castType, layoutType)) {
+                compatible = false;
+            }
+        } else {
+            compatible = false;
+            assert layoutTypes != null;
+            for (String type : layoutTypes) {
+                if (type.equals(castType)
+                        || context.getSdkInfo().isSubViewOf(castType, type)) {
+                    compatible = true;
+                    break;
+                }
+            }
+        }
+
+        if (!compatible) {
+            if (layoutType == null) {
+                layoutType = Joiner.on("|").join(layoutTypes);
             }
+            String message = String.format(
+                    "Unexpected cast to %1$s: layout tag was %2$s",
+                    castType, layoutType);
+            context.report(ISSUE, node, context.parser.getLocation(context, node), message,
+                    null);
         }
     }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ViewTypeDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ViewTypeDetectorTest.java
index bfc56e2..cf38197 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ViewTypeDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ViewTypeDetectorTest.java
@@ -30,8 +30,7 @@ public class ViewTypeDetectorTest extends AbstractCheckTest {
             "src/test/pkg/WrongCastActivity.java:13: Error: Unexpected cast to ToggleButton: layout tag was Button [WrongViewCast]\n" +
             "        ToggleButton toggleButton = (ToggleButton) findViewById(R.id.button);\n" +
             "                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
-            "1 errors, 0 warnings\n" +
-            "",
+            "1 errors, 0 warnings\n",
 
             lintProject(
                     "res/layout/casts.xml",
@@ -39,6 +38,31 @@ public class ViewTypeDetectorTest extends AbstractCheckTest {
                 ));
     }
 
+    public void test2() throws Exception {
+        assertEquals(
+            "src/test/pkg/WrongCastActivity.java:13: Error: Unexpected cast to ToggleButton: layout tag was Button|RadioButton [WrongViewCast]\n" +
+            "        ToggleButton toggleButton = (ToggleButton) findViewById(R.id.button);\n" +
+            "                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+            "1 errors, 0 warnings\n",
+
+            lintProject(
+                    "res/layout/casts.xml",
+                    "res/layout/casts3.xml",
+                    "src/test/pkg/WrongCastActivity.java.txt=>src/test/pkg/WrongCastActivity.java"
+                ));
+    }
+
+    public void test3() throws Exception {
+        assertEquals(
+            "No warnings.",
+
+            lintProject(
+                    "res/layout/casts.xml",
+                    "res/layout/casts4.xml",
+                    "src/test/pkg/WrongCastActivity.java.txt=>src/test/pkg/WrongCastActivity.java"
+                ));
+    }
+
     public void test27441() throws Exception {
         assertEquals(
             "No warnings.",
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/casts3.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/casts3.xml
new file mode 100644
index 0000000..990e5f0
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/casts3.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="utf-8"?>
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    android:layout_width="match_parent"
+    android:layout_height="match_parent"
+    android:orientation="vertical" >
+
+    <RadioButton
+        android:id="@+id/button"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:text="Button" />
+
+    <EditText
+        android:id="@+id/edittext"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:text="EditText" />
+
+</LinearLayout>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/casts4.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/casts4.xml
new file mode 100644
index 0000000..371964e
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/casts4.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="utf-8"?>
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    android:layout_width="match_parent"
+    android:layout_height="match_parent"
+    android:orientation="vertical" >
+
+    <ToggleButton
+        android:id="@+id/button"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:text="Button" />
+
+    <EditText
+        android:id="@+id/edittext"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:text="EditText" />
+
+</LinearLayout>