37329: lint checks for conflicting permission definition

Change-Id: I3b131872cc9938ceb5df271ab10ec60644d44b26

7 files changed, 169 insertions, 3 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 80c3a5a..16a5686 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 = 116;
+        final int initialCapacity = 117;
         List<Issue> issues = new ArrayList<Issue>(initialCapacity);
 
         issues.add(AccessibilityDetector.ISSUE);
@@ -121,6 +121,7 @@ public class BuiltinIssueRegistry extends IssueRegistry {
         issues.add(ManifestOrderDetector.DUPLICATE_ACTIVITY);
         issues.add(ManifestOrderDetector.TARGET_NEWER);
         issues.add(ManifestOrderDetector.ALLOW_BACKUP);
+        issues.add(ManifestOrderDetector.UNIQUE_PERMISSION);
         issues.add(SecurityDetector.EXPORTED_PROVIDER);
         issues.add(SecurityDetector.EXPORTED_SERVICE);
         issues.add(SecurityDetector.EXPORTED_ACTIVITY);
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java
index 73d5bb6..bc5084f 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/ManifestOrderDetector.java
@@ -24,6 +24,7 @@ import static com.android.SdkConstants.ATTR_PACKAGE;
 import static com.android.SdkConstants.ATTR_TARGET_SDK_VERSION;
 import static com.android.SdkConstants.TAG_ACTIVITY;
 import static com.android.SdkConstants.TAG_APPLICATION;
+import static com.android.SdkConstants.TAG_PERMISSION;
 import static com.android.SdkConstants.TAG_PROVIDER;
 import static com.android.SdkConstants.TAG_RECEIVER;
 import static com.android.SdkConstants.TAG_SERVICE;
@@ -42,6 +43,7 @@ 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.collect.Maps;
 
 import org.w3c.dom.Attr;
 import org.w3c.dom.Element;
@@ -52,6 +54,7 @@ import java.io.File;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 
 /**
@@ -202,6 +205,27 @@ public class ManifestOrderDetector extends Detector implements Detector.XmlScann
             Scope.MANIFEST_SCOPE).setMoreInfo(
                     "http://developer.android.com/reference/android/R.attr.html#allowBackup");
 
+    /** Conflicting permission names */
+    public static final Issue UNIQUE_PERMISSION = Issue.create(
+            "UniquePermission", //$NON-NLS-1$
+            "Checks that permission names are unique",
+
+            "The unqualified names or your permissions must be unique. The reason for this " +
+            "is that at build time, the `aapt` tool will generate a class named `Manifest` " +
+            "which contains a field for each of your permissions. These fields are named " +
+            "using your permission unqualified names (i.e. the name portion after the last " +
+            "dot).\n" +
+            "\n" +
+            "If more than one permission maps to the same field name, that field will " +
+            "arbitrarily name just one of them.",
+
+            Category.CORRECTNESS,
+            6,
+            Severity.ERROR,
+            ManifestOrderDetector.class,
+            Scope.MANIFEST_SCOPE);
+
+
     /** Constructs a new {@link ManifestOrderDetector} check */
     public ManifestOrderDetector() {
     }
@@ -214,6 +238,9 @@ public class ManifestOrderDetector extends Detector implements Detector.XmlScann
     /** Activities we've encountered */
     private Set<String> mActivities = new HashSet<String>();
 
+    /** Permission basenames */
+    private Map<String, String> mPermissionNames;
+
     /** Package declared in the manifest */
     private String mPackage;
 
@@ -250,7 +277,7 @@ public class ManifestOrderDetector extends Detector implements Detector.XmlScann
         return Arrays.asList(
                 TAG_APPLICATION,
                 TAG_USES_PERMISSION,
-                "permission",              //$NON-NLS-1$
+                TAG_PERMISSION,
                 "permission-tree",         //$NON-NLS-1$
                 "permission-group",        //$NON-NLS-1$
                 TAG_USES_SDK,
@@ -379,6 +406,46 @@ public class ManifestOrderDetector extends Detector implements Detector.XmlScann
             }
         }
 
