RequiresPermissionChecker: change detection method

Change the way the checker find the intent and the permission by using
the order instead of using the name of the variable. (since some target
have their parameters name being obfuscated prior to pass them to the
annotation processor)

Also fix when the intent is set in a chained pattern like:
new Intent(Foo).putExtra(...)

Add various test to make sure the new detection works for differently
ordered parameters

Fix checkstyle reported errors and move some check to be early return
instead of having a unneeded indentation level

Bug: 352610940
Fix: 352610940
Test: atest error_prone_android_framework_test | for unit test
Test: m Bluetooth and check the warning / errors
Flag: TEST_ONLY
Change-Id: I9308c08c7275708c5b1db3f35a9753563ba05dc9

4 files changed, 101 insertions, 71 deletions

diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java
index 7c7cb18afc4d..9887c272e7f8 100644
--- a/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java
+++ b/errorprone/java/com/google/errorprone/bugpatterns/android/RequiresPermissionChecker.java
@@ -55,9 +55,9 @@ import com.sun.source.util.TreeScanner;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.ClassSymbol;
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
-import com.sun.tools.javac.code.Symbol.VarSymbol;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Type.ClassType;
+import com.sun.tools.javac.tree.JCTree.JCNewClass;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -67,7 +67,6 @@ import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.function.Predicate;
 import java.util.regex.Pattern;
 
 import javax.lang.model.element.Name;
@@ -125,6 +124,12 @@ public final class RequiresPermissionChecker extends BugChecker
             instanceMethod()
                     .onDescendantOf("android.content.Context")
                     .withNameMatching(Pattern.compile("^send(Ordered|Sticky)?Broadcast.*$")));
+    private static final Matcher<ExpressionTree> SEND_BROADCAST_AS_USER =
+            methodInvocation(
+                    instanceMethod()
+                            .onDescendantOf("android.content.Context")
+                            .withNameMatching(
+                                    Pattern.compile("^send(Ordered|Sticky)?Broadcast.*AsUser.*$")));
     private static final Matcher<ExpressionTree> SEND_PENDING_INTENT = methodInvocation(
             instanceMethod()
                     .onDescendantOf("android.app.PendingIntent")
@@ -306,18 +311,6 @@ public final class RequiresPermissionChecker extends BugChecker
         }
     }
 
