ImmutableChecker handles null types

`ASTHelpers#getType(ClassTree)` and `ASTHelpers#targetType(VisitorState)` both may return null, which ImmutableChecker did not previously handle safely, which could lead to `NullPointerException` in cases such as switch expressions. This PR handles these potential `null` results more safely. Fixes #3220 & #3225 Fixes #3267 COPYBARA_INTEGRATE_REVIEW=#3267 from schlosna:ds/3220 dd8f38f PiperOrigin-RevId: 466185256
google · Aug 8, 2022 · 7504736 · 7504736
1 parent ee76912
commit 7504736
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 8 deletions.
diff --git a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java
@@ -1764,14 +1764,30 @@ public Type visitAnnotation(AnnotationTree tree, Void unused) {
     @Nullable
     @Override
     public Type visitCase(CaseTree tree, Void unused) {
-      Tree t = parent.getParentPath().getLeaf();
-      // JDK 12+, t can be SwitchExpressionTree
-      if (t instanceof SwitchTree) {
-        SwitchTree switchTree = (SwitchTree) t;
-        return getType(switchTree.getExpression());
-      }
-      // TODO(b/176098078): When the ErrorProne project switches to JDK 12, we should check
-      // for SwitchExpressionTree.
+      Tree switchTree = parent.getParentPath().getLeaf();
+      return getType(getSwitchExpression(switchTree));
+    }
+
+    @Nullable
+    private static ExpressionTree getSwitchExpression(Tree tree) {
+      if (tree instanceof SwitchTree) {
+        return ((SwitchTree) tree).getExpression();
+      }
+      // Reflection is required for JDK < 12
+      try {
+        Class<?> switchExpression = Class.forName("com.sun.source.tree.SwitchExpressionTree");
+        Class<?> clazz = tree.getClass();
+        if (switchExpression.isAssignableFrom(clazz)) {
+          try {
+            Method method = clazz.getMethod("getExpression");
+            return (ExpressionTree) method.invoke(tree);
+          } catch (ReflectiveOperationException e) {
+            throw new LinkageError(e.getMessage(), e);
+          }
+        }
+      } catch (ClassNotFoundException e) {
+        // continue below
+      }
       return null;
     }
 

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/ImmutableCheckerTest.java
@@ -16,7 +16,10 @@
 
 package com.google.errorprone.bugpatterns.threadsafety;
 
+import static org.junit.Assume.assumeTrue;
+
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.java.version.RuntimeVersion;
 import com.google.errorprone.CompilationTestHelper;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.errorprone.annotations.Immutable;
@@ -2967,4 +2970,27 @@ public void anonymousClass_canCallSuperMethodOnNonImmutableSuperClass() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void switchExpressionsResultingInGenericTypes_doesNotThrow() {
+    assumeTrue(RuntimeVersion.isAtLeast14());
+    compilationHelper
+        .addSourceLines(
+            "Kind.java",
+            "import com.google.errorprone.annotations.Immutable;",
+            "@Immutable enum Kind { A, B; }")
+        .addSourceLines(
+            "Test.java",
+            "import java.util.Optional;",
+            "import java.util.function.Supplier;",
+            "class Test {",
+            "  Supplier<Optional<String>> test(Kind kind) {",
+            "    return switch (kind) {",
+            "      case A -> Optional::empty;",
+            "      case B -> () -> Optional.of(\"\");",
+            "    };",
+            "  }",
+            "}")
+        .doTest();
+  }
 }