+        if (tag.equals(TAG_PERMISSION)) {
+            Attr nameNode = element.getAttributeNodeNS(ANDROID_URI, ATTR_NAME);
+            if (nameNode != null) {
+                String name = nameNode.getValue();
+                String base = name.substring(name.lastIndexOf('.') + 1);
+                if (mPermissionNames == null) {
+                    mPermissionNames = Maps.newHashMap();
+                } else if (mPermissionNames.containsKey(base)) {
+                    String prevName = mPermissionNames.get(base);
+                    Location location = context.getLocation(nameNode);
+                    NodeList siblings = element.getParentNode().getChildNodes();
+                    for (int i = 0, n = siblings.getLength(); i < n; i++) {
+                        Node node = siblings.item(i);
+                        if (node == element) {
+                            break;
+                        } else if (node.getNodeType() == Node.ELEMENT_NODE) {
+                            Element sibling = (Element) node;
+                            String suffix = '.' + base;
+                            if (sibling.getTagName().equals(TAG_PERMISSION)) {
+                                String b = element.getAttributeNS(ANDROID_URI, ATTR_NAME);
+                                if (b.endsWith(suffix)) {
+                                    Location prevLocation = context.getLocation(node);
+                                    prevLocation.setMessage("Previous permission here");
+                                    location.setSecondary(prevLocation);
+                                    break;
+                                }
+
+                            }
+                        }
+                    }
+
+                    String message = String.format("Permission name %1$s is not unique " +
+                            "(appears in both %2$s and %3$s)", base, prevName, name);
+                    context.report(UNIQUE_PERMISSION, element, location, message, null);
+                }
+
+                mPermissionNames.put(base, name);
+            }
+        }
+
         if (tag.equals(TAG_APPLICATION)) {
             mSeenApplication = true;
             if (!element.hasAttributeNS(ANDROID_URI, SdkConstants.ATTR_ALLOW_BACKUP)
diff --git a/lint/libs/lint_checks/src/com/android/tools/lint/checks/MissingClassDetector.java b/lint/libs/lint_checks/src/com/android/tools/lint/checks/MissingClassDetector.java
index ca7e33a..336f820 100644
--- a/lint/libs/lint_checks/src/com/android/tools/lint/checks/MissingClassDetector.java
+++ b/lint/libs/lint_checks/src/com/android/tools/lint/checks/MissingClassDetector.java
@@ -101,7 +101,7 @@ public class MissingClassDetector extends LayoutDetector implements ClassScanner
         "Ensures that inner classes are referenced using '$' instead of '.' in class names",
 
         "When you reference an inner class in a manifest file, you must use '$' instead of '.' " +
-        "as the separator character, e.g. Outer$Inner instead of Outer.Inner.\n" +
+        "as the separator character, i.e. Outer$Inner instead of Outer.Inner.\n" +
         "\n" +
         "(If you get this warning for a class which is not actually an inner class, it's " +
         "because you are using uppercase characters in your package name, which is not " +
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ManifestOrderDetectorTest.java b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ManifestOrderDetectorTest.java
index 8d49bfa..5b4d565 100644
--- a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ManifestOrderDetectorTest.java
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/ManifestOrderDetectorTest.java
@@ -20,6 +20,8 @@ import com.android.tools.lint.detector.api.Detector;
 import com.android.tools.lint.detector.api.Issue;
 import com.android.tools.lint.detector.api.Project;
 
+import java.io.File;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
@@ -232,4 +234,43 @@ public class ManifestOrderDetectorTest extends AbstractCheckTest {
                         "res/values/strings.xml"));
     }
 
