Skip to content

Commit ab27a78

Browse files
committed
Fix what NullAway reports
Most of the 60 findings were a null the code meant, with no @nullable where it went. Now marked @nullable: - RawTok's and Tok's kind and string value, and a comment's text before it is first read; - the parent of the root exploration node; - the methods maybeGetMethod looks up, and the fields that keep them; - the receiver of an unqualified call, and a dimension list's missing type; - getLineEnding's result for a string without one; - the three scan overrides, which javac calls with null for a missing tree. Where a lookup cannot miss (a token at a position the input has, a range the output built, a level's children), checkNotNull says so and fails as the dereference did. Besides that: - variableFragmentDims no longer takes a missing type: its one caller that may have none maps the Optional instead. - The javadoc lexer's newline after a `}` it moves has the "\n" of every other newline token rather than null. The formatter writes a line break for it and never reads the value. - Newlines.LineIterator returns the line it reads instead of keeping it in a field. - BootstrappingFormatterService runs the formatter without a working directory, rather than throwing, if the java binary has no parent directory. - PalantirJavaFormatSettings.getState() is no longer @nullable: it never returns null. - ParameterizedClass uses JUnit 6's computeIfAbsent, which never returns null, in place of the deprecated getOrComputeIfAbsent. - FormatterIntegrationTest reads the expected output's line separator in the tests that use it. - NullAway takes AssertJ's assertThat(x).isNotNull() in tests for the null check it is.
1 parent 1d03bf9 commit ab27a78

25 files changed

Lines changed: 127 additions & 59 deletions

File tree