-    private static ExpressionTree findArgumentByParameterName(MethodInvocationTree tree,
-            Predicate<String> paramName) {
-        final MethodSymbol sym = ASTHelpers.getSymbol(tree);
-        final List<VarSymbol> params = sym.getParameters();
-        for (int i = 0; i < params.size(); i++) {
-            if (paramName.test(params.get(i).name.toString())) {
-                return tree.getArguments().get(i);
-            }
-        }
-        return null;
-    }
-
     private static Name resolveName(ExpressionTree tree) {
         if (tree instanceof IdentifierTree) {
             return ((IdentifierTree) tree).getName();
@@ -345,76 +338,85 @@ public final class RequiresPermissionChecker extends BugChecker
 
     private static ParsedRequiresPermission parseBroadcastSourceRequiresPermission(
             MethodInvocationTree methodTree, VisitorState state) {
-        final ExpressionTree arg = findArgumentByParameterName(methodTree,
-                (name) -> name.toLowerCase().contains("intent"));
-        if (arg instanceof IdentifierTree) {
-            final Name argName = ((IdentifierTree) arg).getName();
-            final MethodTree method = state.findEnclosing(MethodTree.class);
-            final AtomicReference<ParsedRequiresPermission> res = new AtomicReference<>();
-            method.accept(new TreeScanner<Void, Void>() {
-                private ParsedRequiresPermission last;
-
-                @Override
-                public Void visitMethodInvocation(MethodInvocationTree tree, Void param) {
-                    if (Objects.equal(methodTree, tree)) {
-                        res.set(last);
-                    } else {
-                        final Name name = resolveName(tree.getMethodSelect());
-                        if (Objects.equal(argName, name)
-                                && INTENT_SET_ACTION.matches(tree, state)) {
-                            last = parseIntentAction(tree);
+        if (methodTree.getArguments().size() < 1) {
+            return null;
+        }
+        final ExpressionTree arg = methodTree.getArguments().get(0);
+        if (!(arg instanceof IdentifierTree)) {
+            return null;
+        }
+        final Name argName = ((IdentifierTree) arg).getName();
+        final MethodTree method = state.findEnclosing(MethodTree.class);
+        final AtomicReference<ParsedRequiresPermission> res = new AtomicReference<>();
+        method.accept(new TreeScanner<Void, Void>() {
+            private ParsedRequiresPermission mLast;
+
+            @Override
+            public Void visitMethodInvocation(MethodInvocationTree tree, Void param) {
+                if (Objects.equal(methodTree, tree)) {
+                    res.set(mLast);
+                } else {
+                    final Name name = resolveName(tree.getMethodSelect());
+                    if (Objects.equal(argName, name) && INTENT_SET_ACTION.matches(tree, state)) {
+                        mLast = parseIntentAction(tree);
+                    } else if (name == null && tree.getMethodSelect() instanceof MemberSelectTree) {
+                        ExpressionTree innerTree =
+                                ((MemberSelectTree) tree.getMethodSelect()).getExpression();
+                        if (innerTree instanceof JCNewClass) {
+                            mLast = parseIntentAction((NewClassTree) innerTree);
                         }
                     }
-                    return super.visitMethodInvocation(tree, param);
                 }
+                return super.visitMethodInvocation(tree, param);
+            }
 
-                @Override
-                public Void visitAssignment(AssignmentTree tree, Void param) {
-                    final Name name = resolveName(tree.getVariable());
-                    final Tree init = tree.getExpression();
-                    if (Objects.equal(argName, name)
-                            && init instanceof NewClassTree) {
-                        last = parseIntentAction((NewClassTree) init);
-                    }
-                    return super.visitAssignment(tree, param);
+            @Override
+            public Void visitAssignment(AssignmentTree tree, Void param) {
+                final Name name = resolveName(tree.getVariable());
+                final Tree init = tree.getExpression();
+                if (Objects.equal(argName, name) && init instanceof NewClassTree) {
+                    mLast = parseIntentAction((NewClassTree) init);
                 }
+                return super.visitAssignment(tree, param);
+            }
 
-                @Override
-                public Void visitVariable(VariableTree tree, Void param) {
-                    final Name name = tree.getName();
-                    final ExpressionTree init = tree.getInitializer();
-                    if (Objects.equal(argName, name)
-                            && init instanceof NewClassTree) {
-                        last = parseIntentAction((NewClassTree) init);
-                    }
-                    return super.visitVariable(tree, param);
+            @Override
+            public Void visitVariable(VariableTree tree, Void param) {
+                final Name name = tree.getName();
+                final ExpressionTree init = tree.getInitializer();
+                if (Objects.equal(argName, name) && init instanceof NewClassTree) {
+                    mLast = parseIntentAction((NewClassTree) init);
                 }
-            }, null);
-            return res.get();
-        }
-        return null;
+                return super.visitVariable(tree, param);
+            }
+        }, null);
+        return res.get();
     }
 
     private static ParsedRequiresPermission parseBroadcastTargetRequiresPermission(
             MethodInvocationTree tree, VisitorState state) {
-        final ExpressionTree arg = findArgumentByParameterName(tree,
-                (name) -> name.toLowerCase().contains("permission"));
         final ParsedRequiresPermission res = new ParsedRequiresPermission();
-        if (arg != null) {
-            arg.accept(new TreeScanner<Void, Void>() {
-                @Override
-                public Void visitIdentifier(IdentifierTree tree, Void param) {
-                    res.addConstValue(tree);
-                    return super.visitIdentifier(tree, param);
-                }
-
-                @Override
-                public Void visitMemberSelect(MemberSelectTree tree, Void param) {
-                    res.addConstValue(tree);
-                    return super.visitMemberSelect(tree, param);
-                }
-            }, null);
+        int permission_position = 1;
+        if (SEND_BROADCAST_AS_USER.matches(tree, state)) {
+            permission_position = 2;
         }
+        if (tree.getArguments().size() < permission_position + 1) {
+            return res;
+        }
+        final ExpressionTree arg = tree.getArguments().get(permission_position);
+        arg.accept(new TreeScanner<Void, Void>() {
+            @Override
+            public Void visitIdentifier(IdentifierTree tree, Void param) {
+                res.addConstValue(tree);
+                return super.visitIdentifier(tree, param);
+            }
+
+            @Override
+            public Void visitMemberSelect(MemberSelectTree tree, Void param) {
+                res.addConstValue(tree);
+                return super.visitMemberSelect(tree, param);
+            }
+        }, null);
         return res;
     }
 
diff --git a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/RequiresPermissionCheckerTest.java b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/RequiresPermissionCheckerTest.java
index e53372d97f3d..05fde7c4fe57 100644
--- a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/RequiresPermissionCheckerTest.java
+++ b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/RequiresPermissionCheckerTest.java
@@ -412,6 +412,19 @@ public class RequiresPermissionCheckerTest {
                         "      context.sendBroadcast(intent);",
                         "    }",
                         "  }",
+                        "  public void exampleWithChainedMethod(Context context) {",
+                        "    Intent intent = new Intent(FooManager.ACTION_RED)",
+                        "            .putExtra(\"foo\", 42);",
+                        "    context.sendBroadcast(intent, FooManager.PERMISSION_RED);",
+                        "    context.sendBroadcastWithMultiplePermissions(intent,",
+                        "        new String[] { FooManager.PERMISSION_RED });",
+                        "  }",
+                        "  public void exampleWithAsUser(Context context) {",
+                        "    Intent intent = new Intent(FooManager.ACTION_RED);",
+                        "    context.sendBroadcastAsUser(intent, 42, FooManager.PERMISSION_RED);",
+                        "    context.sendBroadcastAsUserMultiplePermissions(intent, 42,",
+                        "        new String[] { FooManager.PERMISSION_RED });",
+                        "  }",
                         "}")
                 .doTest();
     }
diff --git a/errorprone/tests/res/android/content/Context.java b/errorprone/tests/res/android/content/Context.java
index efc4fb122435..9d622ffaf120 100644
--- a/errorprone/tests/res/android/content/Context.java
+++ b/errorprone/tests/res/android/content/Context.java
@@ -36,4 +36,15 @@ public class Context {
     public void sendBroadcastWithMultiplePermissions(Intent intent, String[] receiverPermissions) {
         throw new UnsupportedOperationException();
     }
+
+    /* Fake user type for test purposes */
+    public void sendBroadcastAsUser(Intent intent, int user, String receiverPermission) {
+        throw new UnsupportedOperationException();
+    }
+
+    /* Fake user type for test purposes */
+    public void sendBroadcastAsUserMultiplePermissions(
+            Intent intent, int user, String[] receiverPermissions) {
+        throw new UnsupportedOperationException();
+    }
 }
diff --git a/errorprone/tests/res/android/content/Intent.java b/errorprone/tests/res/android/content/Intent.java
index 288396e60577..7ccea784754a 100644
--- a/errorprone/tests/res/android/content/Intent.java
+++ b/errorprone/tests/res/android/content/Intent.java
@@ -24,4 +24,8 @@ public class Intent {
     public Intent setAction(String action) {
         throw new UnsupportedOperationException();
     }
+
+    public Intent putExtra(String extra, int value) {
+        throw new UnsupportedOperationException();
+    }
 }