Suggestions and tweaksg

PicnicSupermarket · Apr 26, 2023 · d61816d · d61816d
1 parent f9526ee
commit d61816d
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 36 deletions.
diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java
@@ -233,13 +233,13 @@ T after(Optional<T> o1, Optional<T> o2) {
   // XXX: Extend rule to all method invocations (with less than 2 arguments).
   static final class OptionalOrElseGet<T> {
     @BeforeTemplate
-    T before(Optional<T> o, @Matches(IsMethodInvocationWithTwoOrMoreArgs.class) T value) {
-      return o.orElse(value);
+    T before(Optional<T> optional, @Matches(IsMethodInvocationWithTwoOrMoreArgs.class) T value) {
+      return optional.orElse(value);
     }
 
     @AfterTemplate
-    T after(Optional<T> o, T value) {
-      return o.orElseGet(() -> value);
+    T after(Optional<T> optional, T value) {
+      return optional.orElseGet(() -> value);
     }
   }
 

diff --git a/...ntrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java b/...ntrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java
@@ -123,8 +123,9 @@ Optional<String> testOptionalMap() {
   ImmutableSet<String> testOptionalOrElseGet() {
     return ImmutableSet.of(
         Optional.of("foo").orElse("bar"),
-        Optional.of("foo").orElse(toString()),
-        Optional.of("foo").orElse(String.valueOf(true)),
-        Optional.of("foo").orElse(String.format("%s", "bar")));
+        Optional.of("baz").orElse(toString()),
+        Optional.of("qux").orElse(String.valueOf(true)),
+        Optional.of("quux").orElse(String.format("%s", "quuz")),
+        Optional.of("corge").orElse(String.format("%s", "grault", "garply")));
   }
 }
diff --git a/...trib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java b/...trib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestOutput.java
@@ -116,8 +116,9 @@ Optional<String> testOptionalMap() {
   ImmutableSet<String> testOptionalOrElseGet() {
     return ImmutableSet.of(
         Optional.of("foo").orElse("bar"),
-        Optional.of("foo").orElse(toString()),
-        Optional.of("foo").orElse(String.valueOf(true)),
-        Optional.of("foo").orElseGet(() -> String.format("%s", "bar")));
+        Optional.of("baz").orElse(toString()),
+        Optional.of("qux").orElse(String.valueOf(true)),
+        Optional.of("quux").orElseGet(() -> String.format("%s", "quuz")),
+        Optional.of("corge").orElseGet(() -> String.format("%s", "grault", "garply")));
   }
 }
diff --git a/...in/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java b/...in/java/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgs.java
@@ -1,8 +1,5 @@
 package tech.picnic.errorprone.refaster.matchers;
 
-import static com.google.errorprone.matchers.Matchers.anyMethod;
-import static com.sun.source.tree.Tree.Kind.METHOD_INVOCATION;
-
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.matchers.Matcher;
 import com.sun.source.tree.ExpressionTree;
@@ -17,11 +14,7 @@ public IsMethodInvocationWithTwoOrMoreArgs() {}
 
   @Override
   public boolean matches(ExpressionTree expressionTree, VisitorState state) {
-    if (expressionTree.getKind() == METHOD_INVOCATION) {
-      MethodInvocationTree methodInvocationTree = (MethodInvocationTree) expressionTree;
-      return anyMethod().matches(methodInvocationTree.getMethodSelect(), state)
-          && methodInvocationTree.getArguments().size() > 1;
-    }
-    return false;
+    return expressionTree instanceof MethodInvocationTree
+        && ((MethodInvocationTree) expressionTree).getArguments().size() > 1;
   }
 }
diff --git a/...ava/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java b/...ava/tech/picnic/errorprone/refaster/matchers/IsMethodInvocationWithTwoOrMoreArgsTest.java
@@ -10,23 +10,10 @@
 final class IsMethodInvocationWithTwoOrMoreArgsTest {
   @Test
   void matches() {
-    CompilationTestHelper.newInstance(
-            IsMethodInvocationWithTwoOrMoreArgsTest.MatcherTestChecker.class, getClass())
+    CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass())
         .addSourceLines(
             "A.java",
             "class A {",
-            "  String foo(String foo, String bar) {",
-            "    return foo + bar;",
-            "  }",
-            "",
-            "  String foo(String foo) {",
-            "    return foo;",
-            "  }",
-            "",
-            "  String foo() {",
-            "    return \"foo\";",
-            "  }",
-            "",
             "  Boolean negative1() {",
             "    return Boolean.TRUE;",
             "  }",
@@ -36,22 +23,31 @@ IsMethodInvocationWithTwoOrMoreArgsTest.MatcherTestChecker.class, getClass())
             "  }",
             "",
             "  String negative3() {",
-            "    return foo(\"foo\");",
+            "    return toString();",
             "  }",
             "",
             "  String negative4() {",
-            "    return foo();",
+            "    return String.valueOf(1);",
             "  }",
             "",
             "  String positive1() {",
             "    // BUG: Diagnostic contains:",
-            "    return foo(\"foo\", \"bar\");",
+            "    return m1(\"foo\", \"bar\");",
             "  }",
             "",
             "  String positive2() {",
             "    // BUG: Diagnostic contains:",
             "    return String.format(\"%s\", \"foo\");",
             "  }",
+            "",
+            "  String positive3() {",
+            "    // BUG: Diagnostic contains:",
+            "    return String.format(\"%s-%s\", \"foo\", \"bar\");",
+            "  }",
+            "",
+            "  private static String m1(String foo, String bar) {",
+            "    return foo + bar;",
+            "  }",
             "}")
         .doTest();
   }