Added OptionalOrElseGetConstant error prone check.

.circleci/config.yml

@@ -5,7 +5,7 @@
 version: 2.1
 jobs:
   compile:
-    docker: [{ image: 'circleci/openjdk:8u222-stretch-node' }]
+    docker: [{ image: 'circleci/openjdk:11-node' }]
     resource_class: large
     environment:
       CIRCLE_TEST_REPORTS: /home/circleci/junit
@@ -54,7 +54,7 @@ jobs:
           paths: [ project, .gradle/init.gradle ]
 
   check:
-    docker: [{ image: 'circleci/openjdk:8u222-stretch-node' }]
+    docker: [{ image: 'circleci/openjdk:11-node' }]
     environment:
       CIRCLE_TEST_REPORTS: /home/circleci/junit
       CIRCLE_ARTIFACTS: /home/circleci/artifacts
@@ -75,7 +75,7 @@ jobs:
       - store_artifacts: { path: ~/artifacts }
 
   unit-test:
-    docker: [{ image: 'circleci/openjdk:8u222-stretch-node' }]
+    docker: [{ image: 'circleci/openjdk:11-node' }]
     resource_class: large
     environment:
       CIRCLE_TEST_REPORTS: /home/circleci/junit
@@ -96,21 +96,29 @@ jobs:
       - store_test_results: { path: ~/junit }
       - store_artifacts: { path: ~/artifacts }
 
-  unit-test-11:
+
+  unit-test-14:
     docker: [{ image: 'circleci/openjdk:11-node' }]
     resource_class: large
     environment:
       CIRCLE_TEST_REPORTS: /home/circleci/junit
       CIRCLE_ARTIFACTS: /home/circleci/artifacts
       GRADLE_OPTS: -Dorg.gradle.jvmargs='-XX:MaxMetaspaceSize=256m' -Dorg.gradle.workers.max=4
       _JAVA_OPTIONS: -XX:ActiveProcessorCount=4 -Xmx1177m -XX:MaxMetaspaceSize=512m  -XX:ErrorFile=/home/circleci/artifacts/hs_err_pid%p.log -XX:HeapDumpPath=/home/circleci/artifacts
+      JAVA_HOME: /opt/java14
     steps:
       - checkout
+      - run:
+          name: Install Java
+          command: |
+            sudo mkdir -p /opt/java && cd /opt/java && sudo chown -R circleci:circleci .
+            curl https://cdn.azul.com/zulu/bin/zulu14.28.21-ca-jdk14.0.1-linux_x64.tar.gz | tar -xzf - -C /opt/java
+            sudo ln -s /opt/java/zulu*/ /opt/java14
       - restore_cache: { key: 'gradle-wrapper-v2-{{ checksum "gradle/wrapper/gradle-wrapper.properties" }}' }
-      - restore_cache: { key: 'unit-test-11-gradle-cache-v2-{{ checksum "versions.props" }}-{{ checksum "build.gradle" }}' }
+      - restore_cache: { key: 'unit-test-14-gradle-cache-v2-{{ checksum "versions.props" }}-{{ checksum "build.gradle" }}' }
       - run: ./gradlew --parallel --stacktrace --continue test -Pcom.palantir.baseline-error-prone.disable
       - save_cache:
-          key: 'unit-test-11-gradle-cache-v2-{{ checksum "versions.props" }}-{{ checksum "build.gradle" }}'
+          key: 'unit-test-14-gradle-cache-v2-{{ checksum "versions.props" }}-{{ checksum "build.gradle" }}'
           paths: [ ~/.gradle/caches ]
       - run:
           command: mkdir -p ~/junit && find . -type f -regex ".*/build/.*TEST.*xml" -exec cp --parents {} ~/junit/ \;
@@ -119,7 +127,7 @@ jobs:
       - store_artifacts: { path: ~/artifacts }
 
   trial-publish:
-    docker: [{ image: 'circleci/openjdk:8u222-stretch-node' }]
+    docker: [{ image: 'circleci/openjdk:11-node' }]
     environment:
       CIRCLE_TEST_REPORTS: /home/circleci/junit
       CIRCLE_ARTIFACTS: /home/circleci/artifacts
@@ -137,7 +145,7 @@ jobs:
       - store_artifacts: { path: ~/artifacts }
 
   publish:
-    docker: [{ image: 'circleci/openjdk:8u222-stretch-node' }]
+    docker: [{ image: 'circleci/openjdk:11-node' }]
     environment:
       CIRCLE_TEST_REPORTS: /home/circleci/junit
       CIRCLE_ARTIFACTS: /home/circleci/artifacts
@@ -173,7 +181,7 @@ workflows:
           requires: [ compile ]
           filters: { tags: { only: /.*/ } }
 
-      - unit-test-11:
+      - unit-test-14:
           filters: { tags: { only: /.*/ } }
 
       - check:
@@ -188,5 +196,5 @@ workflows:
           filters: { branches: { ignore: develop } }
 
       - publish:
-          requires: [ unit-test, unit-test-11, check, trial-publish ]
+          requires: [ unit-test, unit-test-14, check, trial-publish ]
           filters: { tags: { only: /.*/ }, branches: { only: develop } }

.circleci/template.sh

@@ -1,2 +1,3 @@
 #!/usr/bin/env bash
 export CIRCLECI_TEMPLATE=java-library-oss
+export ONLY_11=true

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/main/java/com/palantir/baseline/errorprone/StrictUnusedVariable.java

@@ -768,6 +768,14 @@ public Void visitLambdaExpression(LambdaExpressionTree node, Void unused) {
 
         @Override
         public Void visitMethod(MethodTree tree, Void unused) {
+            // From the perspective of an errorprone rule there are two standalone trees for a single `record`
+            // definition; A MethodTree which looks like a void function and a ClassTree which has the record fields.
+            //
+            // Its unclear why both trees are emitted, but we can identify and ignore a record's MethodTree by checking
+            // if it does not have any associated source.
+            if (state.getEndPosition(tree) < 0) {
+                return null;
+            }
             return isSuppressed(tree) ? null : super.visitMethod(tree, unused);
         }
     }

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);
+    }
+}