Introduce Qodana analysis

Local run:
- Get the token: https://qodana.cloud/organizations/pW0Je/teams/3X0Qq
- Run the Docker image: https://www.jetbrains.com/help/qodana/qodana-jvm-community.html#docker-image-tab

.github/workflows/qodana.yml

@@ -0,0 +1,32 @@
+# Analyzes the code using JetBrains' Qodana.
+# XXX: Expand this text.
+name: Qodana analysis
+on:
+  pull_request:
+  push:
+    branches: [ master ]
+permissions:
+  contents: read
+jobs:
+  analyze:
+    runs-on: ubuntu-22.04
+    # XXX: Validate which of these is necessary.
+    #permissions:
+    #  checks: write
+    #  contents: write
+    #  pull-requests: write
+    steps:
+      - name: Check out code
+        uses: actions/checkout@v3.1.0
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+      - name: Perform Qodana analysis
+        uses: JetBrains/qodana-action@a8363b702c2d2d49a77620bcd10541686df21307 # v2023.3.0
+        env:
+          QODANA_TOKEN: ${{ secrets.QODANA_TOKEN }}
+      # XXX: Review whether this step is still necessary.
+      - name: Update "Code scanning" dashboard
+        uses: github/codeql-action/upload-sarif@v2.2.11
+        with:
+          sarif_file: ${{ runner.temp }}/qodana/results/qodana.sarif.json

documentation-support/src/main/java/tech/picnic/errorprone/documentation/BugPatternTestExtractor.java

@@ -2,6 +2,7 @@ package tech.picnic.errorprone.documentation;
 
 import static com.google.errorprone.matchers.Matchers.instanceMethod;
 import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
+import static java.util.Objects.requireNonNull;
 import static java.util.function.Predicate.not;
 
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
@@ -165,9 +166,13 @@ public final class BugPatternTestExtractor implements Extractor<TestCases> {
         /*
          * Retrieve the method invocation that contains the input source code. Note that this cast
          * is safe, because this code is guarded by an earlier call to `#getClassUnderTest(..)`,
-         * which ensures that `tree` is part of a longer method invocation chain.
+         * which ensures that `tree` is part of a longer method invocation chain. Additionally, as
+         * the invoked method is non-static and defined on a third-party class, a receiver must
+         * exist.
          */
-        MethodInvocationTree inputTree = (MethodInvocationTree) ASTHelpers.getReceiver(tree);
+        MethodInvocationTree inputTree =
+            (MethodInvocationTree)
+                requireNonNull(ASTHelpers.getReceiver(tree), "Input tree definition");
 
         String path = ASTHelpers.constValue(inputTree.getArguments().get(0), String.class);
         Optional<String> inputCode = getSourceCode(inputTree);
@@ -200,7 +205,7 @@ public final class BugPatternTestExtractor implements Extractor<TestCases> {
         if (value == null) {
           return Optional.empty();
         }
-        source.append(value).append('\n');
+        source.append(value).append(System.lineSeparator());
       }
 
       return Optional.of(source.toString());

documentation-support/src/main/java/tech/picnic/errorprone/documentation/Json.java

@@ -25,7 +25,9 @@ final class Json {
   static <T> T read(Path path, Class<T> clazz) {
     try {
       return OBJECT_MAPPER.readValue(path.toFile(), clazz);
-    } catch (IOException e) {
+    } catch (
+        @SuppressWarnings("OverlyBroadCatchBlock" /* No need to further disambiguate errors. */)
+        IOException e) {
       throw failure(e, "Failure reading from '%s'", path);
     }
   }
@@ -33,7 +35,9 @@ final class Json {
   static <T> void write(Path path, T object) {
     try {
       OBJECT_MAPPER.writeValue(path.toFile(), object);
-    } catch (IOException e) {
+    } catch (
+        @SuppressWarnings("OverlyBroadCatchBlock" /* No need to further disambiguate errors. */)
+        IOException e) {
       throw failure(e, "Failure writing to '%s'", path);
     }
   }

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclaration.java

@@ -73,13 +73,13 @@ public final class JUnitMethodDeclaration extends BugChecker implements MethodTr
         .ifPresent(fixBuilder::merge);
 
     if (isTestMethod) {
-      suggestTestMethodRenameIfApplicable(tree, fixBuilder, state);
+      trySuggestTestMethodRename(tree, fixBuilder, state);
     }
 
     return fixBuilder.isEmpty() ? Description.NO_MATCH : describeMatch(tree, fixBuilder.build());
   }
 
-  private void suggestTestMethodRenameIfApplicable(
+  private void trySuggestTestMethodRename(
       MethodTree tree, SuggestedFix.Builder fixBuilder, VisitorState state) {
     MethodSymbol symbol = ASTHelpers.getSymbol(tree);
     tryCanonicalizeMethodName(symbol)

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java

@@ -17,6 +17,7 @@ import static com.google.errorprone.matchers.Matchers.isSameType;
 import static com.google.errorprone.matchers.Matchers.methodHasParameters;
 import static com.google.errorprone.matchers.Matchers.staticMethod;
 import static com.google.errorprone.matchers.Matchers.toType;
+import static java.util.Objects.requireNonNull;
 import static java.util.function.Predicate.not;
 import static java.util.stream.Collectors.joining;
 import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
@@ -129,19 +130,21 @@ public final class JUnitValueSource extends BugChecker implements MethodTreeMatc
       return Description.NO_MATCH;
     }
 
-    Type parameterType = ASTHelpers.getType(Iterables.getOnlyElement(tree.getParameters()));
+    Type parameterType =
+        requireNonNull(
+            ASTHelpers.getType(Iterables.getOnlyElement(tree.getParameters())), "Parameter type");
 
     return findMethodSourceAnnotation(tree, state)
         .flatMap(
-            methodSourceAnnotation ->
-                getSoleLocalFactoryName(methodSourceAnnotation, tree)
+            annotation ->
+                getSoleLocalFactoryName(annotation, tree)
                     .filter(factory -> !hasSiblingReferencingValueFactory(tree, factory, state))
                     .flatMap(factory -> findSiblingWithName(tree, factory, state))
                     .flatMap(
                         factoryMethod ->
                             tryConstructValueSourceFix(
-                                parameterType, methodSourceAnnotation, factoryMethod, state))
-                    .map(fix -> describeMatch(methodSourceAnnotation, fix)))
+                                parameterType, annotation, factoryMethod, state))
+                    .map(fix -> describeMatch(annotation, fix)))
         .orElse(Description.NO_MATCH);
   }
 
