Add detector for fragments missing ids

See issue 35959 for background.

Change-Id: I8042b42e4649163cf8e6974d074ac05777685766

5 files changed, 153 insertions, 2 deletions

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 4b12a9e..70d741e 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
@@ -55,7 +55,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
     private static final List<Issue> sIssues;
 
     static {
-        final int initialCapacity = 112;
+        final int initialCapacity = 113;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -94,6 +94,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(MissingClassDetector.MISSING);
         issues.add(MissingClassDetector.INSTANTIATABLE);
         issues.add(MissingClassDetector.INNERCLASS);
+        issues.add(MissingIdDetector.ISSUE);
         issues.add(HandlerDetector.ISSUE);
         issues.add(FragmentDetector.ISSUE);
         issues.add(TranslationDetector.EXTRA);
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/MissingIdDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/MissingIdDetector.java
new file mode 100644
index 0000000..b02e44a
--- /dev/null
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/MissingIdDetector.java
@@ -0,0 +1,90 @@
+/*
+ * 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 static com.android.SdkConstants.ANDROID_URI;
+import static com.android.SdkConstants.ATTR_ID;
+import static com.android.SdkConstants.ATTR_TAG;
+import static com.android.SdkConstants.VIEW_FRAGMENT;
+
+import com.android.annotations.NonNull;
+import com.android.tools.lint.detector.api.Category;
+import com.android.tools.lint.detector.api.Issue;
+import com.android.tools.lint.detector.api.LayoutDetector;
+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.Element;
+
+import java.util.Collection;
+import java.util.Collections;
+
+/**
+ * Check which looks for missing id's in views where they are probably needed
+ */
+public class MissingIdDetector extends LayoutDetector {
+    /** The main issue discovered by this detector */
+    public static final Issue ISSUE = Issue.create(
+            "MissingId", //$NON-NLS-1$
+            "Ensures that XML tags like <fragment> specify an id or tag attribute",
+
+            "If you do not specify an android:id or an android:tag attribute on a " +
+            "<fragment> element, then if the activity is restarted (for example for " +
+            "an orientation rotation) you may lose state. From the fragment " +
+            "documentation:\n" +
+            "\n" +
+            "\"Each fragment requires a unique identifier that the system can use " +
+            "to restore the fragment if the activity is restarted (and which you can " +
+            "use to capture the fragment to perform transactions, such as remove it). " +
+            "* Supply the android:id attribute with a unique ID.\n" +
+            "* Supply the android:tag attribute with a unique string.\n" +
+            "If you provide neither of the previous two, the system uses the ID of the " +
+            "container view.",
+
+            Category.CORRECTNESS,
+            5,
+            Severity.WARNING,
+            MissingIdDetector.class,
+            Scope.RESOURCE_FILE_SCOPE)
+            .setMoreInfo("http://developer.android.com/guide/components/fragments.html"); //$NON-NLS-1$
+
+    /** Constructs a new {@link MissingIdDetector} */
+    public MissingIdDetector() {
+    }
+
+    @Override
+    public @NonNull Speed getSpeed() {
+        return Speed.FAST;
+    }
+
+    @Override
+    public Collection<String> getApplicableElements() {
+        return Collections.singletonList(VIEW_FRAGMENT);
+    }
+
+    @Override
+    public void visitElement(@NonNull XmlContext context, @NonNull Element element) {
+        if (!element.hasAttributeNS(ANDROID_URI, ATTR_ID) &&
+                !element.hasAttributeNS(ANDROID_URI, ATTR_TAG)) {
+            context.report(ISSUE, element, context.getLocation(element),
+                "This <fragment> tag should specify an id or a tag to preserve state " +
+                "across activity restarts", null);
+        }
+    }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/MissingIdDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/MissingIdDetectorTest.java
new file mode 100644
index 0000000..ac6b6c0
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/MissingIdDetectorTest.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2012 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.tools.lint.checks;
+
+import com.android.tools.lint.detector.api.Detector;
+
+@SuppressWarnings("javadoc")
+public class MissingIdDetectorTest extends AbstractCheckTest {
+    @Override
+    protected Detector getDetector() {
+        return new MissingIdDetector();
+    }
+
+    public void test() throws Exception {
+        assertEquals(
+        "res/layout/fragment.xml:7: Warning: This <fragment> tag should specify an id or a tag to preserve state across activity restarts [MissingId]\n" +
+        "    <fragment\n" +
+        "    ^\n" +
+        "0 errors, 1 warnings\n",
+
+        lintProject("res/layout/fragment.xml"));
+    }
+}
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/PxUsageDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/PxUsageDetectorTest.java
index 9b74676..7b5dd0c 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/PxUsageDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/PxUsageDetectorTest.java
@@ -59,7 +59,7 @@ public class PxUsageDetectorTest extends AbstractCheckTest {
             "        <item name=\"android:textSize\">50mm</item>\n" +
             "                                      ^\n" +
             "res/values/pxsp.xml:25: Warning: Avoid using \"in\" as units (it does not work accurately on all devices); use \"dp\" instead [InOrMmUsage]\n" +
-            "            50in \n" +
+            "            50in\n" +
             "            ^\n" +
             "res/values/pxsp.xml:6: Warning: Should use \"sp\" instead of \"dp\" for text sizes [SpUsage]\n" +
             "        <item name=\"android:textSize\">50dp</item>\n" +
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/fragment.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/fragment.xml
new file mode 100644
index 0000000..bec72b2
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/fragment.xml
@@ -0,0 +1,24 @@
+<?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" >
+
+    <fragment
+        android:name="android.app.ListFragment"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content" />
+
+    <fragment
+        android:name="android.app.DialogFragment"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:tag="mytag" />
+
+    <fragment
+        android:id="@+id/fragment3"
+        android:name="android.preference.PreferenceFragment"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content" />
+
+</LinearLayout>