Run ReturnMissingNullable over ErrorProne.

PiperOrigin-RevId: 458895205
google · Jul 4, 2022 · 66ec4fd · 66ec4fd
1 parent abf2681
commit 66ec4fd
Show file tree

Hide file tree

Showing 42 changed files with 99 additions and 33 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
@@ -1752,6 +1752,7 @@ public Type visitAnnotation(AnnotationTree tree, Void unused) {
       return null;
     }
 
+    @Nullable
     @Override
     public Type visitCase(CaseTree tree, Void unused) {
       Tree t = parent.getParentPath().getLeaf();
@@ -1856,6 +1857,7 @@ public Type visitReturn(ReturnTree tree, Void unused) {
       throw new AssertionError("return not enclosed by method or lambda");
     }
 
+    @Nullable
     @Override
     public Type visitSynchronized(SynchronizedTree node, Void unused) {
       // The null occurs if you've asked for the type of the parentheses around the expression.

diff --git a/check_api/src/main/java/com/google/errorprone/util/ErrorProneScope.java b/check_api/src/main/java/com/google/errorprone/util/ErrorProneScope.java
@@ -27,6 +27,7 @@
 import java.lang.reflect.Proxy;
 import java.util.Arrays;
 import java.util.function.Predicate;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /** A compatibility wrapper around {@code com.sun.tools.javac.util.Filter} */
 public final class ErrorProneScope {
@@ -59,7 +60,7 @@ public boolean anyMatch(Predicate<Symbol> predicate) {
 
   private static final Class<?> FILTER_CLASS = getFilterClass();
 
-  private static Class<?> getFilterClass() {
+  private static @Nullable Class<?> getFilterClass() {
     if (RuntimeVersion.isAtLeast17()) {
       return null;
     }

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractJUnit4InitMethodNotRun.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractJUnit4InitMethodNotRun.java
@@ -37,6 +37,7 @@
 import java.io.Serializable;
 import java.util.List;
 import javax.lang.model.element.Modifier;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * Base class for JUnit4SetUp/TearDown not run. This will take care of the nitty-gritty about
@@ -139,7 +140,7 @@ private static void makeProtectedPublic(
     }
   }
 
-  private Description tryToReplaceAnnotation(
+  private @Nullable Description tryToReplaceAnnotation(
       MethodTree methodTree, VisitorState state, String badAnnotation, String goodAnnotation) {
     String finalName = getUnqualifiedClassName(goodAnnotation);
     if (hasAnnotation(badAnnotation).matches(methodTree, state)) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AlwaysThrows.java b/core/src/main/java/com/google/errorprone/bugpatterns/AlwaysThrows.java
@@ -48,6 +48,7 @@
 import java.lang.reflect.InvocationTargetException;
 import java.util.UUID;
 import java.util.function.Consumer;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
 @BugPattern(summary = "Detects calls that will fail at runtime", severity = ERROR)
@@ -219,7 +220,7 @@ private Description checkImmutableMapOf(
     return checkForRepeatedKeys(tree, keys);
   }
 
-  private Object getConstantKey(ExpressionTree key, VisitorState state) {
+  private @Nullable Object getConstantKey(ExpressionTree key, VisitorState state) {
     return constantExpressions.constantExpression(key, state).orElse(null);
   }
 

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ComplexBooleanConstant.java b/core/src/main/java/com/google/errorprone/bugpatterns/ComplexBooleanConstant.java
@@ -29,6 +29,7 @@
 import com.sun.source.util.SimpleTreeVisitor;
 import com.sun.tools.javac.tree.JCTree.JCLiteral;
 import java.util.Objects;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * @author Sumit Bhagwani (bhagwani@google.com)
@@ -53,7 +54,7 @@ public Description matchBinary(BinaryTree tree, VisitorState state) {
         .build();
   }
 
-  Boolean booleanValue(BinaryTree tree) {
+  @Nullable Boolean booleanValue(BinaryTree tree) {
     if (tree.getLeftOperand() instanceof JCLiteral && tree.getRightOperand() instanceof JCLiteral) {
       return ASTHelpers.constValue(tree, Boolean.class);
     }
@@ -62,15 +63,15 @@ Boolean booleanValue(BinaryTree tree) {
     SimpleTreeVisitor<Boolean, Void> boolValue =
         new SimpleTreeVisitor<Boolean, Void>() {
           @Override
-          public Boolean visitLiteral(LiteralTree node, Void unused) {
+          public @Nullable Boolean visitLiteral(LiteralTree node, Void unused) {
             if (node.getValue() instanceof Boolean) {
               return (Boolean) node.getValue();
             }
             return null;
           }
 
           @Override
-          public Boolean visitUnary(UnaryTree node, Void unused) {
+          public @Nullable Boolean visitUnary(UnaryTree node, Void unused) {
             Boolean r = node.getExpression().accept(this, null);
             if (r == null) {
               return null;

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ConstantOverflow.java b/core/src/main/java/com/google/errorprone/bugpatterns/ConstantOverflow.java
@@ -110,6 +110,7 @@ private static Fix longFix(ExpressionTree expr, VisitorState state) {
   private static final SimpleTreeVisitor<Number, Void> CONSTANT_VISITOR =
       new SimpleTreeVisitor<Number, Void>() {
 
+        @Nullable
         @Override
         public Number visitConditionalExpression(ConditionalExpressionTree node, Void p) {
           Number ifTrue = node.getTrueExpression().accept(this, null);
@@ -126,6 +127,7 @@ public Number visitParenthesized(ParenthesizedTree node, Void p) {
           return node.getExpression().accept(this, null);
         }
 
+        @Nullable
         @Override
         public Number visitUnary(UnaryTree node, Void p) {
           Number value = node.getExpression().accept(this, null);
@@ -139,6 +141,7 @@ public Number visitUnary(UnaryTree node, Void p) {
           }
         }
 
+        @Nullable
         @Override
         public Number visitBinary(BinaryTree node, Void p) {
           Number lhs = node.getLeftOperand().accept(this, null);
@@ -170,6 +173,7 @@ public Number visitBinary(BinaryTree node, Void p) {
           }
         }
 
+        @Nullable
         @Override
         public Number visitTypeCast(TypeCastTree node, Void p) {
           Number value = node.getExpression().accept(this, null);
@@ -199,6 +203,7 @@ public Number visitLiteral(LiteralTree node, Void unused) {
         }
       };
 
+  @Nullable
   private static Long unop(Kind kind, long value) {
     switch (kind) {
       case UNARY_PLUS:
@@ -212,6 +217,7 @@ private static Long unop(Kind kind, long value) {
     }
   }
 
+  @Nullable
   private static Integer unop(Kind kind, int value) {
     switch (kind) {
       case UNARY_PLUS:
@@ -225,6 +231,7 @@ private static Integer unop(Kind kind, int value) {
     }
   }
 
+  @Nullable
   static Long binop(Kind kind, long lhs, long rhs) {
     switch (kind) {
       case MULTIPLY:
@@ -254,6 +261,7 @@ static Long binop(Kind kind, long lhs, long rhs) {
     }
   }
 
+  @Nullable
   static Integer binop(Kind kind, int lhs, int rhs) {
     switch (kind) {
       case MULTIPLY:
@@ -283,6 +291,7 @@ static Integer binop(Kind kind, int lhs, int rhs) {
     }
   }
 
+  @Nullable
   private static Number cast(TypeKind kind, Number value) {
     switch (kind) {
       case SHORT:
@@ -300,6 +309,7 @@ private static Number cast(TypeKind kind, Number value) {
     }
   }
 
+  @Nullable
   private static Number getIntegralConstant(Tree node) {
     Number number = ASTHelpers.constValue(node, Number.class);
     if (number instanceof Integer || number instanceof Long) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ConstantPatternCompile.java b/core/src/main/java/com/google/errorprone/bugpatterns/ConstantPatternCompile.java
@@ -53,6 +53,7 @@
 import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.Modifier;
 import javax.lang.model.element.NestingKind;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * Flags variables initialized with {@link java.util.regex.Pattern#compile(String)} calls that could
@@ -206,7 +207,7 @@ public Void visitMemberSelect(MemberSelectTree tree, Void unused) {
   }
 
   /** Infer a name when upgrading the {@code Pattern} local to a constant. */
-  private static String inferName(VariableTree tree, VisitorState state) {
+  private static @Nullable String inferName(VariableTree tree, VisitorState state) {
     String name;
     if ((name = fromName(tree)) != null) {
       return name;
@@ -221,7 +222,7 @@ private static String inferName(VariableTree tree, VisitorState state) {
   }
 
   /** Use the existing local variable's name, unless it's terrible. */
-  private static String fromName(VariableTree tree) {
+  private static @Nullable String fromName(VariableTree tree) {
     String name = LOWER_CAMEL.to(UPPER_UNDERSCORE, tree.getName().toString());
     if (name.length() > 1 && !name.equals("PATTERN")) {
       return name;
@@ -235,7 +236,7 @@ private static String fromName(VariableTree tree) {
    * <p>e.g. use {@code FOO_PATTERN} for {@code Pattern.compile(FOO)} and {@code
    * Pattern.compile(FOO_REGEX)}.
    */
-  private static String fromInitializer(VariableTree tree) {
+  private static @Nullable String fromInitializer(VariableTree tree) {
     ExpressionTree regex = ((MethodInvocationTree) tree.getInitializer()).getArguments().get(0);
     if (!(regex instanceof IdentifierTree)) {
       return null;
@@ -256,7 +257,7 @@ private static String fromInitializer(VariableTree tree) {
    * If the pattern is only used once in a call to {@code matcher}, and the argument is a local, use
    * that local's name. For example, infer {@code FOO_PATTERN} from {@code pattern.matcher(foo)}.
    */
-  private static String fromUse(VariableTree tree, VisitorState state) {
+  private static @Nullable String fromUse(VariableTree tree, VisitorState state) {
     VarSymbol sym = getSymbol(tree);
     ImmutableList.Builder<TreePath> usesBuilder = ImmutableList.builder();
     new TreePathScanner<Void, Void>() {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ForOverrideChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/ForOverrideChecker.java
@@ -205,6 +205,7 @@ private static ImmutableList<MethodSymbol> getOverriddenMethods(
   }
 
   /** Get the outermost class/interface/enum of an element, or null if none. */
+  @Nullable
   private static Type getOutermostClass(VisitorState state) {
     return findLast(
             stream(state.getPath())

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/InfiniteRecursion.java b/core/src/main/java/com/google/errorprone/bugpatterns/InfiniteRecursion.java
@@ -37,6 +37,7 @@
 import com.sun.source.tree.TypeCastTree;
 import com.sun.source.util.SimpleTreeVisitor;
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
 @BugPattern(
@@ -74,7 +75,7 @@ public MethodInvocationTree visitTypeCast(TypeCastTree tree, Void unused) {
               }
 
               @Override
-              protected MethodInvocationTree defaultAction(Tree tree, Void unused) {
+              protected @Nullable MethodInvocationTree defaultAction(Tree tree, Void unused) {
                 return tree instanceof MethodInvocationTree ? (MethodInvocationTree) tree : null;
               }
             },

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/IsInstanceIncompatibleType.java b/core/src/main/java/com/google/errorprone/bugpatterns/IsInstanceIncompatibleType.java
@@ -38,6 +38,7 @@
 import com.sun.source.tree.Tree;
 import com.sun.tools.javac.code.Type;
 import java.util.List;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * @author cushon@google.com (Liam Miller-Cushon)
@@ -84,7 +85,7 @@ public Description matchMemberReference(MemberReferenceTree tree, VisitorState s
         : buildMessage(argumentType, receiverType, tree, state);
   }
 
-  private static Type classTypeArgument(ExpressionTree tree) {
+  private static @Nullable Type classTypeArgument(ExpressionTree tree) {
     ExpressionTree receiver = getReceiver(tree);
     if (receiver == null) {
       return null;

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/JdkObsolete.java b/core/src/main/java/com/google/errorprone/bugpatterns/JdkObsolete.java
@@ -354,6 +354,7 @@ public Void visitIdentifier(IdentifierTree tree, Void unused) {
     return Optional.of(fix.build());
   }
 
+  @Nullable
   private static TreePath findEnclosingMethod(VisitorState state) {
     TreePath path = state.getPath();
     while (path != null) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/LockNotBeforeTry.java b/core/src/main/java/com/google/errorprone/bugpatterns/LockNotBeforeTry.java
@@ -39,6 +39,7 @@
 import com.sun.source.tree.TryTree;
 import com.sun.source.util.TreePath;
 import com.sun.source.util.TreeScanner;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * Suggests that calls to {@code Lock.lock} must be immediately followed by a {@code try-finally}
@@ -171,7 +172,7 @@ private static boolean releases(TryTree tryTree, ExpressionTree lockee, VisitorS
     Boolean released =
         new TreeScanner<Boolean, Void>() {
           @Override
-          public Boolean reduce(Boolean r1, Boolean r2) {
+          public @Nullable Boolean reduce(Boolean r1, Boolean r2) {
             return r1 == null ? r2 : (r2 == null ? null : r1 && r2);
           }
 

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ProtoFieldNullComparison.java b/core/src/main/java/com/google/errorprone/bugpatterns/ProtoFieldNullComparison.java
@@ -181,6 +181,7 @@ public Void visitVariable(VariableTree variable, Void unused) {
     private Optional<ExpressionTree> getInitializer(ExpressionTree tree) {
       return Optional.ofNullable(
           new SimpleTreeVisitor<ExpressionTree, Void>() {
+            @Nullable
             @Override
             public ExpressionTree visitMethodInvocation(MethodInvocationTree node, Void unused) {
               return PROTO_RECEIVER.matches(node, state) ? node : null;
@@ -290,6 +291,7 @@ private interface Fixer {
   private enum GetterTypes {
     /** {@code proto.getFoo()} */
     SCALAR {
+      @Nullable
       @Override
       Fixer match(ExpressionTree tree, VisitorState state) {
         if (tree.getKind() != Kind.METHOD_INVOCATION) {
@@ -335,6 +337,7 @@ private String replaceLast(String text, String pattern, String replacement) {
     },
     /** {@code proto.getRepeatedFoo(index)} */
     VECTOR_INDEXED {
+      @Nullable
       @Override
       Fixer match(ExpressionTree tree, VisitorState state) {
         if (tree.getKind() != Kind.METHOD_INVOCATION) {
@@ -365,6 +368,7 @@ private String generateFix(
     },
     /** {@code proto.getRepeatedFooList()} */
     VECTOR {
+      @Nullable
       @Override
       Fixer match(ExpressionTree tree, VisitorState state) {
         if (tree.getKind() != Kind.METHOD_INVOCATION) {
@@ -391,6 +395,7 @@ private String generateFix(
     },
     /** {@code proto.getField(f)} or {@code proto.getExtension(outer, extension)}; */
     EXTENSION_METHOD {
+      @Nullable
       @Override
       Fixer match(ExpressionTree tree, VisitorState state) {
         if (EXTENSION_METHODS_WITH_NO_FIX.matches(tree, state)) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ProtoRedundantSet.java b/core/src/main/java/com/google/errorprone/bugpatterns/ProtoRedundantSet.java
@@ -41,6 +41,7 @@
 import java.util.Collection;
 import java.util.Map;
 import java.util.regex.Pattern;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * Checks that protocol buffers built with chained builders don't set the same field twice.
@@ -167,7 +168,7 @@ interface ProtoField {}
   enum FieldType {
     SINGLE {
       @Override
-      FieldWithValue match(String name, MethodInvocationTree tree) {
+      @Nullable FieldWithValue match(String name, MethodInvocationTree tree) {
         if (name.startsWith("set") && tree.getArguments().size() == 1) {
           return FieldWithValue.of(SingleField.of(name), tree, tree.getArguments().get(0));
         }
@@ -176,7 +177,7 @@ FieldWithValue match(String name, MethodInvocationTree tree) {
     },
     REPEATED {
       @Override
-      FieldWithValue match(String name, MethodInvocationTree tree) {
+      @Nullable FieldWithValue match(String name, MethodInvocationTree tree) {
         if (name.startsWith("set") && tree.getArguments().size() == 2) {
           Integer index = ASTHelpers.constValue(tree.getArguments().get(0), Integer.class);
           if (index != null) {
@@ -189,7 +190,7 @@ FieldWithValue match(String name, MethodInvocationTree tree) {
     },
     MAP {
       @Override
-      FieldWithValue match(String name, MethodInvocationTree tree) {
+      @Nullable FieldWithValue match(String name, MethodInvocationTree tree) {
         if (name.startsWith("put") && tree.getArguments().size() == 2) {
           Object key = ASTHelpers.constValue(tree.getArguments().get(0), Object.class);
           if (key != null) {

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/SelfAssignment.java b/core/src/main/java/com/google/errorprone/bugpatterns/SelfAssignment.java
@@ -49,6 +49,7 @@
 import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Type;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * TODO(eaftan): Consider cases where the parent is not a statement or there is no parent?
@@ -122,7 +123,7 @@ public ExpressionTree visitTypeCast(TypeCastTree node, Void unused) {
       }
 
       @Override
-      protected ExpressionTree defaultAction(Tree node, Void unused) {
+      protected @Nullable ExpressionTree defaultAction(Tree node, Void unused) {
         return node instanceof ExpressionTree ? (ExpressionTree) node : null;
       }
     }.visit(expression, null);

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TruthAssertExpected.java b/core/src/main/java/com/google/errorprone/bugpatterns/TruthAssertExpected.java
@@ -132,6 +132,7 @@ public List<? extends ExpressionTree> visitNewClass(NewClassTree node, Void unus
                 return node.getArguments();
               }
 
+              @Nullable
               @Override
               public List<? extends ExpressionTree> visitMethodInvocation(
                   MethodInvocationTree node, Void unused) {