@@ -173,7 +176,9 @@ public final class JUnitValueSource extends BugChecker implements MethodTreeMatc
 
   private static Optional<MethodTree> findMatchingSibling(
       MethodTree tree, Predicate<? super MethodTree> predicate, VisitorState state) {
-    return state.findEnclosing(ClassTree.class).getMembers().stream()
+    return requireNonNull(state.findEnclosing(ClassTree.class), "Enclosing class")
+        .getMembers()
+        .stream()
         .filter(MethodTree.class::isInstance)
         .map(MethodTree.class::cast)
         .filter(not(tree::equals))
@@ -202,7 +207,7 @@ public final class JUnitValueSource extends BugChecker implements MethodTreeMatc
     return getSingleReturnExpression(valueFactoryMethod)
         .flatMap(expression -> tryExtractValueSourceAttributeValue(expression, state))
         .map(
-            valueSourceAttributeValue -> {
+            attributeValue -> {
               SuggestedFix.Builder fix = SuggestedFix.builder();
               String valueSource =
                   SuggestedFixes.qualifyType(
@@ -211,9 +216,7 @@ public final class JUnitValueSource extends BugChecker implements MethodTreeMatc
                       methodSourceAnnotation,
                       String.format(
                           "@%s(%s = %s)",
-                          valueSource,
-                          toValueSourceAttributeName(parameterType),
-                          valueSourceAttributeValue))
+                          valueSource, toValueSourceAttributeName(parameterType), attributeValue))
                   .delete(valueFactoryMethod)
                   .build();
             });

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/StringJoin.java

@@ -100,7 +100,7 @@ public final class StringJoin extends BugChecker implements MethodInvocationTree
        * This `String#format` invocation performs a straightforward string conversion; use
        * `String#valueOf` instead.
        */
-      return trySuggestExplicitStringConversion(tree, state);
+      return trySuggestExplicitConversion(tree, state);
     }
 
     String separator = Iterables.getOnlyElement(innerSeparators);
@@ -121,8 +121,7 @@ public final class StringJoin extends BugChecker implements MethodInvocationTree
    * <p>If {@code arg} is already a string then the resultant conversion is vacuous. The {@link
    * IdentityConversion} check will subsequently drop it.
    */
