Merge changes from topic "oct5"

* changes:
  Slight relaxing of Context.getUserId() checks.
  Exclude Telephony Binder interfaces.

5 files changed, 147 insertions, 12 deletions

diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java
index 4f1af3e9bea2..3a1bc1eeb9ae 100644
--- a/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java
+++ b/errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java
@@ -18,6 +18,7 @@ package com.google.errorprone.bugpatterns.android;
 
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.bugpatterns.android.UidChecker.getFlavor;
+import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.Matchers.enclosingClass;
 import static com.google.errorprone.matchers.Matchers.hasAnnotation;
 import static com.google.errorprone.matchers.Matchers.instanceMethod;
@@ -60,8 +61,13 @@ public final class ContextUserIdChecker extends BugChecker implements MethodInvo
 
     private static final Matcher<ExpressionTree> BINDER_CALL = methodInvocation(
             instanceMethod().onDescendantOf("android.os.IInterface").withAnyName());
-    private static final Matcher<ExpressionTree> GET_USER_ID_CALL = methodInvocation(
-            instanceMethod().onDescendantOf("android.content.Context").named("getUserId"));
+    private static final Matcher<ExpressionTree> GET_USER_ID_CALL = methodInvocation(anyOf(
+            instanceMethod().onExactClass("android.app.admin.DevicePolicyManager")
+                    .named("myUserId"),
+            instanceMethod().onExactClass("android.content.pm.ShortcutManager")
+                    .named("injectMyUserId"),
+            instanceMethod().onDescendantOf("android.content.Context")
+                    .named("getUserId")));
 
     private static final Matcher<ExpressionTree> USER_ID_FIELD = new Matcher<ExpressionTree>() {
         @Override
diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/RethrowFromSystemChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/RethrowFromSystemChecker.java
index 48123abd26cb..130b256e6622 100644
--- a/errorprone/java/com/google/errorprone/bugpatterns/android/RethrowFromSystemChecker.java
+++ b/errorprone/java/com/google/errorprone/bugpatterns/android/RethrowFromSystemChecker.java
@@ -17,11 +17,14 @@
 package com.google.errorprone.bugpatterns.android;
 
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.matchers.Matchers.allOf;
+import static com.google.errorprone.matchers.Matchers.contains;
 import static com.google.errorprone.matchers.Matchers.enclosingClass;
 import static com.google.errorprone.matchers.Matchers.hasAnnotation;
 import static com.google.errorprone.matchers.Matchers.instanceMethod;
 import static com.google.errorprone.matchers.Matchers.isSameType;
 import static com.google.errorprone.matchers.Matchers.methodInvocation;
+import static com.google.errorprone.matchers.Matchers.not;
 import static com.google.errorprone.matchers.Matchers.throwStatement;
 import static com.google.errorprone.matchers.Matchers.variableType;
 
@@ -29,13 +32,17 @@ import com.google.auto.service.AutoService;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
-import com.google.errorprone.bugpatterns.BugChecker.CatchTreeMatcher;
+import com.google.errorprone.bugpatterns.BugChecker.TryTreeMatcher;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.predicates.TypePredicate;
 import com.sun.source.tree.CatchTree;
+import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.StatementTree;
 import com.sun.source.tree.Tree;
+import com.sun.source.tree.TryTree;
 import com.sun.source.tree.VariableTree;
+import com.sun.tools.javac.code.Type;
 
 import java.util.List;
 
@@ -54,9 +61,17 @@ import java.util.List;
     name = "AndroidFrameworkRethrowFromSystem",
     summary = "Verifies that system_server calls use rethrowFromSystemServer()",
     severity = WARNING)
