Run NullAway in JSpecify mode (#1608)

This commit is contained in:
Stephan Schroevers
2025-04-02 14:23:03 +02:00
committed by Stephan Schroevers
parent b58be2b89b
commit aa988ef2b0
9 changed files with 76 additions and 24 deletions

View File

@@ -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;
@@ -167,7 +168,10 @@ public final class BugPatternTestExtractor implements Extractor<BugPatternTestCa
* 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.
*/
MethodInvocationTree inputTree = (MethodInvocationTree) ASTHelpers.getReceiver(tree);
MethodInvocationTree inputTree =
(MethodInvocationTree)
requireNonNull(
ASTHelpers.getReceiver(tree), "Instance method invocation must have receiver");
String path = ASTHelpers.constValue(inputTree.getArguments().get(0), String.class);
Optional<String> inputCode = getSourceCode(inputTree);

View File

@@ -11,6 +11,7 @@ import static com.google.errorprone.matchers.Matchers.not;
import static com.google.errorprone.matchers.Matchers.returnStatement;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.matchers.Matchers.toType;
import static java.util.Objects.requireNonNull;
import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
@@ -79,7 +80,10 @@ public final class DirectReturn extends BugChecker implements BlockTreeMatcher {
return Description.NO_MATCH;
}
Symbol variableSymbol = ASTHelpers.getSymbol(((ReturnTree) finalStatement).getExpression());
Symbol variableSymbol =
requireNonNull(
ASTHelpers.getSymbol(((ReturnTree) finalStatement).getExpression()),
"Missing symbol for returned variable");
StatementTree precedingStatement = statements.get(statements.size() - 2);
return tryMatchAssignment(variableSymbol, precedingStatement)

View File

@@ -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.utils.Documentation.BUG_PATTERNS_BASE_URL;
@@ -129,7 +130,10 @@ 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())),
"Missing type for method parameter");
return findMethodSourceAnnotation(tree, state)
.flatMap(
@@ -173,7 +177,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), "No class enclosing method")
.getMembers()
.stream()
.filter(MethodTree.class::isInstance)
.map(MethodTree.class::cast)
.filter(not(tree::equals))

View File

@@ -5,6 +5,7 @@ import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static java.util.Objects.requireNonNull;
import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
@@ -86,7 +87,9 @@ public final class NonEmptyMono extends BugChecker implements MethodInvocationTr
return Description.NO_MATCH;
}
ExpressionTree receiver = ASTHelpers.getReceiver(tree);
ExpressionTree receiver =
requireNonNull(
ASTHelpers.getReceiver(tree), "Instance method invocation must have receiver");
if (!NON_EMPTY_MONO.matches(receiver, state)) {
return Description.NO_MATCH;
}

View File

