Automatic code cleanup.

PiperOrigin-RevId: 551640093

core/src/main/java/com/google/errorprone/bugpatterns/NullableOptional.java

@@ -0,0 +1,81 @@
+/*
+ * Copyright 2023 The Error Prone Authors.
+ *
+ * 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.google.errorprone.bugpatterns;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.BugPattern.SeverityLevel;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
+import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.predicates.TypePredicate;
+import com.google.errorprone.predicates.TypePredicates;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.ModifiersTree;
+import com.sun.source.tree.VariableTree;
+import com.sun.tools.javac.code.Type;
+
+/**
+ * A Checker that catches {@link java.util.Optional}/{@link com.google.common.base.Optional} with
+ * {@code Nullable} annotation.
+ */
+@BugPattern(
+    summary =
+        "Using an Optional variable which is expected to possibly be null is discouraged. It is"
+            + " best to indicate the absence of the value by assigning it an empty optional.",
+    severity = SeverityLevel.WARNING)
+public final class NullableOptional extends BugChecker
+    implements MethodTreeMatcher, VariableTreeMatcher {
+  private static final TypePredicate IS_OPTIONAL_TYPE =
+      TypePredicates.isExactTypeAny(
+          ImmutableSet.of(
+              java.util.Optional.class.getCanonicalName(),
+              com.google.common.base.Optional.class.getCanonicalName()));
+
+  @Override
+  public Description matchMethod(MethodTree tree, VisitorState state) {
+    if (hasNullableAnnotation(tree.getModifiers())
+        && isOptional(ASTHelpers.getType(tree.getReturnType()), state)) {
+      return describeMatch(tree);
+    }
+    return Description.NO_MATCH;
+  }
+
+  @Override
+  public Description matchVariable(VariableTree tree, VisitorState state) {
+    if (hasNullableAnnotation(tree.getModifiers()) && isOptional(ASTHelpers.getType(tree), state)) {
+      return describeMatch(tree);
+    }
+    return Description.NO_MATCH;
+  }
+
+  /** Check if the input ModifiersTree has any kind of "Nullable" annotation. */
+  private static boolean hasNullableAnnotation(ModifiersTree modifiersTree) {
+    return ASTHelpers.getAnnotationWithSimpleName(modifiersTree.getAnnotations(), "Nullable")
+        != null;
+  }
+
+  /**
+   * Check if the input Type is either {@link java.util.Optional} or{@link
+   * com.google.common.base.Optional}.
+   */
+  private static boolean isOptional(Type type, VisitorState state) {
+    return IS_OPTIONAL_TYPE.apply(type, state);
+  }
+}

core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java

@@ -271,6 +271,7 @@
 import com.google.errorprone.bugpatterns.NullTernary;
 import com.google.errorprone.bugpatterns.NullableConstructor;
 import com.google.errorprone.bugpatterns.NullableOnContainingClass;
+import com.google.errorprone.bugpatterns.NullableOptional;
 import com.google.errorprone.bugpatterns.NullablePrimitive;
 import com.google.errorprone.bugpatterns.NullablePrimitiveArray;
 import com.google.errorprone.bugpatterns.NullableVoid;
@@ -979,6 +980,7 @@ public static ScannerSupplier warningChecks() {
           NotJavadoc.class,
           NullOptional.class,
           NullableConstructor.class,
+          NullableOptional.class,
           NullablePrimitive.class,
           NullablePrimitiveArray.class,
           NullableVoid.class,

core/src/test/java/com/google/errorprone/bugpatterns/NullableOptionalTest.java

@@ -0,0 +1,160 @@
+/*
+ * Copyright 2023 The Error Prone Authors.
+ *
+ * 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.google.errorprone.bugpatterns;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public final class NullableOptionalTest {
+
+  private final CompilationTestHelper compilationHelper =
+      CompilationTestHelper.newInstance(NullableOptional.class, getClass());
+
+  @Test
+  public void optionalFieldWithNullableAnnotation_showsError() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.util.Optional;",
+            "import javax.annotation.Nullable;",
+            "final class Test {",
+            " // BUG: Diagnostic contains:",
+            " @Nullable private Optional<Object> foo;",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void guavaOptionalFieldWithNullableAnnotation_showsError() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import com.google.common.base.Optional;",
+            "import javax.annotation.Nullable;",
+            "final class Test {",
+            " @Nullable",
+            " // BUG: Diagnostic contains:",
+            " private Optional<Object> foo;",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodReturnsOptionalWithNullableAnnotation_showsError() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.util.Optional;",
+            "import javax.annotation.Nullable;",
+            "final class Test {",
+            " @Nullable",
+            " // BUG: Diagnostic contains:",
+            " private Optional<Object> foo() {",
+            "   return Optional.empty();",
+            " }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodReturnsOptionalWithAnotherNullableAnnotation_showsError() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.util.Optional;",
+            "import org.jspecify.nullness.Nullable;",
+            "final class Test {",
+            " @Nullable",
+            " // BUG: Diagnostic contains:",
+            " private Optional<Object> foo() {",
+            "   return Optional.empty();",
+            " }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodHasNullableOptionalAsParameter_showsError() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.util.Optional;",
+            "import javax.annotation.Nullable;",
+            "final class Test {",
+            " // BUG: Diagnostic contains:",
+            " private void foo(@Nullable Optional<Object> optional) {}",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void objectFieldWithNullableAnnotation_noError() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import javax.annotation.Nullable;",
+            "final class Test {",
+            " @Nullable Object field;",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodReturnsNonOptionalWithNullableAnnotation_noError() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import javax.annotation.Nullable;",
+            "final class Test {",
+            " @Nullable",
+            " private Object foo() {",
+            "   return null;",
+            " }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodReturnsNonOptionalWithAnotherNullableAnnotation_noError() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import org.jspecify.nullness.Nullable;",
+            "final class Test {",
+            " @Nullable",
+            " private Object foo() {",
+            "   return null;",
+            " }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void methodHasNullableNonOptionalAsParameter_noError() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import org.jspecify.nullness.Nullable;",
+            "final class Test {",
+            " private void foo(@Nullable Object object) {}",
+            "}")
+        .doTest();
+  }
+}

docs/bugpattern/NullableOptional.md

@@ -0,0 +1,9 @@
+`Optional` is a container object which may or may not contain a value. The
+presence or absence of the contained value should be demonstrated by the
+`Optional` object itself.
+
+Using an Optional variable which is expected to possibly be null is discouraged.
+An nullable Optional which uses `null` to indicate the absence of the value will
+lead to extra work for `null` checking when using the object and even cause
+exceptions such as `NullPointerException`. It is best to indicate the absence of
+the value by assigning it an empty optional.