[abashev]

Error Prone has run on every compile, but nothing it reported failed the build except two JUnit checks, and the warnings scrolled past. This is the first step towards making it fail the build: the suppressions go, nullness moves to JSpecify, and eleven checks become errors once their findings are fixed.

• @SuppressWarnings. 105 of the 110 are gone. They hid almost nothing. 94 named their checks with palantir-baseline's for-rollout: prefix, which plain Error Prone does not take for a check's name, so NullAway and the rest reported those lines all along. Others named checks this build does not run (TooManyArguments, ImmutablesStyle), IntelliJ inspections, or javac's unchecked lint, which is off. Removing UnicodeEscape's brings back its one finding.
• Immutable stays suppressed. The five suppressions on Doc's and Level's memoized suppliers remain. Immutable is an error by default, and the check is right: Suppliers.memoize caches its value, so Break, Token and NonBreakingSpace, which are @Immutable and extend Doc, are not immutable. Fixing that is a change of its own.
• Four checks are errors, after their findings are fixed. Each one has its own commit, and every commit builds. All the checks that fail the build are now set in one block; the ones that were only listed as comments for later are gone.
  • UnusedVariable: ExecutableTransform's logger, StringWrapper's endLine, and the stderr that two MainTest tests read after the process had exited.
  • SameNameButDifferent: in PalantirJavaFormatSettings, State named both IntelliJ's @State and the settings bean. The bean is now StoredSettings. Its class name is not stored in .idea/open-java-format.xml, and PalantirJavaFormatSettingsTest loads files written before.
  • RefactorSwitch: four switch statements that returned from every case are now the switch expressions they return.
  • FormatStringConcatenation: seven calls in the tests pass a format string and its arguments to AssertJ's describedAs, hasMessage and isEqualTo and to Guava's checkState, instead of a concatenated string. In JavaCommentsHelperTest the expected comment stays written out like its input, and the check is suppressed on that test.
• JSpecify. org.jspecify.annotations.Nullable replaces jsr305's javax.annotation.Nullable, and the jspecify dependency (1.0.1, the version Guava brings) replaces jsr305. JetBrains' @NotNull goes from the IntelliJ plugin, and Obs's jsr305 @CheckReturnValue is now Error Prone's. CLAUDE.md says where the type-use annotation goes on nested and array types.
• NullAway is an error.
  • It runs in JSpecify mode. Outside that mode NullAway took Guava's @ParametricNullness for @Nullable, so every token read from a PeekingIterator<Token> counted as possibly null. That made 49 of its 102 findings; the mode reports 7 others that were real, which leaves 60. On javac 21 the mode needs -XDaddTypeAnnotationsToSymbol=true to see @Nullable on classes read from jars.
  • Most of the 60 real findings were a null the code meant, with no @Nullable where it went. A lookup that cannot miss now goes through checkNotNull, which fails as the dereference did. The commit message lists the few changes beyond annotations, such as the javadoc lexer's one null-valued newline token and JUnit 6's computeIfAbsent.
• Three of Picnic's suggestions are errors, after their findings are fixed:
  • RedundantStringConversion: six toString() calls on values that concatenation or Path.of converts anyway.
  • CanonicalAnnotationSyntax: braces around a single annotation value, and a trailing comma in an annotation array.
  • DirectReturn: two methods that put their last value in a variable only to return it.
• CollectorMutability is an error. The ten Collectors.toList() calls are Stream.toList(), as CLAUDE.md asks of new code, and StringWrapper's Collectors.toMap is toUnmodifiableMap. Nothing changed these collections after they were built, and none holds a null. A new test round-trips the IntelliJ settings' classpath, the one list that goes through a serializer.
• StringConcatToTextBlock is an error. The Java sources that DiagnosticTest and FormatterTest feed the formatter, strings joined by + or Joiner, are text blocks.
• EffectivelyPrivate is an error. Twelve members of private nested classes lose a public or protected that reached no further than the file.
• Four of Picnic's suggestions are off:
  • LexicographicalAnnotationListing wants annotations in alphabetical order. Its 25 suggestions would move @Override and @Test below the other annotations.
  • LexicographicalAnnotationAttributeListing would sort annotation arrays, {false, true} in a dozen @ValueSource tests and a @CsvSource's rows.
  • StringJoining would replace String.format("%s:%s", a, b) with String.join and two String.valueOf calls.
  • IsInstanceLambdaUsage would replace doc -> doc instanceof Level with Level.class::isInstance.
• UTF_8 is imported statically, as most of the code already had it: 36 uses of StandardCharsets.UTF_8.

Tests

./gradlew test passes on JDK 21: 1627 tests, 14 skipped. With an unused local added to CommandLineOptionsParser, :open-java-format:compileJava fails with error: [UnusedVariable]. With ModifierOrderer.compareTo dereferencing its @Nullable field again, it fails with error: [NullAway].

Not here

30 warnings remain: ConstantNaming 4, and 1 to 3 each for 17 more checks. There are also 49 suggestions: StaticImport 47 and NonStaticImport 2. Those checks, Immutable included, follow one at a time.