Merge "Slight relaxing of ContextUserIdChecker."

errorprone/java/com/google/errorprone/bugpatterns/android/ContextUserIdChecker.java

@@ -17,6 +17,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.enclosingClass;
 import static com.google.errorprone.matchers.Matchers.hasAnnotation;
 import static com.google.errorprone.matchers.Matchers.instanceMethod;
@@ -27,17 +28,17 @@ import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.bugpatterns.android.UidChecker.Flavor;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.IdentifierTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.source.tree.Tree;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
 
 import java.util.List;
-import java.util.Locale;
-import java.util.function.Predicate;
 
 /**
  * To avoid an explosion of {@code startActivityForUser} style methods, we've
@@ -62,39 +63,34 @@ public final class ContextUserIdChecker extends BugChecker implements MethodInvo
     private static final Matcher<ExpressionTree> GET_USER_ID_CALL = methodInvocation(
             instanceMethod().onDescendantOf("android.content.Context").named("getUserId"));
 
+    private static final Matcher<ExpressionTree> USER_ID_FIELD = new Matcher<ExpressionTree>() {
+        @Override
+        public boolean matches(ExpressionTree tree, VisitorState state) {
+            if (tree instanceof IdentifierTree) {
+                return "mUserId".equals((((IdentifierTree) tree).getName().toString()));
+            }
+            return false;
+        }
+    };
+
     @Override
     public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
         if (INSIDE_MANAGER.matches(tree, state)
                 && BINDER_CALL.matches(tree, state)) {
             final List<VarSymbol> vars = ASTHelpers.getSymbol(tree).params();
             for (int i = 0; i < vars.size(); i++) {
-                if (USER_ID_VAR.test(vars.get(i)) &&
-                        !GET_USER_ID_CALL.matches(tree.getArguments().get(i), state)) {
+                final Flavor varFlavor = getFlavor(vars.get(i));
+                final ExpressionTree arg = tree.getArguments().get(i);
+                if (varFlavor == Flavor.USER_ID &&
+                        !GET_USER_ID_CALL.matches(arg, state) &&
+                        !USER_ID_FIELD.matches(arg, state)) {
                     return buildDescription(tree)
                             .setMessage("Must pass Context.getUserId() as user ID"
-                                    + "to enable createContextAsUser()")
+                                    + " to enable createContextAsUser()")
                             .build();
                 }
             }
         }
         return Description.NO_MATCH;
     }
-
-    private static final UserIdMatcher USER_ID_VAR = new UserIdMatcher();
-
-    private static class UserIdMatcher implements Predicate<VarSymbol> {
-        @Override
-        public boolean test(VarSymbol t) {
-            if ("int".equals(t.type.toString())) {
-                switch (t.name.toString().toLowerCase(Locale.ROOT)) {
-                    case "user":
-                    case "userid":
-                    case "userhandle":
-                    case "user_id":
-                        return true;
-                }
-            }
-            return false;
-        }
-    }
 }

errorprone/java/com/google/errorprone/bugpatterns/android/UidChecker.java

@@ -63,7 +63,7 @@ public final class UidChecker extends BugChecker implements MethodInvocationTree
         return Description.NO_MATCH;
     }
 
-    private static enum Flavor {
+    public static enum Flavor {
         UNKNOWN(null),
         PID(Pattern.compile("(^pid$|Pid$)")),
         UID(Pattern.compile("(^uid$|Uid$)")),
@@ -78,7 +78,7 @@ public final class UidChecker extends BugChecker implements MethodInvocationTree
         }
     }
 
-    private static Flavor getFlavor(String name) {
+    public static Flavor getFlavor(String name) {
         for (Flavor f : Flavor.values()) {
             if (f.matches(name)) {
                 return f;
@@ -87,7 +87,7 @@ public final class UidChecker extends BugChecker implements MethodInvocationTree
         return Flavor.UNKNOWN;
     }
 
-    private static Flavor getFlavor(VarSymbol symbol) {
+    public static Flavor getFlavor(VarSymbol symbol) {
         final String type = symbol.type.toString();
         if ("int".equals(type)) {
             return getFlavor(symbol.name.toString());
@@ -95,7 +95,7 @@ public final class UidChecker extends BugChecker implements MethodInvocationTree
         return Flavor.UNKNOWN;
     }
 
-    private static Flavor getFlavor(ExpressionTree tree) {
+    public static Flavor getFlavor(ExpressionTree tree) {
         if (tree instanceof IdentifierTree) {
             return getFlavor(((IdentifierTree) tree).getName().toString());
         } else if (tree instanceof MemberSelectTree) {

errorprone/tests/java/com/google/errorprone/bugpatterns/android/ContextUserIdCheckerTest.java

@@ -49,9 +49,16 @@ public class ContextUserIdCheckerTest {
                         "@SystemService(\"foo\") public class FooManager {",
                         "  Context mContext;",
                         "  IFooService mService;",
+                        "  final int mUserId;",
+                        "  FooManager(Context context) {",
+                        "    mUserId = mContext.getUserId();",
+                        "  }",
                         "  void bar() throws RemoteException {",
                         "    mService.baz(null, mContext.getUserId());",
                         "  }",
+                        "  void baz() throws RemoteException {",
+                        "    mService.baz(null, mUserId);",
+                        "  }",
                         "}")
                 .doTest();
     }
@@ -63,11 +70,13 @@ public class ContextUserIdCheckerTest {
                 .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("FooManager.java",
                         "import android.annotation.SystemService;",
                         "import android.content.Context;",
                         "import android.foo.IFooService;",
+                        "import android.os.UserHandle;",
                         "import android.os.RemoteException;",
                         "@SystemService(\"foo\") public class FooManager {",
                         "  Context mContext;",
@@ -76,6 +85,10 @@ public class ContextUserIdCheckerTest {
                         "    // BUG: Diagnostic contains:",
                         "    mService.baz(null, 0);",
                         "  }",
+                        "  void baz() throws RemoteException {",
+                        "    // BUG: Diagnostic contains:",
+                        "    mService.baz(null, UserHandle.myUserId());",
+                        "  }",
                         "}")
                 .doTest();
     }

errorprone/tests/res/android/os/UserHandle.java

@@ -28,4 +28,8 @@ public class UserHandle {
     public static int getUid(int userId, int appId) {
         throw new UnsupportedOperationException();
     }
+
+    public static int myUserId() {
+        throw new UnsupportedOperationException();
+    }
 }