Skip to content

Commit

Permalink
Migrate from JSR 305 to JSpecify (#181)
Browse files Browse the repository at this point in the history
JSpecify's annotations have more well-defined semantics. Its `@Nullable`
annotation is also a type-use annotation recognized by Google Java
Format, so the formatter places it after any field or method modifiers.

See https://jspecify.dev
  • Loading branch information
Stephan202 authored Oct 25, 2022
1 parent 50e730f commit b2e1560
Show file tree
Hide file tree
Showing 28 changed files with 71 additions and 92 deletions.
5 changes: 0 additions & 5 deletions error-prone-contrib/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,6 @@
<artifactId>auto-service-annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.googlejavaformat</groupId>
<artifactId>google-java-format</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;

/**
Expand Down Expand Up @@ -207,7 +207,7 @@ private static boolean isStringTyped(ExpressionTree tree, VisitorState state) {
}

private static class ReplacementArgumentsConstructor
extends SimpleTreeVisitor<Void, VisitorState> {
extends SimpleTreeVisitor<@Nullable Void, VisitorState> {
private final StringBuilder formatString = new StringBuilder();
private final List<Tree> formatArguments = new ArrayList<>();
private final String formatSpecifier;
Expand All @@ -216,9 +216,8 @@ private static class ReplacementArgumentsConstructor
this.formatSpecifier = formatSpecifier;
}

@Nullable
@Override
public Void visitBinary(BinaryTree tree, VisitorState state) {
public @Nullable Void visitBinary(BinaryTree tree, VisitorState state) {
if (tree.getKind() == Kind.PLUS && isStringTyped(tree, state)) {
tree.getLeftOperand().accept(this, state);
tree.getRightOperand().accept(this, state);
Expand All @@ -229,15 +228,13 @@ public Void visitBinary(BinaryTree tree, VisitorState state) {
return null;
}

@Nullable
@Override
public Void visitParenthesized(ParenthesizedTree tree, VisitorState state) {
public @Nullable Void visitParenthesized(ParenthesizedTree tree, VisitorState state) {
return tree.getExpression().accept(this, state);
}

@Nullable
@Override
protected Void defaultAction(Tree tree, VisitorState state) {
protected @Nullable Void defaultAction(Tree tree, VisitorState state) {
appendExpression(tree);
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;

Expand Down Expand Up @@ -184,17 +184,15 @@ private static ImmutableList<? extends ExpressionTree> doSort(
private static ImmutableList<ImmutableList<String>> getStructure(ExpressionTree array) {
ImmutableList.Builder<ImmutableList<String>> nodes = ImmutableList.builder();

new TreeScanner<Void, Void>() {
@Nullable
new TreeScanner<@Nullable Void, @Nullable Void>() {
@Override
public Void visitIdentifier(IdentifierTree node, @Nullable Void unused) {
public @Nullable Void visitIdentifier(IdentifierTree node, @Nullable Void unused) {
nodes.add(ImmutableList.of(node.getName().toString()));
return super.visitIdentifier(node, unused);
}

@Nullable
@Override
public Void visitLiteral(LiteralTree node, @Nullable Void unused) {
public @Nullable Void visitLiteral(LiteralTree node, @Nullable Void unused) {
Object value = ASTHelpers.constValue(node);
nodes.add(
value instanceof String
Expand All @@ -204,9 +202,8 @@ public Void visitLiteral(LiteralTree node, @Nullable Void unused) {
return super.visitLiteral(node, unused);
}

@Nullable
@Override
public Void visitPrimitiveType(PrimitiveTypeTree node, @Nullable Void unused) {
public @Nullable Void visitPrimitiveType(PrimitiveTypeTree node, @Nullable Void unused) {
nodes.add(ImmutableList.of(node.getPrimitiveTypeKind().toString()));
return super.visitPrimitiveType(node, unused);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import java.util.Formattable;
import java.util.Iterator;
import java.util.List;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** Picnic Error Prone Contrib checks. */
@com.google.errorprone.annotations.CheckReturnValue
@javax.annotation.ParametersAreNonnullByDefault
@org.jspecify.nullness.NullMarked
package tech.picnic.errorprone.bugpatterns;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** Auxiliary utilities for use by Error Prone checks. */
@com.google.errorprone.annotations.CheckReturnValue
@javax.annotation.ParametersAreNonnullByDefault
@org.jspecify.nullness.NullMarked
package tech.picnic.errorprone.bugpatterns.util;
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;

/**
Expand Down Expand Up @@ -89,14 +89,12 @@ Map<K, V> after() {

static final class MapGetOrNull<K, V, L> {
@BeforeTemplate
@Nullable
V before(Map<K, V> map, L key) {
@Nullable V before(Map<K, V> map, L key) {
return map.getOrDefault(key, null);
}

@AfterTemplate
@Nullable
V after(Map<K, V> map, L key) {
@Nullable V after(Map<K, V> map, L key) {
return map.get(key);
}
}
Expand Down Expand Up @@ -134,8 +132,7 @@ T before(Iterator<T> iterator, T defaultValue) {
}

@AfterTemplate
@Nullable
T after(Iterator<T> iterator, T defaultValue) {
@Nullable T after(Iterator<T> iterator, T defaultValue) {
return Iterators.getNext(iterator, defaultValue);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import java.util.Collection;
import java.util.Set;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;

/** Refaster rules related to expressions dealing with {@link Multimap}s. */
Expand Down Expand Up @@ -50,8 +50,7 @@ int after(Multimap<K, V> multimap) {
*/
static final class MultimapGet<K, V> {
@BeforeTemplate
@Nullable
Collection<V> before(Multimap<K, V> multimap, K key) {
@Nullable Collection<V> before(Multimap<K, V> multimap, K key) {
return Refaster.anyOf(multimap.asMap(), Multimaps.asMap(multimap)).get(key);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import com.google.errorprone.refaster.annotation.UseImportPolicy;
import java.util.Objects;
import java.util.function.Predicate;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;

/** Refaster rules related to expressions dealing with (possibly) null values. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;

/** Refaster rules related to expressions dealing with {@link Optional}s. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
import javax.annotation.Nullable;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;

/** Refaster rules related to expressions dealing with {@link String}s. */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** Picnic Refaster rules. */
@com.google.errorprone.annotations.CheckReturnValue
@javax.annotation.ParametersAreNonnullByDefault
@org.jspecify.nullness.NullMarked
package tech.picnic.errorprone.refasterrules;
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ void identification() {
"import java.util.List;",
"import java.util.Map;",
"import java.util.Set;",
"import javax.annotation.Nullable;",
"import org.jspecify.nullness.Nullable;",
"import org.springframework.web.bind.annotation.DeleteMapping;",
"import org.springframework.web.bind.annotation.GetMapping;",
"import org.springframework.web.bind.annotation.PostMapping;",
Expand Down
15 changes: 7 additions & 8 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,6 @@
<artifactId>auto-value-annotations</artifactId>
<version>${version.auto-value}</version>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<version>3.0.2</version>
</dependency>
<dependency>
<groupId>com.google.googlejavaformat</groupId>
<artifactId>google-java-format</artifactId>
Expand Down Expand Up @@ -628,11 +623,15 @@
</property>
<property name="illegalClasses" value="com.mongodb.lang.Nullable">
<!-- Instead, please use
`javax.annotation.Nullable`. -->
`org.jspecify.nullness.Nullable`. -->
</property>
<property name="illegalClasses" value="io.micrometer.core.lang.Nullable">
<!-- Instead, please use
`javax.annotation.Nullable`. -->
`org.jspecify.nullness.Nullable`. -->
</property>
<property name="illegalClasses" value="javax.annotation.Nullable">
<!-- Instead, please use
`org.jspecify.nullness.Nullable`. -->
</property>
<property name="illegalClasses" value="javax.annotation.concurrent.Immutable">
<!-- Instead, please use
Expand All @@ -648,7 +647,7 @@
</property>
<property name="illegalClasses" value="org.springframework.lang.Nullable">
<!-- Instead, please use
`javax.annotation.Nullable`. -->
`org.jspecify.nullness.Nullable`. -->
</property>
<property name="illegalPkgs" value="com.amazonaws.annotation" />
<property name="illegalPkgs" value="com.beust.jcommander.internal" />
Expand Down
10 changes: 5 additions & 5 deletions refaster-compiler/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@
<artifactId>auto-service-annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>
<dependency>
<groupId>org.jspecify</groupId>
<artifactId>jspecify</artifactId>
<scope>provided</scope>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@
import java.io.UncheckedIOException;
import java.lang.annotation.Annotation;
import java.util.Map;
import javax.annotation.Nullable;
import javax.tools.FileObject;
import javax.tools.JavaFileManager;
import javax.tools.StandardLocation;
import org.jspecify.nullness.Nullable;
import tech.picnic.errorprone.refaster.AnnotatedCompositeCodeTransformer;

/**
Expand Down Expand Up @@ -71,10 +71,10 @@ public void finished(TaskEvent taskEvent) {

private ImmutableMap<ClassTree, CodeTransformer> compileRefasterRules(ClassTree tree) {
ImmutableMap.Builder<ClassTree, CodeTransformer> rules = ImmutableMap.builder();
new TreeScanner<Void, ImmutableClassToInstanceMap<Annotation>>() {
@Nullable
new TreeScanner<@Nullable Void, ImmutableClassToInstanceMap<Annotation>>() {
@Override
public Void visitClass(ClassTree node, ImmutableClassToInstanceMap<Annotation> annotations) {
public @Nullable Void visitClass(
ClassTree node, ImmutableClassToInstanceMap<Annotation> annotations) {
ClassSymbol symbol = ASTHelpers.getSymbol(node);

ImmutableList<CodeTransformer> transformers =
Expand Down Expand Up @@ -105,7 +105,7 @@ private FileObject getOutputFile(TaskEvent taskEvent, ClassTree tree) throws IOE

private static boolean containsRefasterRules(ClassTree tree) {
return Boolean.TRUE.equals(
new TreeScanner<Boolean, Void>() {
new TreeScanner<Boolean, @Nullable Void>() {
@Override
public Boolean visitAnnotation(AnnotationTree node, @Nullable Void unused) {
Symbol sym = ASTHelpers.getSymbol(node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
* files on the classpath.
*/
@com.google.errorprone.annotations.CheckReturnValue
@javax.annotation.ParametersAreNonnullByDefault
@org.jspecify.nullness.NullMarked
package tech.picnic.errorprone.refaster.plugin;
10 changes: 5 additions & 5 deletions refaster-runner/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,6 @@
<artifactId>auto-service-annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
Expand All @@ -66,6 +61,11 @@
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jspecify</groupId>
<artifactId>jspecify</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** Exposes Refaster rules found on the classpath through a regular Error Prone check. */
@com.google.errorprone.annotations.CheckReturnValue
@javax.annotation.ParametersAreNonnullByDefault
@org.jspecify.nullness.NullMarked
package tech.picnic.errorprone.refaster.runner;
11 changes: 5 additions & 6 deletions refaster-support/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,20 @@
<artifactId>auto-value-annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jspecify</groupId>
<artifactId>jspecify</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
* non-patch mode.
*/
@com.google.errorprone.annotations.CheckReturnValue
@javax.annotation.ParametersAreNonnullByDefault
@org.jspecify.nullness.NullMarked
package tech.picnic.errorprone.refaster.annotation;
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
* com.google.errorprone.refaster.annotation.NotMatches @NotMatches} annotations.
*/
@com.google.errorprone.annotations.CheckReturnValue
@javax.annotation.ParametersAreNonnullByDefault
@org.jspecify.nullness.NullMarked
package tech.picnic.errorprone.refaster.matchers;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** Assorted classes that aid the compilation or evaluation of Refaster rules. */
@com.google.errorprone.annotations.CheckReturnValue
@javax.annotation.ParametersAreNonnullByDefault
@org.jspecify.nullness.NullMarked
package tech.picnic.errorprone.refaster;
Loading

0 comments on commit b2e1560

Please sign in to comment.