+    public void testDuplicatePermissions() throws Exception {
+        mEnabled = Collections.singleton(ManifestOrderDetector.UNIQUE_PERMISSION);
+        assertEquals(
+                "AndroidManifest.xml:12: Error: Permission name SEND_SMS is not unique (appears in both foo.permission.SEND_SMS and bar.permission.SEND_SMS) [UniquePermission]\n" +
+                "    <permission android:name=\"bar.permission.SEND_SMS\"\n" +
+                "                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+                "    AndroidManifest.xml:9: Previous permission here\n" +
+                "1 errors, 0 warnings\n",
+
+                lintProject(
+                        "duplicate_permissions1.xml=>AndroidManifest.xml",
+                        "res/values/strings.xml"));
+    }
+
+    public void testDuplicatePermissionsMultiProject() throws Exception {
+        mEnabled = Collections.singleton(ManifestOrderDetector.UNIQUE_PERMISSION);
+
+        File master = getProjectDir("MasterProject",
+                // Master project
+                "duplicate_permissions2.xml=>AndroidManifest.xml",
+                "multiproject/main-merge.properties=>project.properties",
+                "multiproject/MainCode.java.txt=>src/foo/main/MainCode.java"
+        );
+        File library = getProjectDir("LibraryProject",
+                // Library project
+                "duplicate_permissions3.xml=>AndroidManifest.xml",
+                "multiproject/library.properties=>project.properties",
+                "multiproject/LibraryCode.java.txt=>src/foo/library/LibraryCode.java",
+                "multiproject/strings.xml=>res/values/strings.xml"
+        );
+        assertEquals(
+                "LibraryProject/AndroidManifest.xml:9: Error: Permission name SEND_SMS is not unique (appears in both foo.permission.SEND_SMS and bar.permission.SEND_SMS) [UniquePermission]\n" +
+                "    <permission android:name=\"bar.permission.SEND_SMS\"\n" +
+                "                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
+                "1 errors, 0 warnings\n",
+
+           checkLint(Arrays.asList(master, library)));
+    }
+
 }
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/duplicate_permissions1.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/duplicate_permissions1.xml
new file mode 100644
index 0000000..511fa46
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/duplicate_permissions1.xml
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="foo.bar2"
+    android:versionCode="1"
+    android:versionName="1.0" >
+
+    <uses-sdk android:minSdkVersion="14" />
+
+    <permission android:name="foo.permission.SEND_SMS"
+        android:label="@string/foo"
+        android:description="@string/foo" />
+    <permission android:name="bar.permission.SEND_SMS"
+        android:label="@string/foo"
+        android:description="@string/foo" />
+
+    <application
+        android:icon="@drawable/ic_launcher"
+        android:label="@string/app_name" >
+    </application>
+
+</manifest>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/duplicate_permissions2.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/duplicate_permissions2.xml
new file mode 100644
index 0000000..3729f79
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/duplicate_permissions2.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="foo.bar2"
+    android:versionCode="1"
+    android:versionName="1.0" >
+
+    <uses-sdk android:minSdkVersion="14" />
+
+    <permission android:name="foo.permission.SEND_SMS"
+        android:label="@string/foo"
+        android:description="@string/foo" />
+
+    <application
+        android:icon="@drawable/ic_launcher"
+        android:label="@string/app_name" >
+    </application>
+
+</manifest>
diff --git a/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/duplicate_permissions3.xml b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/duplicate_permissions3.xml
new file mode 100644
index 0000000..a1a0638
--- /dev/null
+++ b/lint/libs/lint_checks/tests/src/com/android/tools/lint/checks/data/duplicate_permissions3.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="foo.bar2"
+    android:versionCode="1"
+    android:versionName="1.0" >
+
+    <uses-sdk android:minSdkVersion="14" />
+
+    <permission android:name="bar.permission.SEND_SMS"
+        android:label="@string/foo"
+        android:description="@string/foo" />
+
+    <application
+        android:icon="@drawable/ic_launcher"
+        android:label="@string/app_name" >
+    </application>
+
+</manifest>