@@ -515,8 +515,14 @@ final class ReactorRules {
mono.switchIfEmpty(Mono.empty()), mono.flux().next(), mono.flux().singleOrEmpty());
}
// XXX: Consider filing a SonarCloud issue for the S2637 false positive.
@BeforeTemplate
Mono<@Nullable Void> before2(Mono<@Nullable Void> mono) {
@SuppressWarnings({
"java:S2637" /* False positive: result is never `null`. */,
"java:S4968" /* Result may be `Mono<Void>`. */,
"key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict"
})
Mono<? extends @Nullable Void> before2(Mono<@Nullable Void> mono) {
return Refaster.anyOf(mono.ignoreElement(), mono.then());
}
@@ -945,7 +951,8 @@ final class ReactorRules {
/** Prefer direct invocation of {@link Mono#then()}} over more contrived alternatives. */
static final class MonoThen<T> {
@BeforeTemplate
Mono<@Nullable Void> before(Mono<T> mono) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> before(Mono<T> mono) {
return Refaster.anyOf(
mono.ignoreElement().then(),
mono.flux().then(),
@@ -954,7 +961,8 @@ final class ReactorRules {
}
@AfterTemplate
Mono<@Nullable Void> after(Mono<T> mono) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> after(Mono<T> mono) {
return mono.then();
}
}
@@ -962,17 +970,25 @@ final class ReactorRules {
/** Avoid vacuous invocations of {@link Flux#ignoreElements()}. */
static final class FluxThen<T> {
@BeforeTemplate
Mono<@Nullable Void> before(Flux<T> flux) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> before(Flux<T> flux) {
return flux.ignoreElements().then();
}
// XXX: Consider filing a SonarCloud issue for the S2637 false positive.
@BeforeTemplate
Mono<@Nullable Void> before2(Flux<@Nullable Void> flux) {
@SuppressWarnings({
"java:S2637" /* False positive: result is never `null`. */,
"java:S4968" /* Result may be `Mono<Void>`. */,
"key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict"
})
Mono<? extends @Nullable Void> before2(Flux<@Nullable Void> flux) {
return flux.ignoreElements();
}
@AfterTemplate
Mono<@Nullable Void> after(Flux<T> flux) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> after(Flux<T> flux) {
return flux.then();
}
}
@@ -980,12 +996,14 @@ final class ReactorRules {
/** Avoid vacuous invocations of {@link Mono#ignoreElement()}. */
static final class MonoThenEmpty<T> {
@BeforeTemplate
Mono<@Nullable Void> before(Mono<T> mono, Publisher<@Nullable Void> publisher) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> before(Mono<T> mono, Publisher<@Nullable Void> publisher) {
return mono.ignoreElement().thenEmpty(publisher);
}
@AfterTemplate
Mono<@Nullable Void> after(Mono<T> mono, Publisher<@Nullable Void> publisher) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> after(Mono<T> mono, Publisher<@Nullable Void> publisher) {
return mono.thenEmpty(publisher);
}
}
@@ -993,12 +1011,14 @@ final class ReactorRules {
/** Avoid vacuous invocations of {@link Flux#ignoreElements()}. */
static final class FluxThenEmpty<T> {
@BeforeTemplate
Mono<@Nullable Void> before(Flux<T> flux, Publisher<@Nullable Void> publisher) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> before(Flux<T> flux, Publisher<@Nullable Void> publisher) {
return flux.ignoreElements().thenEmpty(publisher);
}
@AfterTemplate
Mono<@Nullable Void> after(Flux<T> flux, Publisher<@Nullable Void> publisher) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> after(Flux<T> flux, Publisher<@Nullable Void> publisher) {
return flux.thenEmpty(publisher);
}
}
@@ -1054,7 +1074,8 @@ final class ReactorRules {
}
@BeforeTemplate
Mono<@Nullable Void> before2(Mono<T> mono1, Mono<@Nullable Void> mono2) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> before2(Mono<T> mono1, Mono<@Nullable Void> mono2) {
return mono1.thenEmpty(mono2);
}
@@ -1072,7 +1093,8 @@ final class ReactorRules {
}
@BeforeTemplate
Mono<@Nullable Void> before2(Flux<T> flux, Mono<@Nullable Void> mono) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> before2(Flux<T> flux, Mono<@Nullable Void> mono) {
return flux.thenEmpty(mono);
}

View File

@@ -23,14 +23,16 @@ final class RxJava2AdapterRules {
/** Use the fluent API style when using {@link RxJava2Adapter#completableToMono}. */
static final class CompletableToMono {
@BeforeTemplate
Mono<@Nullable Void> before(Completable completable) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> before(Completable completable) {
return Refaster.anyOf(
RxJava2Adapter.completableToMono(completable),
completable.to(RxJava2Adapter::completableToMono));
}
@AfterTemplate
Mono<@Nullable Void> after(Completable completable) {
@SuppressWarnings("java:S4968" /* Result may be `Mono<Void>`. */)
Mono<? extends @Nullable Void> after(Completable completable) {
return completable.as(RxJava2Adapter::completableToMono);
}
}

View File

@@ -29,6 +29,7 @@ import com.sun.source.tree.ReturnTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.PackageSymbol;
import com.sun.tools.javac.code.Type;
import java.util.List;
import java.util.Optional;
@@ -126,7 +127,7 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
return Optional.empty();
}
Symbol sym = ASTHelpers.getSymbol(methodSelect);
Symbol sym = ASTHelpers.getSymbol(subTree);
return ASTHelpers.isStatic(sym)
? constructFix(lambdaExpr, sym.owner, methodSelect)
: constructFix(lambdaExpr, "this", methodSelect);
@@ -200,7 +201,8 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr
Name sName = target.getSimpleName();
Optional<SuggestedFix.Builder> fix = constructFix(lambdaExpr, sName, methodName);
if (!"java.lang".equals(ASTHelpers.enclosingPackage(target).toString())) {
PackageSymbol pkg = ASTHelpers.enclosingPackage(target);
if (pkg != null && !"java.lang".equals(pkg.toString())) {
Name fqName = target.getQualifiedName();
if (!sName.equals(fqName)) {
return fix.map(b -> b.addImport(fqName.toString()));

View File

@@ -1,5 +1,7 @@
package tech.picnic.errorprone.utils;
import static java.util.Objects.requireNonNull;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableSet;
@@ -115,9 +117,15 @@ public final class AnnotationAttributeMatcher implements Serializable {
}
private static String extractAttributeName(ExpressionTree expr) {
return (expr instanceof AssignmentTree assignment)
? ASTHelpers.getSymbol(assignment.getVariable()).getSimpleName().toString()
: "value";
if (!(expr instanceof AssignmentTree assignment)) {
return "value";
}
return requireNonNull(
ASTHelpers.getSymbol(assignment.getVariable()),
"Missing symbol for annotation attribute")
.getSimpleName()
.toString();
}
// XXX: The caller of this method can be implemented more efficiently in case of a "wholeTypes"

View File

@@ -1940,6 +1940,7 @@
-XepOpt:NullAway:AnnotatedPackages=tech.picnic
-XepOpt:NullAway:AssertsEnabled=true
-XepOpt:NullAway:CheckOptionalEmptiness=true
-XepOpt:NullAway:JSpecifyMode=true
-XepOpt:Nullness:Conservative=false
-XepOpt:StatementSwitchToExpressionSwitch:EnableDirectConversion=true
<!-- Append additional custom arguments. -->