-  private Description trySuggestExplicitStringConversion(
-      MethodInvocationTree tree, VisitorState state) {
+  private Description trySuggestExplicitConversion(MethodInvocationTree tree, VisitorState state) {
     ExpressionTree argument = tree.getArguments().get(1);
     if (isSubtype(ASTHelpers.getType(argument), FORMATTABLE_TYPE, state)) {
       /*

error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/SourceCode.java

@@ -1,7 +1,6 @@
 package tech.picnic.errorprone.bugpatterns.util;
 
 import static com.sun.tools.javac.parser.Tokens.TokenKind.RPAREN;
-import static com.sun.tools.javac.util.Position.NOPOS;
 import static java.util.stream.Collectors.joining;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -58,7 +57,7 @@ public final class SourceCode {
   public static SuggestedFix deleteWithTrailingWhitespace(Tree tree, VisitorState state) {
     CharSequence sourceCode = state.getSourceCode();
     int endPos = state.getEndPosition(tree);
-    if (sourceCode == null || endPos == NOPOS) {
+    if (sourceCode == null || endPos == Position.NOPOS) {
       /* We can't identify the trailing whitespace; delete just the tree. */
       return SuggestedFix.delete(tree);
     }

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StringRules.java

@@ -83,7 +83,7 @@ final class StringRules {
 
     @AfterTemplate
     @AlsoNegation
-    boolean after(String str) {
+    boolean after(@Nullable String str) {
       return Strings.isNullOrEmpty(str);
     }
   }
@@ -92,14 +92,14 @@ final class StringRules {
   // XXX: This is a special case of `TernaryOperatorOptionalNegativeFiltering`.
   static final class OptionalNonEmptyString {
     @BeforeTemplate
-    Optional<String> before(String str) {
+    Optional<String> before(@Nullable String str) {
       return Strings.isNullOrEmpty(str)
           ? Optional.empty()
           : Refaster.anyOf(Optional.of(str), Optional.ofNullable(str));
     }
 
     @AfterTemplate
-    Optional<String> after(String str) {
+    Optional<String> after(@Nullable String str) {
       return Optional.ofNullable(str).filter(not(String::isEmpty));
     }
   }

qodana.yaml

@@ -0,0 +1,51 @@
+version: 1.0
+linter: jetbrains/qodana-jvm-community:2023.3
+profile:
+  # XXX: Review which additional inspections to enable.
+  name: qodana.recommended
+exclude:
+  # Refaster rules are not analyzed, as this would yield many false positives:
+  # - Template methods are not meant to be executed; applying data flow
+  #   analysis to them yields incorrect conclusions.
+  # - Template parameters present arbitrary expressions; treating them as plain
+  #   parameters yields incorrect conclusions.
+  # - By their nature, `@BeforeTemplate` methods contain non-idiomatic or
+  #   inefficient expressions; pointing that out is generally redundant.
+  - name: All
+    paths:
+      - error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules
+  # XXX: Enable if/when it is possible to customize the set of permissible
+  # method name prefixes.
+  - name: BooleanMethodNameMustStartWithQuestion
+  # XXX: Enable if/when it is possible to exclude Google AutoValue constructs.
+  - name: ClassReferencesSubclass
+  # This inspection makes little sense, given that types are generally not
+  # fully qualified, outside import statements.
+  - name: ClassNamePrefixedWithPackageName
+  # XXX: Enable if/when it is possible to disable the name length constraint.
+  - name: FieldNamingConvention
+  # XXX: Enable this inspection once it doesn't report so many false-positives.
+  - name: HardcodedFileSeparators
+  # XXX: Enable if/when it is possible to disable the name length constraint.
+  - name: LocalVariableNamingConvention
+  # XXX: Enable if/when it is possible to disable the name length constraint.
+  - name: NewClassNamingConvention
+  # XXX: Enable if/when it is possible to exclude selected non-boolean types,
+  # such as `Matcher`, `Predicate` and `TypePredicate`.
+  - name: NonBooleanMethodNameMayNotStartWithQuestion
+  # XXX: Enable if/when it is possible to disable the name length constraint.
+  - name: ParameterNamingConvention
+  # Not all APIs benefit from an interface abstraction.
+  - name: PublicMethodNotExposedInInterface
+  # XXX: Enable if/when it is possible to customize the set of types for which
+  # not to fire. For example, we prefer consistent use of Guava's immutable
+  # collection types.
+  - name: TypeMayBeWeakened
+  # The nullary constructor of bug checkers is always defined explicitly,
+  # because we require all public members to be documented.
+  # XXX: Enable this inspection if/when it is possible to exclude constructors
+  # with Javadoc.
+  - name: UnnecessaryConstructor
+  # XXX: Review whether to enable this. See also `-Xep:BetaApi:OFF` in
+  # `pom.xml`.
+  - name: UnstableApiUsage

refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompilerTaskListener.java

@@ -123,6 +123,8 @@ final class RefasterRuleCompilerTaskListener implements TaskListener {
   }
 
   /** Merges two annotation mappings, preferring the second over the first in case of conflicts. */
+  @SuppressWarnings(
+      "MethodOnlyUsedFromInnerClass" /* Expanding the anonymous class is undesirable. */)
   private static ImmutableClassToInstanceMap<Annotation> merge(
       ImmutableClassToInstanceMap<Annotation> first,
       ImmutableClassToInstanceMap<Annotation> second) {

refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/ThrowsCheckedException.java

@@ -14,7 +14,10 @@ import java.util.Collection;
  * A matcher of functional interface expressions for which execution of the functional interface
  * method may throw a checked exception.
  */
-@SuppressWarnings("java:S2166" /* This type's name is suitable for a `Matcher`. */)
+@SuppressWarnings({
+  "java:S2166",
+  "NonExceptionNameEndsWithException"
+} /* This type's name is suitable for a `Matcher`. */)
 public final class ThrowsCheckedException implements Matcher<ExpressionTree> {
   private static final long serialVersionUID = 1L;
 

refaster-test-support/src/main/java/tech/picnic/errorprone/refaster/test/RefasterRuleCollectionTestCase.java

@@ -7,6 +7,7 @@ import com.google.common.collect.ImmutableSet;
  * *Input.java} and {@code *Output.java} pairs, demonstrating the expected result of applying the
  * associated Refaster rules. These classes are <em>resources</em> on the test classpath.
  */
+@SuppressWarnings("InterfaceNeverImplemented" /* Implemented by test code resources. */)
 public interface RefasterRuleCollectionTestCase {
   /**
    * In some test classes there are types and statically imported methods that are fully replaced by