Resolve assorted issues

PicnicSupermarket · Jun 5, 2022 · 606d9d9 · 606d9d9
1 parent 792027f
commit 606d9d9
Show file tree

Hide file tree

Showing 12 changed files with 32 additions and 20 deletions.
diff --git a/...e-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/AutowiredConstructorCheck.java b/...e-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/AutowiredConstructorCheck.java
@@ -8,6 +8,7 @@
 import static com.google.errorprone.matchers.Matchers.isType;
 
 import com.google.auto.service.AutoService;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
@@ -43,7 +44,7 @@ public Description matchClass(ClassTree tree, VisitorState state) {
       return Description.NO_MATCH;
     }
 
-    List<AnnotationTree> annotations =
+    ImmutableList<AnnotationTree> annotations =
         AUTOWIRED_ANNOTATION
             .multiMatchResult(Iterables.getOnlyElement(constructors), state)
             .matchingNodes();

diff --git a/...trib/src/main/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenationCheck.java b/...trib/src/main/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenationCheck.java
@@ -31,6 +31,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
+import javax.annotation.Nullable;
 
 /**
  * A {@link BugChecker} which flags string concatenations that produce a format string; in such
@@ -210,6 +211,7 @@ private static class ReplacementArgumentsConstructor
       this.formatSpecifier = formatSpecifier;
     }
 
+    @Nullable
     @Override
     public Void visitBinary(BinaryTree tree, VisitorState state) {
       if (tree.getKind() == Kind.PLUS && isStringTyped(tree, state)) {
@@ -222,11 +224,13 @@ public Void visitBinary(BinaryTree tree, VisitorState state) {
       return null;
     }
 
+    @Nullable
     @Override
     public Void visitParenthesized(ParenthesizedTree tree, VisitorState state) {
       return tree.getExpression().accept(this, state);
     }
 
+    @Nullable
     @Override
     protected Void defaultAction(Tree tree, VisitorState state) {
       appendExpression(tree);

diff --git a/...va/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListingCheck.java b/...va/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListingCheck.java
@@ -37,6 +37,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Stream;
+import javax.annotation.Nullable;
 
 /**
  * A {@link BugChecker} which flags annotation array listings which aren't sorted lexicographically.
@@ -171,18 +172,21 @@ private static ImmutableList<ImmutableList<String>> getStructure(
     ImmutableList.Builder<ImmutableList<String>> nodes = ImmutableList.builder();
 
     new TreeScanner<Void, Void>() {
+      @Nullable
       @Override
       public Void visitIdentifier(IdentifierTree node, Void ctx) {
         nodes.add(tokenize(node));
         return super.visitIdentifier(node, ctx);
       }
 
+      @Nullable
       @Override
       public Void visitLiteral(LiteralTree node, Void ctx) {
         nodes.add(tokenize(node));
         return super.visitLiteral(node, ctx);
       }
 
+      @Nullable
       @Override
       public Void visitPrimitiveType(PrimitiveTypeTree node, Void ctx) {
         nodes.add(tokenize(node));

diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterCheck.java
@@ -166,11 +166,12 @@ private static CodeTransformer createCompositeCodeTransformer(ErrorProneFlags fl
         flags
             .get(INCLUDED_TEMPLATES_PATTERN_FLAG)
             .map(Pattern::compile)
-            .map(nameFilter -> filterCodeTransformers(allTransformers, nameFilter))
+            .<ImmutableCollection<CodeTransformer>>map(
+                nameFilter -> filterCodeTransformers(allTransformers, nameFilter))
             .orElseGet(allTransformers::values));
   }
 
-  private static ImmutableCollection<CodeTransformer> filterCodeTransformers(
+  private static ImmutableList<CodeTransformer> filterCodeTransformers(
       ImmutableListMultimap<String, CodeTransformer> transformers, Pattern nameFilter) {
     return transformers.entries().stream()
         .filter(e -> nameFilter.matcher(e.getKey()).matches())

diff --git a/...trib/src/main/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceCheck.java b/...trib/src/main/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceCheck.java
@@ -75,7 +75,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
     return describeMatch(
         traceAnnotation,
         SuggestedFixes.updateAnnotationArgumentValues(
-                traceAnnotation, "dispatcher", ImmutableList.of("true"))
+                traceAnnotation, state, "dispatcher", ImmutableList.of("true"))
             .build());
   }
 

diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsageCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsageCheck.java
@@ -59,7 +59,7 @@ public final class TimeZoneUsageCheck extends BugChecker implements MethodInvoca
                   LocalDateTime.class.getName(),
                   LocalTime.class.getName())
               .named("now"),
-          staticMethod().onClassAny(Instant.class.getName()).named("now").withParameters());
+          staticMethod().onClassAny(Instant.class.getName()).named("now").withNoParameters());
 
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {

diff --git a/...rc/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListMultimapTemplates.java b/...rc/main/java/tech/picnic/errorprone/refastertemplates/ImmutableListMultimapTemplates.java
@@ -92,7 +92,7 @@ ImmutableListMultimap<K, V> after(K key, V value) {
    */
   static final class EntryToImmutableListMultimap<K, V> {
     @BeforeTemplate
-    ImmutableMultimap<K, V> before(Map.Entry<? extends K, ? extends V> entry) {
+    ImmutableListMultimap<K, V> before(Map.Entry<? extends K, ? extends V> entry) {
       return Refaster.anyOf(
           ImmutableListMultimap.<K, V>builder().put(entry).build(),
           Stream.of(entry)

diff --git a/.../main/java/tech/picnic/errorprone/refastertemplates/ImmutableSortedMultisetTemplates.java b/.../main/java/tech/picnic/errorprone/refastertemplates/ImmutableSortedMultisetTemplates.java
@@ -6,7 +6,6 @@
 import static java.util.stream.Collectors.collectingAndThen;
 import static java.util.stream.Collectors.toList;
 
-import com.google.common.collect.ImmutableMultiset;
 import com.google.common.collect.ImmutableSortedMultiset;
 import com.google.common.collect.Streams;
 import com.google.errorprone.refaster.Refaster;
@@ -93,22 +92,22 @@ ImmutableSortedMultiset<T> after() {
   // `reverseOrder`.) Worth the hassle?
   static final class IterableToImmutableSortedMultiset<T extends Comparable<? super T>> {
     @BeforeTemplate
-    ImmutableMultiset<T> before(T[] iterable) {
+    ImmutableSortedMultiset<T> before(T[] iterable) {
       return Refaster.anyOf(
           ImmutableSortedMultiset.<T>naturalOrder().add(iterable).build(),
           Arrays.stream(iterable).collect(toImmutableSortedMultiset(naturalOrder())));
     }
 
     @BeforeTemplate
-    ImmutableMultiset<T> before(Iterator<T> iterable) {
+    ImmutableSortedMultiset<T> before(Iterator<T> iterable) {
       return Refaster.anyOf(
           ImmutableSortedMultiset.copyOf(naturalOrder(), iterable),
           ImmutableSortedMultiset.<T>naturalOrder().addAll(iterable).build(),
           Streams.stream(iterable).collect(toImmutableSortedMultiset(naturalOrder())));
     }
 
     @BeforeTemplate
-    ImmutableMultiset<T> before(Iterable<T> iterable) {
+    ImmutableSortedMultiset<T> before(Iterable<T> iterable) {
       return Refaster.anyOf(
           ImmutableSortedMultiset.copyOf(naturalOrder(), iterable),
           ImmutableSortedMultiset.<T>naturalOrder().addAll(iterable).build(),

diff --git a/...b/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSortedSetTemplates.java b/...b/src/main/java/tech/picnic/errorprone/refastertemplates/ImmutableSortedSetTemplates.java
@@ -6,7 +6,6 @@
 import static java.util.stream.Collectors.collectingAndThen;
 import static java.util.stream.Collectors.toList;
 
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Streams;
 import com.google.errorprone.refaster.Refaster;
@@ -91,22 +90,22 @@ ImmutableSortedSet<T> after() {
   // `reverseOrder`.) Worth the hassle?
   static final class IterableToImmutableSortedSet<T extends Comparable<? super T>> {
     @BeforeTemplate
-    ImmutableSet<T> before(T[] iterable) {
+    ImmutableSortedSet<T> before(T[] iterable) {
       return Refaster.anyOf(
           ImmutableSortedSet.<T>naturalOrder().add(iterable).build(),
           Arrays.stream(iterable).collect(toImmutableSortedSet(naturalOrder())));
     }
 
     @BeforeTemplate
-    ImmutableSet<T> before(Iterator<T> iterable) {
+    ImmutableSortedSet<T> before(Iterator<T> iterable) {
       return Refaster.anyOf(
           ImmutableSortedSet.copyOf(naturalOrder(), iterable),
           ImmutableSortedSet.<T>naturalOrder().addAll(iterable).build(),
           Streams.stream(iterable).collect(toImmutableSortedSet(naturalOrder())));
     }
 
     @BeforeTemplate
-    ImmutableSet<T> before(Iterable<T> iterable) {
+    ImmutableSortedSet<T> before(Iterable<T> iterable) {
       return Refaster.anyOf(
           ImmutableSortedSet.copyOf(naturalOrder(), iterable),
           ImmutableSortedSet.<T>naturalOrder().addAll(iterable).build(),

diff --git a/.../src/test/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceCheckTest.java b/.../src/test/java/tech/picnic/errorprone/bugpatterns/ScheduledTransactionTraceCheckTest.java
@@ -4,8 +4,6 @@
 import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
 import com.google.errorprone.CompilationTestHelper;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.condition.DisabledForJreRange;
-import org.junit.jupiter.api.condition.JRE;
 
 public final class ScheduledTransactionTraceCheckTest {
   private final CompilationTestHelper compilationTestHelper =
@@ -45,10 +43,7 @@ void identification() {
         .doTest();
   }
 
-  // XXX: Enable this test for all JREs once https://github.com/google/error-prone/pull/2820 is
-  // merged and released.
   @Test
-  @DisabledForJreRange(min = JRE.JAVA_12)
   void replacement() {
     refactoringTestHelper
         .addInputLines(

diff --git a/pom.xml b/pom.xml
@@ -136,7 +136,7 @@
         <version.error-prone>${version.error-prone-orig}</version.error-prone>
         <version.error-prone-fork>v${version.error-prone-orig}-picnic-1</version.error-prone-fork>
         <version.error-prone-orig>2.13.1</version.error-prone-orig>
-        <version.error-prone-slf4j>0.1.4</version.error-prone-slf4j>
+        <version.error-prone-slf4j>0.1.11</version.error-prone-slf4j>
         <version.findbugs-format-string>3.0.0</version.findbugs-format-string>
         <version.guava-beta-checker>1.0</version.guava-beta-checker>
         <version.jdk>11</version.jdk>
@@ -1469,18 +1469,25 @@
                                     <!-- XXX: Enable this once we open-source
                                     this library. -->
                                     -Xep:BetaApi:OFF
+                                    <!-- XXX: Enable this check. -->
+                                    -Xep:BugPatternNaming:OFF
                                     <!-- We don't target JDK 7. -->
                                     -Xep:Java7ApiChecker:OFF
                                     <!-- We don't target JDK 8. -->
                                     -Xep:Java8ApiChecker:OFF
                                     <!-- We don't target Android. -->
                                     -Xep:StaticOrDefaultInterfaceMethod:OFF
+                                    <!-- We generally discourage `var` use. -->
+                                    -Xep:Varifier:OFF
                                     <!-- XXX: This check flags false positives.
                                     See https://github.com/google/error-prone/issues/2679. -->
                                     -Xep:VoidMissingNullable:OFF
+                                    -XepOpt:CheckReturnValue:CheckAllConstructors=true
+                                    -XepOpt:InlineMe:SkipInliningsWithComments=false
                                     -XepOpt:NullAway:AnnotatedPackages=tech.picnic
                                     -XepOpt:NullAway:AssertsEnabled=true
                                     -XepOpt:NullAway:CheckOptionalEmptiness=true
+                                    -XepOpt:Nullness:Conservative=false
                                     <!-- Append additional custom arguments. -->
                                     ${error-prone.patch-args}
                                     ${error-prone.self-check-args}

diff --git a/...rc/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompilerTaskListener.java b/...rc/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompilerTaskListener.java
@@ -29,6 +29,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import javax.annotation.Nullable;
 import javax.tools.FileObject;
 import javax.tools.JavaFileManager;
 import javax.tools.StandardLocation;
@@ -92,6 +93,7 @@ private ImmutableListMultimap<ClassTree, CodeTransformer> compileRefasterTemplat
     ListMultimap<ClassTree, CodeTransformer> rules = ArrayListMultimap.create();
     new TreeScanner<Void, Void>() {
       @Override
+      @Nullable
       public Void visitClass(ClassTree node, Void v) {
         rules.putAll(node, RefasterRuleBuilderScanner.extractRules(node, context));
         return super.visitClass(node, null);