Added OptionalOrElseGetConstant error prone check. (#1401)

Recommends using optional.orElse(constant) instead of optional.orElseGet(() -> constant)

README.md

@@ -159,6 +159,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c
 - `NonComparableStreamSort`: Stream.sorted() should only be called on streams of Comparable types.
 - `DangerousStringInternUsage`: Disallow String.intern() invocations in favor of more predictable, scalable alternatives.
 - `OptionalOrElseThrowThrows`: Optional.orElseThrow argument must return an exception, not throw one.
+- `OptionalOrElseGetValue`: Prefer `Optional.orElse(value)` over `Optional.orElseGet(() -> value)` for trivial expressions.
 - `LambdaMethodReference`: Lambda should use a method reference.
 - `SafeLoggingExceptionMessageFormat`: SafeLoggable exceptions do not interpolate parameters.
 - `StrictUnusedVariable`: Functions shouldn't have unused parameters.

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/OptionalOrElseGetValue.java

@@ -0,0 +1,96 @@
+/*
+ * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved.
+ *
+ * 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.palantir.baseline.errorprone;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.BugPattern.SeverityLevel;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.fixes.SuggestedFixes;
+import com.google.errorprone.matchers.CompileTimeConstantExpressionMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.method.MethodMatchers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.LambdaExpressionTree;
+import com.sun.source.tree.LiteralTree;
+import com.sun.source.tree.MemberSelectTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.Tree;
+
+@AutoService(BugChecker.class)
+@BugPattern(
+        name = "OptionalOrElseGetValue",
+        link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
+        linkType = BugPattern.LinkType.CUSTOM,
+        providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION,
+        severity = SeverityLevel.WARNING,
+        summary = "If lambda passed to Optional#orElseGet returns a simple expression, use Optional#orElse instead")
+public final class OptionalOrElseGetValue extends BugChecker implements MethodInvocationTreeMatcher {
+
+    private static final long serialVersionUID = 1L;
+    private static final Matcher<ExpressionTree> OR_ELSE_GET_METHOD =
+            MethodMatchers.instanceMethod().onExactClass("java.util.Optional").named("orElseGet");
+    private static final Matcher<ExpressionTree> COMPILE_TIME_CONSTANT = new CompileTimeConstantExpressionMatcher();
+
+    @Override
+    public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+        if (!OR_ELSE_GET_METHOD.matches(tree, state)) {
+            return Description.NO_MATCH;
+        }
+
+        ExpressionTree orElseGetArg = tree.getArguments().get(0);
+
+        if (orElseGetArg.getKind() != Tree.Kind.LAMBDA_EXPRESSION) {
+            return Description.NO_MATCH;
+        }
+
+        LambdaExpressionTree lambdaExpressionTree = (LambdaExpressionTree) orElseGetArg;
+        LambdaExpressionTree.BodyKind bodyKind = lambdaExpressionTree.getBodyKind();
+
+        if (bodyKind != LambdaExpressionTree.BodyKind.EXPRESSION) {
+            return Description.NO_MATCH;
+        }
+
+        ExpressionTree expressionBody = (ExpressionTree) lambdaExpressionTree.getBody();
+        if (!COMPILE_TIME_CONSTANT.matches(expressionBody, state)
+                && !isTrivialExpression(expressionBody)
+                && !isTrivialSelect(expressionBody)) {
+            return Description.NO_MATCH;
+        }
+
+        return buildDescription(tree)
+                .setMessage("Prefer Optional#orElse instead of Optional#orElseGet for compile time constants")
+                .addFix(SuggestedFix.builder()
+                        .merge(SuggestedFixes.renameMethodInvocation(tree, "orElse", state))
+                        .replace(orElseGetArg, state.getSourceForNode(expressionBody))
+                        .build())
+                .build();
+    }
+
+    private static boolean isTrivialExpression(ExpressionTree tree) {
+        return tree instanceof LiteralTree || tree instanceof IdentifierTree;
+    }
+
+    private static boolean isTrivialSelect(ExpressionTree tree) {
+        return tree instanceof MemberSelectTree && isTrivialExpression(((MemberSelectTree) tree).getExpression());
+    }
+}

baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/OptionalOrElseGetValueTest.java

@@ -0,0 +1,150 @@
+/*
+ * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved.
+ *
+ * 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.palantir.baseline.errorprone;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public final class OptionalOrElseGetValueTest {
+
+    private CompilationTestHelper compilationHelper;
+    private RefactoringValidator refactoringTestHelper;
+
+    @BeforeEach
+    public void before() {
+        compilationHelper = CompilationTestHelper.newInstance(OptionalOrElseGetValue.class, getClass());
+        refactoringTestHelper = RefactoringValidator.of(new OptionalOrElseGetValue(), getClass());
+    }
+
+    @Test
+    public void testOrElseGetStringLiteral() {
+        compilationHelper
+                .addSourceLines(
+                        "Test.java",
+                        "import java.util.Optional;",
+                        "class Test {",
+                        "    // BUG: Diagnostic contains: Prefer Optional#orElse",
+                        "    private final String string = Optional.of(\"hello\").orElseGet(() -> \"world\");",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testOrElseGetVariable() {
+        compilationHelper
+                .addSourceLines(
+                        "Test.java",
+                        "import java.util.Optional;",
+                        "class Test {",
+                        "    String s = \"hello\";",
+                        "    // BUG: Diagnostic contains: Prefer Optional#orElse",
+                        "    private final String string = Optional.of(\"hello\").orElseGet(() -> s);",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testOrElseGetMethodInvocation() {
+        compilationHelper
+                .addSourceLines(
+                        "Test.java",
+                        "import java.util.Optional;",
+                        "class Test {",
+                        "    String s = \"hello\";",
+                        "    private final String string = Optional.of(\"hello\").orElseGet(() -> s.toString());",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testOrElseGetConstantString() {
+        compilationHelper
+                .addSourceLines(
+                        "Test.java",
+                        "import java.util.Optional;",
+                        "class Test {",
+                        "    private static final String constant = \"constant\";",
+                        "    // BUG: Diagnostic contains: Prefer Optional#orElse",
+                        "    private final String string = Optional.of(\"hello\").orElseGet(() -> constant);",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testOrElseGetThisField() {
+        compilationHelper
+                .addSourceLines(
+                        "Test.java",
+                        "import java.util.Optional;",
+                        "class Test {",
+                        "    private static double field = Math.random();",
+                        "    // BUG: Diagnostic contains: Prefer Optional#orElse",
+                        "    private final double string = Optional.of(0.1).orElseGet(() -> this.field);",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testOrElseGetOtherField() {
+        compilationHelper
+                .addSourceLines(
+                        "Test.java",
+                        "import java.util.Optional;",
+                        "class Test {",
+                        "    private Test test = new Test();",
+                        "    private double field = Math.random();",
+                        "    // BUG: Diagnostic contains: Prefer Optional#orElse",
+                        "    private final double string = Optional.of(0.1).orElseGet(() -> test.field);",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testOrElseGetCompileTimeConstant() {
+        compilationHelper
+                .addSourceLines(
+                        "Test.java",
+                        "import java.util.Optional;",
+                        "class Test {",
+                        "    private final String part1 = \"Hello \";",
+                        "    private final String part2 = \"World\";",
+                        "    // BUG: Diagnostic contains: Prefer Optional#orElse",
+                        "    private final String string = Optional.of(\"str\").orElseGet(() -> part1 + part2);",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testReplacementOfLiteral() {
+        refactoringTestHelper
+                .addInputLines(
+                        "Test.java",
+                        "import java.util.Optional;",
+                        "class Test {",
+                        "    private final boolean b = Optional.of(true).orElseGet(() -> false);",
+                        "}")
+                .addOutputLines(
+                        "Test.java",
+                        "import java.util.Optional;",
+                        "class Test {",
+                        "    private final boolean b = Optional.of(true).orElse(false);",
+                        "}")
+                .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
+    }
+}

changelog/@unreleased/pr-1401.v2.yml

@@ -0,0 +1,6 @@
+type: improvement
+improvement:
+  description: Adds `OptionalOrElseGetValue` error prone rule which recommends
+    using `Optional.orElse(value)` over `Optional.orElseGet(() -> value)`.
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/1401