-public final class RethrowFromSystemChecker extends BugChecker implements CatchTreeMatcher {
+public final class RethrowFromSystemChecker extends BugChecker implements TryTreeMatcher {
     private static final Matcher<Tree> INSIDE_MANAGER =
             enclosingClass(hasAnnotation("android.annotation.SystemService"));
+
+    // Purposefully exclude telephony Binder interfaces, since we know they
+    // always run under the separate AID_RADIO
+    private static final Matcher<ExpressionTree> SYSTEM_BINDER_CALL = methodInvocation(allOf(
+            instanceMethod().onDescendantOf("android.os.IInterface").withAnyName(),
+            not(instanceMethod().onClass(inPackage("com.android.internal.telephony"))),
+            not(instanceMethod().onClass(inPackage("com.android.internal.telecom")))));
+
     private static final Matcher<VariableTree> REMOTE_EXCEPTION = variableType(
             isSameType("android.os.RemoteException"));
     private static final Matcher<StatementTree> RETHROW_FROM_SYSTEM = throwStatement(
@@ -64,17 +79,33 @@ public final class RethrowFromSystemChecker extends BugChecker implements CatchT
                     .named("rethrowFromSystemServer")));
 
     @Override
-    public Description matchCatch(CatchTree tree, VisitorState state) {
+    public Description matchTry(TryTree tree, VisitorState state) {
         if (INSIDE_MANAGER.matches(tree, state)
-                && REMOTE_EXCEPTION.matches(tree.getParameter(), state)) {
-            final List<? extends StatementTree> statements = tree.getBlock().getStatements();
-            if (statements.size() != 1 || !RETHROW_FROM_SYSTEM.matches(statements.get(0), state)) {
-                return buildDescription(tree)
-                        .setMessage("Must contain single "
-                                + "'throw e.rethrowFromSystemServer()' statement")
-                        .build();
+                && contains(ExpressionTree.class, SYSTEM_BINDER_CALL)
+                        .matches(tree.getBlock(), state)) {
+            for (CatchTree catchTree : tree.getCatches()) {
+                if (REMOTE_EXCEPTION.matches(catchTree.getParameter(), state)) {
+                    final List<? extends StatementTree> statements = catchTree.getBlock()
+                            .getStatements();
+                    if (statements.size() != 1
+                            || !RETHROW_FROM_SYSTEM.matches(statements.get(0), state)) {
+                        return buildDescription(catchTree)
+                                .setMessage("Must contain single "
+                                        + "'throw e.rethrowFromSystemServer()' statement")
+                                .build();
+                    }
+                }
             }
         }
         return Description.NO_MATCH;
     }
+
+    private static TypePredicate inPackage(final String filter) {
+        return new TypePredicate() {
+            @Override
+            public boolean apply(Type type, VisitorState state) {
+                return type.tsym.packge().fullname.toString().startsWith(filter);
+            }
+        };
+    }
 }
diff --git a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java
index c0b8cd745afc..c8772306a59b 100644
--- a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java
+++ b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java
@@ -92,4 +92,56 @@ public class ContextUserIdCheckerTest {
                         "}")
                 .doTest();
     }
+
+    @Test
+    public void testDevicePolicyManager() {
+        compilationHelper
+                .addSourceFile("/android/annotation/SystemService.java")
+                .addSourceFile("/android/content/Context.java")
+                .addSourceFile("/android/foo/IFooService.java")
+                .addSourceFile("/android/os/IInterface.java")
+                .addSourceFile("/android/os/UserHandle.java")
+                .addSourceFile("/android/os/RemoteException.java")
+                .addSourceLines("DevicePolicyManager.java",
+                        "package android.app.admin;",
+                        "import android.annotation.SystemService;",
+                        "import android.content.Context;",
+                        "import android.foo.IFooService;",
+                        "import android.os.UserHandle;",
+                        "import android.os.RemoteException;",
+                        "@SystemService(\"dp\") public class DevicePolicyManager {",
+                        "  IFooService mService;",
+                        "  int myUserId() { return 0; }",
+                        "  void bar() throws RemoteException {",
+                        "    mService.baz(null, myUserId());",
+                        "  }",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testShortcutManager() {
+        compilationHelper
+                .addSourceFile("/android/annotation/SystemService.java")
+                .addSourceFile("/android/content/Context.java")
+                .addSourceFile("/android/foo/IFooService.java")
+                .addSourceFile("/android/os/IInterface.java")
+                .addSourceFile("/android/os/UserHandle.java")
+                .addSourceFile("/android/os/RemoteException.java")
+                .addSourceLines("ShortcutManager.java",
+                        "package android.content.pm;",
+                        "import android.annotation.SystemService;",
+                        "import android.content.Context;",
+                        "import android.foo.IFooService;",
+                        "import android.os.UserHandle;",
+                        "import android.os.RemoteException;",
+                        "@SystemService(\"shortcut\") public class ShortcutManager {",
+                        "  IFooService mService;",
+                        "  int injectMyUserId() { return 0; }",
+                        "  void bar() throws RemoteException {",
+                        "    mService.baz(null, injectMyUserId());",
+                        "  }",
+                        "}")
+                .doTest();
+    }
 }
diff --git a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/RethrowFromSystemCheckerTest.java b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/RethrowFromSystemCheckerTest.java
index 32efbf206a45..0943bd65c06f 100644
--- a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/RethrowFromSystemCheckerTest.java
+++ b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/RethrowFromSystemCheckerTest.java
@@ -105,4 +105,27 @@ public class RethrowFromSystemCheckerTest {
                         "}")
                 .doTest();
     }
+
+    @Test
+    public void testTelephony() {
+        compilationHelper
+                .addSourceFile("/android/annotation/SystemService.java")
+                .addSourceFile("/com/android/internal/telephony/ITelephony.java")
+                .addSourceFile("/android/os/IInterface.java")
+                .addSourceFile("/android/os/RemoteException.java")
+                .addSourceLines("TelephonyManager.java",
+                        "import android.annotation.SystemService;",
+                        "import com.android.internal.telephony.ITelephony;",
+                        "import android.os.RemoteException;",
+                        "@SystemService(\"telephony\") public class TelephonyManager {",
+                        "  ITelephony mService;",
+                        "  void bar() {",
+                        "    try {",
+                        "      mService.bar();",
+                        "    } catch (RemoteException ignored) {",
+                        "    }",
+                        "  }",
+                        "}")
+                .doTest();
+    }
 }
diff --git a/errorprone/tests/res/com/android/internal/telephony/ITelephony.java b/errorprone/tests/res/com/android/internal/telephony/ITelephony.java
new file mode 100644
index 000000000000..61c4dd561b0b
--- /dev/null
+++ b/errorprone/tests/res/com/android/internal/telephony/ITelephony.java
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2020 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.internal.telephony;
+
+import android.os.RemoteException;
+
+public interface ITelephony extends android.os.IInterface {
+    public void bar() throws RemoteException;
+}