‎buildSrc/src/main/groovy/open-java-format.error-prone-conventions.gradle‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ tasks.withType(JavaCompile).configureEach {
2121
options.errorprone {
2222
// Annotation processor output (Immutables) is not ours to fix.
2323
excludedPaths = '.*/build/generated/.*'
24+
nullaway {
25+
handleTestAssertionLibraries = true
26+
}
2427
error('FormatStringConcatenation')
2528
error('JUnitClassModifiers')
2629
error('JUnitMethodDeclaration')

‎gradle-open-java-format/src/test/java/com/palantir/javaformat/gradle/NativeImageAtomicCopyTest.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@ void the_copy_is_executable_and_the_image_is_left_as_it_was() throws IOException
4545

4646
var copy = NativeImageAtomicCopy.copyToCache(image, gradleUserHome.toFile());
4747

48-
assertThat(copy.getParent().getParent()).isEqualTo(gradleUserHome.resolve("open-java-format-caches"));
49-
assertThat(copy.getParent().getFileName().toString()).matches("[0-9a-f]{16}");
48+
var cacheDir = copy.getParent();
49+
assertThat(cacheDir).isNotNull();
50+
assertThat(cacheDir.getParent()).isEqualTo(gradleUserHome.resolve("open-java-format-caches"));
51+
assertThat(cacheDir.getFileName().toString()).matches("[0-9a-f]{16}");
5052
assertThat(copy.getFileName()).isEqualTo(image.getFileName());
5153
assertThat(copy).hasContent("an image");
5254
assertThat(Files.isExecutable(copy)).isTrue();

‎open-java-format-idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatFormattingService.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ Optional<PluginDescriptor> getPluginDescriptor() {
6565
}
6666

6767
@Override
68+
@Nullable
6869
protected FormattingTask createFormattingTask(AsyncFormattingRequest request) {
6970
Project project = request.getContext().getProject();
7071
// The marketplace verifier reports this class as experimental, and newer IDEs also deprecate it. It stays:

‎open-java-format-idea-plugin/src/main/java/com/palantir/javaformat/intellij/PalantirJavaFormatSettings.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ static PalantirJavaFormatSettings getInstance(Project project) {
4949
}
5050

5151
@Override
52-
@Nullable
5352
public StoredSettings getState() {
5453
return state;
5554
}
@@ -164,6 +163,7 @@ public void setImplementationClassPath(@Nullable List<String> value) {
164163
.map(strings -> strings.stream().map(URI::create).collect(Collectors.toList()));
165164
}
166165

166+
@Nullable
167167
public List<String> getImplementationClassPath() {
168168
return implementationClassPath
169169
.map(paths -> paths.stream().map(URI::toString).collect(Collectors.toList()))
@@ -181,6 +181,7 @@ public void setEnabled(@Nullable String enabledStr) {
181181
}
182182
}
183183

184+
@Nullable
184185
public String getEnabled() {
185186
return switch (enabled) {
186187
case ENABLED -> "true";

‎open-java-format-idea-plugin/src/test/java/com/palantir/javaformat/intellij/PalantirJavaFormatFormattingServiceTest.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import java.nio.file.Paths;
4949
import java.util.List;
5050

51+
import static java.util.Objects.requireNonNull;
5152
import static org.assertj.core.api.Assertions.assertThat;
5253

5354
final class PalantirJavaFormatFormattingServiceTest {
@@ -126,10 +127,10 @@ private PsiFile createPsiFile(String path, String... contents) throws IOExceptio
126127

127128
private static final class DelegatingFormatter extends PalantirJavaFormatFormattingService {
128129
public DelegatingFormatter() {
129-
this.setPluginDescriptor(FormattingService.EP_NAME
130-
.findExtension(PalantirJavaFormatFormattingService.class)
131-
.getPluginDescriptor()
132-
.orElseThrow());
130+
this.setPluginDescriptor(
131+
requireNonNull(FormattingService.EP_NAME.findExtension(PalantirJavaFormatFormattingService.class))
132+
.getPluginDescriptor()
133+
.orElseThrow());
133134
}
134135

135136
private boolean invoked = false;
@@ -140,7 +141,7 @@ private boolean wasInvoked() {
140141

141142
@Override
142143
protected FormattingTask createFormattingTask(AsyncFormattingRequest request) {
143-
FormattingTask delegateTask = super.createFormattingTask(request);
144+
FormattingTask delegateTask = requireNonNull(super.createFormattingTask(request));
144145
return new FormattingTask() {
145146
@Override
146147
public boolean cancel() {

‎open-java-format-jdk-bootstrap/src/main/java/com/palantir/javaformat/bootstrap/BootstrappingFormatterService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ private ImmutableList<Replacement> getFormatReplacementsInternal(String input, C
8787
ranges.stream().map(RangeUtils::toStringRange).collect(Collectors.toList()));
8888

8989
Optional<String> output =
90-
FormatterCommandRunner.runWithStdin(command.toArgs(), input, Optional.of(jdkPath.getParent()));
90+
FormatterCommandRunner.runWithStdin(command.toArgs(), input, Optional.ofNullable(jdkPath.getParent()));
9191
if (output.isEmpty() || output.get().isEmpty()) {
9292
return ImmutableList.of();
9393
}

‎open-java-format/src/main/java/com/palantir/javaformat/Newlines.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.common.collect.Iterators;
2020
import java.util.Iterator;
2121
import java.util.NoSuchElementException;
22+
import org.jspecify.annotations.Nullable;
2223

2324
/** Platform-independent newline handling. */
2425
public class Newlines {
@@ -53,6 +54,7 @@ public static int hasNewlineAt(String input, int idx) {
5354
}
5455

5556
/** Returns the terminating line break in the input, or {@code null} if the input does not end in a break. */
57+
@Nullable
5658
public static String getLineEnding(String input) {
5759
for (String b : BREAKS) {
5860
if (input.endsWith(b)) {
@@ -154,7 +156,6 @@ public void remove() {
154156
private static final class LineIterator implements Iterator<String> {
155157

156158
int idx;
157-
String curr;
158159

159160
private final String input;
160161
private final Iterator<Integer> indices;
@@ -165,7 +166,7 @@ private LineIterator(String input) {
165166
idx = indices.next(); // read leading 0
166167
}
167168

168-
private void advance() {
169+
private String advance() {
169170
int last = idx;
170171
if (indices.hasNext()) {
171172
idx = indices.next();
@@ -175,7 +176,7 @@ private void advance() {
175176
} else {
176177
throw new NoSuchElementException();
177178
}
178-
curr = input.substring(last, idx);
179+
return input.substring(last, idx);
179180
}
180181

181182
@Override
@@ -185,8 +186,7 @@ public boolean hasNext() {
185186

186187
@Override
187188
public String next() {
188-
advance();
189-
return curr;
189+
return advance();
190190
}
191191

192192
@Override

‎open-java-format/src/main/java/com/palantir/javaformat/OpsBuilder.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
package com.palantir.javaformat;
1616

17+
import static com.google.common.base.Preconditions.checkNotNull;
18+
1719
import com.google.common.base.MoreObjects;
1820
import com.google.common.base.Preconditions;
1921
import com.google.common.collect.ArrayListMultimap;
@@ -49,14 +51,14 @@ public final class OpsBuilder {
4951

5052
/** Return the actual size of the AST node at position, including comments. */
5153
public int actualSize(int position, int length) {
52-
Input.Token startToken = input.getPositionTokenMap().get(position);
54+
Input.Token startToken = checkNotNull(input.getPositionTokenMap().get(position));
5355
int start = startToken.getTok().getPosition();
5456
for (Input.Tok tok : startToken.getToksBefore()) {
5557
if (tok.isComment()) {
5658
start = Math.min(start, tok.getPosition());
5759
}
5860
}
59-
Input.Token endToken = input.getPositionTokenMap().get(position + length - 1);
61+
Input.Token endToken = checkNotNull(input.getPositionTokenMap().get(position + length - 1));
6062
int end = endToken.getTok().getPosition() + endToken.getTok().length();
6163
for (Input.Tok tok : endToken.getToksAfter()) {
6264
if (tok.isComment()) {
@@ -68,7 +70,7 @@ public int actualSize(int position, int length) {
6870

6971
/** Return the start column of the token at {@code position}, including leading comments. */
7072
public Integer actualStartColumn(int position) {
71-
Input.Token startToken = input.getPositionTokenMap().get(position);
73+
Input.Token startToken = checkNotNull(input.getPositionTokenMap().get(position));
7274
int start = startToken.getTok().getPosition();
7375
int line0 = input.getLineNumber(start);
7476
for (Input.Tok tok : startToken.getToksBefore()) {

‎open-java-format/src/main/java/com/palantir/javaformat/doc/Obs.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.Optional;
2121
import java.util.OptionalInt;
2222
import java.util.function.Function;
23+
import org.jspecify.annotations.Nullable;
2324

2425
/**
2526
* These classes exist purely for observing the operation of {@link Doc#computeBreaks}, including all the alternative
@@ -170,7 +171,7 @@ private static class ExplorationNodeImpl extends HasUniqueId implements Explorat
170171
private final Optional<Level> parentLevel;
171172

172173
public ExplorationNodeImpl(
173-
LevelNodeImpl parent,
174+
@Nullable LevelNodeImpl parent,
174175
String humanDescription,
175176
Sink sink,
176177
int startColumn,

‎open-java-format/src/main/java/com/palantir/javaformat/java/CommandLineOptionsParser.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ static CommandLineOptions parse(Iterable<String> options) {
107107
return optionsBuilder.build();
108108
}
109109

110-
private static Integer parseInteger(Iterator<String> it, String flag, String value) {
110+
private static Integer parseInteger(Iterator<String> it, String flag, @Nullable String value) {
111111
try {
112112
return Integer.valueOf(getValue(flag, it, value));
113113
} catch (NumberFormatException e) {

0 commit comments

Comments
 (0)