Nullable switch expression support

uber · Apr 8, 2019 · e529f53 · e529f53
1 parent 8f06f38
commit e529f53
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 2 deletions.
diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java b/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java
@@ -100,6 +100,7 @@ public Description createErrorDescription(
         case RETURN_NULLABLE:
         case PASS_NULLABLE:
         case ASSIGN_FIELD_NULLABLE:
+        case SWITCH_EXPRESSION_NULLABLE:
           if (config.getCastToNonNullMethod() != null) {
             builder = addCastToNonNullFix(suggestTree, builder);
           } else {

diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java b/nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java
@@ -51,6 +51,7 @@ public enum MessageTypes {
     NULLABLE_VARARGS_UNSUPPORTED,
     ANNOTATION_VALUE_INVALID,
     CAST_TO_NONNULL_ARG_NONNULL,
-    GET_ON_EMPTY_OPTIONAL;
+    GET_ON_EMPTY_OPTIONAL,
+    SWITCH_EXPRESSION_NULLABLE
   }
 }
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
@@ -72,6 +72,7 @@
 import com.sun.source.tree.ParenthesizedTree;
 import com.sun.source.tree.ReturnTree;
 import com.sun.source.tree.StatementTree;
+import com.sun.source.tree.SwitchTree;
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.TryTree;
 import com.sun.source.tree.TypeCastTree;
@@ -167,7 +168,8 @@ public class NullAway extends BugChecker
         BugChecker.LambdaExpressionTreeMatcher,
         BugChecker.IdentifierTreeMatcher,
         BugChecker.MemberReferenceTreeMatcher,
-        BugChecker.CompoundAssignmentTreeMatcher {
+        BugChecker.CompoundAssignmentTreeMatcher,
+        BugChecker.SwitchTreeMatcher {
 
   static final String INITIALIZATION_CHECK_NAME = "NullAway.Init";
 
@@ -495,6 +497,29 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
     return Description.NO_MATCH;
   }
 
+  @Override
+  public Description matchSwitch(SwitchTree tree, VisitorState state) {
+    if (!matchWithinClass) {
+      return Description.NO_MATCH;
+    }
+
+    ExpressionTree switchExpression = tree.getExpression();
+    if (switchExpression instanceof ParenthesizedTree) {
+      switchExpression = ((ParenthesizedTree) switchExpression).getExpression();
+    }
+
+    if (mayBeNullExpr(state, switchExpression)) {
+      final String message = "switch expression " + switchExpression.toString() + " is @Nullable";
+      ErrorMessage errorMessage =
+          new ErrorMessage(MessageTypes.SWITCH_EXPRESSION_NULLABLE, message);
+
+      return errorBuilder.createErrorDescription(
+          errorMessage, switchExpression, buildDescription(switchExpression));
+    }
+
+    return Description.NO_MATCH;
+  }
+
   /**
    * checks that an overriding method does not override a {@code @Nullable} parameter with a
    * {@code @NonNull} parameter

diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayTest.java
@@ -966,6 +966,57 @@ public void supportObjectsIsNull() {
         .doTest();
   }
 
+  @Test
+  public void supportSwitchExpression() {
+    compilationHelper
+        .addSourceLines(
+            "TestPositive.java",
+            "package com.uber;",
+            "import javax.annotation.Nullable;",
+            "enum Level {",
+            " HIGH, MEDIUM, LOW }",
+            "class TestPositive {",
+            "   void foo(@Nullable Integer s) {",
+            "    // BUG: Diagnostic contains: switch expression s is @Nullable",
+            "    switch(s) {",
+            "      case 5: break;",
+            "    }",
+            "    String x = null;",
+            "    // BUG: Diagnostic contains: switch expression x is @Nullable",
+            "    switch(x) {",
+            "      default: break;",
+            "    }",
+            "    Level level = null;",
+            "    // BUG: Diagnostic contains: switch expression level is @Nullable",
+            "    switch (level) {",
+            "      default: break; }",
+            "    }",
+            "}")
+        .addSourceLines(
+            "TestNegative.java",
+            "package com.uber;",
+            "import javax.annotation.Nullable;",
+            "class TestNegative {",
+            "   void foo(Integer s, short y) {",
+            "    switch(s) {",
+            "      case 5: break;",
+            "    }",
+            "    String x = \"irrelevant\";",
+            "    switch(x) {",
+            "      default: break;",
+            "    }",
+            "    switch(y) {",
+            "      default: break;",
+            "    }",
+            "    Level level = Level.HIGH;",
+            "    switch (level) {",
+            "      default: break;",
+            "    }",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void defaultPermissiveOnUnannotated() {
     compilationHelper