Fix 40136: Lint "tools:ignore" does not work in xml

Also fix suppress check for a couple of different  detectors.
Also fix typo in TooManyViewsDetector (issue 40496).

Change-Id: Ia2e90ceea0dcae749581bd5704d169c5b2b3f1b6

13 files changed, 203 insertions, 9 deletions

diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/AlwaysShowActionDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/AlwaysShowActionDetector.java
index 6cdb42f..1406ada 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/AlwaysShowActionDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/AlwaysShowActionDetector.java
@@ -83,6 +83,8 @@ public class AlwaysShowActionDetector extends ResourceXmlDetector implements Jav
 
     /** List of showAsAction attributes appearing in the current menu XML file */
     private List<Attr> mFileAttributes;
+    /** If at least one location has been marked ignore in this file, ignore all */
+    private boolean mIgnoreFile;
     /** List of locations of MenuItem.SHOW_AS_ACTION_ALWAYS references in Java code */
     private List<Location> mAlwaysFields;
     /** True if references to MenuItem.SHOW_AS_ACTION_IF_ROOM were found */
@@ -114,6 +116,10 @@ public class AlwaysShowActionDetector extends ResourceXmlDetector implements Jav
 
     @Override
     public void afterCheckFile(@NonNull Context context) {
+        if (mIgnoreFile) {
+            mFileAttributes = null;
+            return;
+        }
         if (mFileAttributes != null) {
             assert context instanceof XmlContext; // mAFilettributes is only set in XML files
 
@@ -175,6 +181,11 @@ public class AlwaysShowActionDetector extends ResourceXmlDetector implements Jav
 
     @Override
     public void visitAttribute(@NonNull XmlContext context, @NonNull Attr attribute) {
+        if (context.getDriver().isSuppressed(ISSUE, attribute)) {
+            mIgnoreFile = true;
+            return;
+        }
+
         if (mFileAttributes == null) {
             mFileAttributes = new ArrayList<Attr>();
         }
@@ -209,6 +220,9 @@ public class AlwaysShowActionDetector extends ResourceXmlDetector implements Jav
             if ((isIfRoom || isAlways)
                     && node.astOperand().toString().equals("MenuItem")) { //$NON-NLS-1$
                 if (isAlways) {
+                    if (mContext.getDriver().isSuppressed(ISSUE, node)) {
+                        return super.visitSelect(node);
+                    }
                     if (mAlwaysFields == null) {
                         mAlwaysFields = new ArrayList<Location>();
                     }
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ButtonDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ButtonDetector.java
index 6ac4cad..a06fc1f 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ButtonDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ButtonDetector.java
@@ -489,7 +489,7 @@ public class ButtonDetector extends ResourceXmlDetector {
         }
 
         Location location = context.getLocation(element);
-        context.report(ORDER, location, message, null);
+        context.report(ORDER, element, location, message, null);
     }
 
     /**
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/SystemPermissionsDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/SystemPermissionsDetector.java
index 641c74a..139af6c 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/SystemPermissionsDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/SystemPermissionsDetector.java
@@ -177,7 +177,7 @@ public class SystemPermissionsDetector extends Detector implements Detector.XmlS
         if (nameNode != null) {
             String permissionName = nameNode.getValue();
             if (Arrays.binarySearch(SYSTEM_PERMISSIONS, permissionName) >= 0) {
-                context.report(ISSUE, context.getLocation(nameNode),
+                context.report(ISSUE, element, context.getLocation(nameNode),
                     "Permission is only granted to system apps", null);
             }
         }
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TooManyViewsDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TooManyViewsDetector.java
index 3256b91..b4095c8 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TooManyViewsDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TooManyViewsDetector.java
@@ -38,7 +38,7 @@ public class TooManyViewsDetector extends LayoutDetector {
     public static final Issue TOO_MANY = Issue.create(
             "TooManyViews", //$NON-NLS-1$
             "Checks whether a layout has too many views",
-            "Using too many views in a single layout in a layout is bad for " +
+            "Using too many views in a single layout is bad for " +
             "performance. Consider using compound drawables or other tricks for " +
             "reducing the number of views in this layout.\n\n" +
             "The maximum view count defaults to 80 but can be configured with the " +
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java
index a65a263..eecb0c7 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/TranslationDetector.java
@@ -514,7 +514,7 @@ public class TranslationDetector extends ResourceXmlDetector {
             if (mNonTranslatable != null && mNonTranslatable.contains(name)) {
                 String message = String.format("The resource string \"%1$s\" has been marked as " +
                         "translatable=\"false\"", name);
-                context.report(EXTRA, context.getLocation(attribute), message, null);
+                context.report(EXTRA, attribute, context.getLocation(attribute), message, null);
             }
 
             // TBD: Also make sure that the strings are not empty or placeholders?
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/XmlReporterTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/XmlReporterTest.java
index 166f721..9ada70f 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/XmlReporterTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/XmlReporterTest.java
@@ -91,7 +91,7 @@ public class XmlReporterTest extends AbstractCheckTest {
                 "        category=\"Correctness\"\n" +
                 "        priority=\"9\"\n" +
                 "        summary=\"Checks that the minimum SDK and target SDK attributes are defined\"\n" +
-                "        explanation=\"The manifest should contain a `&lt;uses-sdk>` element which defines the minimum minimum API Level required for the application to run, as well as the target version (the highest API level you have tested the version for.)\"\n" +
+                "        explanation=\"The manifest should contain a `&lt;uses-sdk>` element which defines the minimum API Level required for the application to run, as well as the target version (the highest API level you have tested the version for.)\"\n" +
                 "        url=\"http://developer.android.com/guide/topics/manifest/uses-sdk-element.html\"\n" +
                 "        errorLine1=\"    &lt;uses-sdk android:minSdkVersion=&quot;8&quot; />\"\n" +
                 "        errorLine2=\"    ^\">\n" +
@@ -112,7 +112,9 @@ public class XmlReporterTest extends AbstractCheckTest {
                 "\n" +
                 "* When creating configuration variations (for example for landscape or portrait)you have to repeat the actual text (and keep it up to date when making changes)\n" +
                 "\n" +
-                "* The application cannot be translated to other languages by just adding new translations for existing string resources.\"\n" +
+                "* The application cannot be translated to other languages by just adding new translations for existing string resources.\n" +
+                "\n" +
+                "In Eclipse there is a quickfix to automatically extract this hardcoded string into a resource lookup.\"\n" +
                 "        errorLine1=\" (java.lang.String)         android:text=&quot;Fooo&quot; />\"\n" +
                 "        errorLine2=\"        ~~~~~~~~~~~~~~~~~~~\">\n" +
                 "        <location\n" +
@@ -190,7 +192,7 @@ public class XmlReporterTest extends AbstractCheckTest {
                 "        category=\"Correctness\"\n" +
                 "        priority=\"9\"\n" +
                 "        summary=\"Checks that the minimum SDK and target SDK attributes are defined\"\n" +
-                "        explanation=\"The manifest should contain a `&lt;uses-sdk>` element which defines the minimum minimum API Level required for the application to run, as well as the target version (the highest API level you have tested the version for.)\"\n" +
+                "        explanation=\"The manifest should contain a `&lt;uses-sdk>` element which defines the minimum API Level required for the application to run, as well as the target version (the highest API level you have tested the version for.)\"\n" +
                 "        url=\"http://developer.android.com/guide/topics/manifest/uses-sdk-element.html\"\n" +
                 "        errorLine1=\"    &lt;uses-sdk android:minSdkVersion=&quot;8&quot; />\"\n" +
                 "        errorLine2=\"    ^\">\n" +
@@ -211,7 +213,9 @@ public class XmlReporterTest extends AbstractCheckTest {
                 "\n" +
                 "* When creating configuration variations (for example for landscape or portrait)you have to repeat the actual text (and keep it up to date when making changes)\n" +
                 "\n" +
-                "* The application cannot be translated to other languages by just adding new translations for existing string resources.\"\n" +
+                "* The application cannot be translated to other languages by just adding new translations for existing string resources.\n" +
+                "\n" +
+                "In Eclipse there is a quickfix to automatically extract this hardcoded string into a resource lookup.\"\n" +
                 "        errorLine1=\" (java.lang.String)         android:text=&quot;Fooo&quot; />\"\n" +
                 "        errorLine2=\"        ~~~~~~~~~~~~~~~~~~~\">\n" +
                 "        <location\n" +
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AlwaysShowActionDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AlwaysShowActionDetectorTest.java
index 51273d1..f7879bb 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AlwaysShowActionDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/AlwaysShowActionDetectorTest.java
@@ -73,4 +73,13 @@ public class AlwaysShowActionDetectorTest extends AbstractCheckTest {
                         "src/test/pkg/ActionTest1.java.txt=>src/test/pkg/ActionTest1.java",
                         "src/test/pkg/ActionTest2.java.txt=>src/test/pkg/ActionTest2.java"));
     }
+
+    public void testSuppress() throws Exception {
+        assertEquals(
+                "No warnings.",
+
+                lintProject(
+                    "res/menu-land/actions2_ignore.xml",
+                    "src/test/pkg/ActionTest1_ignore.java.txt=>src/test/pkg/ActionTest1.java"));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ButtonDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ButtonDetectorTest.java
index f9fb059..58df83a 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ButtonDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ButtonDetectorTest.java
@@ -183,6 +183,17 @@ public class ButtonDetectorTest extends AbstractCheckTest {
                     "res/values/buttonbar-values.xml"));
     }
 
+    public void testSuppressed() throws Exception {
+        sTestIssue = ButtonDetector.ORDER;
+        assertEquals(
+            "No warnings.",
+
+            lintProject(
+                    "apicheck/minsdk14.xml=>AndroidManifest.xml",
+                    "res/layout/buttonbar_suppressed.xml",
+                    "res/values/buttonbar-values.xml"));
+    }
+
     public void testButtonOrderRelativeLayout() throws Exception {
         sTestIssue = ButtonDetector.ORDER;
         assertEquals(
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SystemPermissionsDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SystemPermissionsDetectorTest.java
index baa6fd4..810c8e9 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SystemPermissionsDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/SystemPermissionsDetectorTest.java
@@ -289,5 +289,12 @@ public class SystemPermissionsDetectorTest extends AbstractCheckTest {
 
               lintProject(
                       "protectedpermissions.xml=>AndroidManifest.xml"));
-  }
+    }
+
+    public void testSuppressed() throws Exception {
+        assertEquals(
+            "No warnings.",
+
+            lintProject("protectedpermissions2.xml=>AndroidManifest.xml"));
+    }
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/protectedpermissions2.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/protectedpermissions2.xml
new file mode 100644
index 0000000..c25d581
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/protectedpermissions2.xml
@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
+    package="foo.bar2"
+    android:versionCode="1"
+    android:versionName="1.0" >
+
+    <uses-sdk android:minSdkVersion="14" />
+
+    <!-- No warnings for these: -->
+    <uses-permission android:name="android.permission.GET_ACCOUNTS" />
+    <uses-permission android:name="android.permission.SEND_SMS" />
+    <uses-permission android:name="android.permission.INTERNET" />
+
+    <!-- Warnings for these: -->
+    <uses-permission android:name="android.intent.category.MASTER_CLEAR.permission.C2D_MESSAGE" tools:ignore="ProtectedPermissons" />
+
+    <application
+        android:icon="@drawable/ic_launcher"
+        android:label="@string/app_name" >
+        <activity
+            android:label="@string/app_name"
+            android:name=".Foo2Activity" >
+            <intent-filter >
+                <action android:name="android.intent.action.MAIN" />
+
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
+    </application>
+
+</manifest>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/buttonbar_suppressed.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/buttonbar_suppressed.xml
new file mode 100644
index 0000000..a03824d
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/layout/buttonbar_suppressed.xml
@@ -0,0 +1,43 @@
+<?xml version="1.0" encoding="utf-8"?>
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
+    android:layout_width="match_parent"
+    android:layout_height="match_parent"
+    android:orientation="vertical" >
+
+    <!-- Hardcoded strings, wrong order -->
+    <LinearLayout
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content" >
+
+        <Button
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:text="OK"
+            tools:ignore="ButtonOrder" />
+
+        <Button
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:text="Cancel"
+            tools:ignore="ButtonOrder" />
+    </LinearLayout>
+
+    <!-- @android:string resources, wrong order -->
+    <LinearLayout
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        tools:ignore="ButtonOrder" >
+
+        <Button
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:text="@android:string/ok" />
+
+        <Button
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:text="@android:string/cancel" />
+    </LinearLayout>
+
+</LinearLayout>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/menu-land/actions2_ignore.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/menu-land/actions2_ignore.xml
new file mode 100644
index 0000000..47cd32e
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/res/menu-land/actions2_ignore.xml
@@ -0,0 +1,64 @@
+<?xml version="1.0" encoding="utf-8"?>
+<menu xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools">
+
+    <item
+        android:id="@+id/menu_search"
+        android:showAsAction="always|collapseActionView"
+        android:actionViewClass="android.widget.SearchView"
+        tools:ignore="AlwaysShowAction" />
+
+    <group android:id="@+id/reader_items">
+
+        <item
+            android:id="@+id/menu_table_of_contents"
+            android:showAsAction="always|collapseActionView"
+            android:actionLayout="@layout/action_table_of_contents" />
+
+        <item
+            android:id="@+id/menu_settings"
+            android:showAsAction="always|collapseActionView" />
+
+        <item android:id="@+id/menu_mode"
+            android:showAsAction="never" />
+
+        <item
+            android:id="@+id/menu_buy"
+            android:showAsAction="never" />
+
+        <item
+            android:id="@+id/menu_about"
+            android:showAsAction="never" />
+
+        <item
+            android:id="@+id/menu_share"
+            android:showAsAction="never" />
+
+        <item
+            android:id="@+id/menu_keep"
+            android:checkable="true"
+            android:showAsAction="never" />
+
+        <item
+            android:id="@+id/menu_d"
+            android:showAsAction="never" />
+
+        <item
+            android:id="@+id/menu_help"
+            android:showAsAction="never" />
+
+    </group>
+
+    <group android:id="@+id/search_items">
+
+        <item
+            android:id="@+id/menu_table_of_contents"
+            android:showAsAction="always|collapseActionView"
+            android:actionLayout="@layout/action_table_of_contents" />
+
+        <item android:id="@+id/menu_search_exit"
+              android:showAsAction="never" />
+
+    </group>
+
+</menu>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/ActionTest1_ignore.java.txt b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/ActionTest1_ignore.java.txt
new file mode 100644
index 0000000..5fea6be
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/src/test/pkg/ActionTest1_ignore.java.txt
@@ -0,0 +1,10 @@
+package test.pkg;
+
+import android.view.MenuItem;
+
+public class ActionTest1 {
+    @SuppressLint("AlwaysShowAction")
+    public void foo() {
+        System.out.println(MenuItem.SHOW_AS_ACTION_ALWAYS);
+    }
+}