Skip to content

Commit

Permalink
Migrate Error Prone to use JSpecify nullness annotations
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 655693769
  • Loading branch information
cushon authored and Error Prone Team committed Jul 24, 2024
1 parent a94279c commit bfe69f8
Show file tree
Hide file tree
Showing 179 changed files with 498 additions and 757 deletions.
8 changes: 4 additions & 4 deletions check_api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@
<version>${project.version}</version>
</dependency>
<dependency>
<!-- BSD New -->
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<version>3.0.2</version>
<!-- Apache 2.0 -->
<groupId>org.jspecify</groupId>
<artifactId>jspecify</artifactId>
<version>${jspecify.version}</version>
</dependency>
<dependency>
<!-- GPLv2 with Classpath Exception -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@
import java.util.Locale;
import java.util.ResourceBundle;
import java.util.Set;
import javax.annotation.Nullable;
import javax.lang.model.SourceVersion;
import javax.tools.DiagnosticListener;
import javax.tools.JavaCompiler;
import javax.tools.JavaCompiler.CompilationTask;
import javax.tools.JavaFileManager;
import javax.tools.JavaFileObject;
import javax.tools.StandardJavaFileManager;
import org.jspecify.annotations.Nullable;

/** An Error Prone compiler that implements {@link javax.tools.JavaCompiler}. */
public class BaseErrorProneJavaCompiler implements JavaCompiler {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import java.util.Map;
import java.util.Set;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

/**
* An accessor for information about a single bug checker, including the metadata in the check's
Expand Down Expand Up @@ -172,8 +172,7 @@ public BugCheckerInfo withCustomDefaultSeverity(SeverityLevel defaultSeverity) {
disableable);
}

@Nullable
private static String createLinkUrl(String canonicalName, BugPattern pattern) {
private static @Nullable String createLinkUrl(String canonicalName, BugPattern pattern) {
switch (pattern.linkType()) {
case AUTOGENERATED:
return String.format("https://errorprone.info/bugpattern/%s", canonicalName);
Expand Down
35 changes: 13 additions & 22 deletions check_api/src/main/java/com/google/errorprone/VisitorState.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import javax.annotation.Nullable;
import javax.lang.model.util.Elements;
import org.jspecify.annotations.Nullable;

/**
* @author [email protected] (Alex Eagle)
Expand Down Expand Up @@ -355,16 +355,14 @@ public Name getName(String nameStr) {
* @param typeStr the JLS 13.1 binary name of the class, e.g. {@code "java.util.Map$Entry"}
* @return the {@link Type}, or null if it cannot be found
*/
@Nullable
public Type getTypeFromString(String typeStr) {
public @Nullable Type getTypeFromString(String typeStr) {
return sharedState
.typeCache
.computeIfAbsent(typeStr, key -> Optional.ofNullable(getTypeFromStringInternal(key)))
.orElse(null);
}

@Nullable
private Type getTypeFromStringInternal(String typeStr) {
private @Nullable Type getTypeFromStringInternal(String typeStr) {
validateTypeStr(typeStr);
Type primitiveOrVoidType = getPrimitiveOrVoidType(typeStr);
if (primitiveOrVoidType != null) {
Expand All @@ -385,8 +383,7 @@ private Type getTypeFromStringInternal(String typeStr) {
* @return the Symbol object, or null if it cannot be found
*/
// TODO(cushon): deal with binary compat issues and return ClassSymbol
@Nullable
public Symbol getSymbolFromString(String symStr) {
public @Nullable Symbol getSymbolFromString(String symStr) {
return getSymbolFromName(binaryNameFromClassname(symStr));
}

Expand All @@ -404,8 +401,7 @@ public Name binaryNameFromClassname(String className) {
*
* @param name the name to look up, which must be in binary form (i.e. with $ for nested classes).
*/
@Nullable
public ClassSymbol getSymbolFromName(Name name) {
public @Nullable ClassSymbol getSymbolFromName(Name name) {
boolean modular = sharedState.modules.getDefaultModule() != getSymtab().noModule;
if (!modular) {
return getSymbolFromString(getSymtab().noModule, name);
Expand All @@ -422,8 +418,7 @@ public ClassSymbol getSymbolFromName(Name name) {
return null;
}

@Nullable
public ClassSymbol getSymbolFromString(ModuleSymbol msym, Name name) {
public @Nullable ClassSymbol getSymbolFromString(ModuleSymbol msym, Name name) {
ClassSymbol result = getSymtab().getClass(msym, name);
if (result == null || result.kind == Kind.ERR || !result.exists()) {
return null;
Expand Down Expand Up @@ -507,9 +502,8 @@ public Type arrayTypeForType(Type baseType) {
*
* @return the path, or {@code null} if there is no match
*/
@Nullable
@SafeVarargs
public final TreePath findPathToEnclosing(Class<? extends Tree>... classes) {
public final @Nullable TreePath findPathToEnclosing(Class<? extends Tree>... classes) {
TreePath enclosingPath = getPath();
while (enclosingPath != null) {
for (Class<? extends Tree> clazz : classes) {
Expand All @@ -527,10 +521,10 @@ public final TreePath findPathToEnclosing(Class<? extends Tree>... classes) {
*
* @return the node, or {@code null} if there is no match
*/
@Nullable
@SuppressWarnings("unchecked") // findPathToEnclosing guarantees that the type is from |classes|
// findPathToEnclosing guarantees that the type is from |classes|
@SuppressWarnings("unchecked")
@SafeVarargs
public final <T extends Tree> T findEnclosing(Class<? extends T>... classes) {
public final <T extends Tree> @Nullable T findEnclosing(Class<? extends T>... classes) {
TreePath pathToEnclosing = findPathToEnclosing(classes);
return (pathToEnclosing == null) ? null : (T) pathToEnclosing.getLeaf();
}
Expand All @@ -540,8 +534,7 @@ public final <T extends Tree> T findEnclosing(Class<? extends T>... classes) {
*
* @return the source file as a sequence of characters, or null if it is not available
*/
@Nullable
public CharSequence getSourceCode() {
public @Nullable CharSequence getSourceCode() {
try {
return getPath().getCompilationUnit().getSourceFile().getCharContent(false);
} catch (IOException e) {
Expand All @@ -559,8 +552,7 @@ public CharSequence getSourceCode() {
* @return the source code that represents the node, or {@code null} if the source code is
* unavailable (e.g. for generated or desugared AST nodes)
*/
@Nullable
public String getSourceForNode(Tree tree) {
public @Nullable String getSourceForNode(Tree tree) {
int start = ((JCTree) tree).getStartPosition();
int end = getEndPosition(tree);
CharSequence source = getSourceCode();
Expand Down Expand Up @@ -634,8 +626,7 @@ private static void validateTypeStr(String typeStr) {
* Given a string that represents a type, if it's a primitive type (e.g., "int") or "void", return
* the corresponding Type, or null otherwise.
*/
@Nullable
private Type getPrimitiveOrVoidType(String typeStr) {
private @Nullable Type getPrimitiveOrVoidType(String typeStr) {
switch (typeStr) {
case "byte":
return getSymtab().byteType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

/**
* Applier of diffs to Java source code
Expand Down Expand Up @@ -132,8 +132,7 @@ public void run() {
}
}

@Nullable
public Future<?> put(Diff diff) {
public @Nullable Future<?> put(Diff diff) {
if (refactoredPaths.add(diff.getRelevantFileName())) {
runState.incrementAndGet();
return workerService.submit(new Task(diff));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package com.google.errorprone.apply;

import java.io.IOException;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

/**
* Supplier of file differences.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import com.sun.tools.javac.tree.JCTree.JCFieldAccess;
import com.sun.tools.javac.tree.JCTree.JCIdent;
import com.sun.tools.javac.tree.JCTree.JCMethodInvocation;
import javax.annotation.Nullable;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.Modifier;
Expand All @@ -38,6 +37,7 @@
import org.checkerframework.errorprone.dataflow.cfg.node.Node;
import org.checkerframework.errorprone.dataflow.cfg.node.VariableDeclarationNode;
import org.checkerframework.errorprone.javacutil.TreeUtils;
import org.jspecify.annotations.Nullable;

/**
* A sequence of field names or autovalue accessors, along with a receiver: either a variable or a
Expand All @@ -61,8 +61,7 @@
public abstract class AccessPath {

/** If present, base of access path is contained Element; if absent, base is `this` */
@Nullable
public abstract Element base();
public abstract @Nullable Element base();

public abstract ImmutableList<String> path();

Expand Down Expand Up @@ -111,8 +110,7 @@ public static boolean isAutoValueAccessor(Tree tree) {
* otherwise (for example, when the receiver of the field access contains an array access or
* non-AutoValue method call.
*/
@Nullable
public static AccessPath fromFieldAccess(FieldAccessNode fieldAccess) {
public static @Nullable AccessPath fromFieldAccess(FieldAccessNode fieldAccess) {
ImmutableList.Builder<String> pathBuilder = ImmutableList.builder();

Tree tree = fieldAccess.getTree();
Expand Down Expand Up @@ -161,8 +159,7 @@ public static AccessPath fromVariableDecl(VariableDeclarationNode node) {
* Returns an AccessPath representing {@code node} if {@code node} is representable as an access
* path and null otherwise
*/
@Nullable
public static AccessPath fromNodeIfTrackable(Node node) {
public static @Nullable AccessPath fromNodeIfTrackable(Node node) {
if (node instanceof LocalVariableNode) {
return fromLocalVariable((LocalVariableNode) node);
} else if (node instanceof VariableDeclarationNode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.LinkedHashMap;
import java.util.Map;
import javax.annotation.Nullable;
import org.checkerframework.errorprone.dataflow.analysis.AbstractValue;
import org.checkerframework.errorprone.dataflow.analysis.Store;
import org.checkerframework.errorprone.dataflow.cfg.visualize.CFGVisualizer;
import org.checkerframework.errorprone.dataflow.expression.JavaExpression;
import org.jspecify.annotations.Nullable;

/**
* Immutable map from local variables or heap access paths to their {@link AbstractValue}
Expand Down Expand Up @@ -58,8 +58,7 @@ public static <V extends AbstractValue<V>> AccessPathStore<V> empty() {
return (AccessPathStore<V>) EMPTY;
}

@Nullable
private V getInformation(AccessPath ap) {
private @Nullable V getInformation(AccessPath ap) {
return heap().get(checkNotNull(ap));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@

import com.sun.source.util.TreePath;
import com.sun.tools.javac.util.Context;
import javax.annotation.Nullable;
import org.checkerframework.errorprone.dataflow.constantpropagation.Constant;
import org.checkerframework.errorprone.dataflow.constantpropagation.ConstantPropagationTransfer;
import org.jspecify.annotations.Nullable;

/** An interface to the constant propagation analysis. */
public final class ConstantPropagationAnalysis {
Expand All @@ -33,8 +33,7 @@ public final class ConstantPropagationAnalysis {
* evaluates to the same numeric value), and null otherwise. Note that returning null does not
* necessarily mean the expression is *not* a constant.
*/
@Nullable
public static Number numberValue(TreePath exprPath, Context context) {
public static @Nullable Number numberValue(TreePath exprPath, Context context) {
Constant val = DataFlow.expressionDataflow(exprPath, context, CONSTANT_PROPAGATION);
if (val == null || !val.isConstant()) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import com.sun.source.util.TreePath;
import com.sun.tools.javac.processing.JavacProcessingEnvironment;
import com.sun.tools.javac.util.Context;
import javax.annotation.Nullable;
import javax.annotation.processing.ProcessingEnvironment;
import org.checkerframework.errorprone.dataflow.analysis.AbstractValue;
import org.checkerframework.errorprone.dataflow.analysis.Analysis;
Expand All @@ -43,6 +42,7 @@
import org.checkerframework.errorprone.dataflow.cfg.ControlFlowGraph;
import org.checkerframework.errorprone.dataflow.cfg.UnderlyingAST;
import org.checkerframework.errorprone.dataflow.cfg.builder.CFGBuilder;
import org.jspecify.annotations.Nullable;

/**
* Provides a wrapper around {@link org.checkerframework.errorprone.dataflow.analysis.Analysis}.
Expand Down Expand Up @@ -127,8 +127,7 @@ public ControlFlowGraph load(CfgParams key) {
});

// TODO(b/158869538): remove once we merge jdk8 specific's with core
@Nullable
private static <T> TreePath findEnclosingMethodOrLambdaOrInitializer(TreePath path) {
private static <T> @Nullable TreePath findEnclosingMethodOrLambdaOrInitializer(TreePath path) {
while (path != null) {
if (path.getLeaf() instanceof MethodTree) {
return path;
Expand Down Expand Up @@ -201,10 +200,9 @@ public ControlFlowGraph getControlFlowGraph() {
* @return dataflow result for the given expression or {@code null} if the expression is not part
* of a method, lambda or initializer
*/
@Nullable
public static <
A extends AbstractValue<A>, S extends Store<S>, T extends ForwardTransferFunction<A, S>>
A expressionDataflow(TreePath exprPath, Context context, T transfer) {
@Nullable A expressionDataflow(TreePath exprPath, Context context, T transfer) {
Tree leaf = exprPath.getLeaf();
Preconditions.checkArgument(
leaf instanceof ExpressionTree,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
Expand All @@ -43,6 +42,7 @@
import javax.lang.model.type.IntersectionType;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.type.TypeVariable;
import org.jspecify.annotations.Nullable;

/** Utilities to extract {@link Nullness} from annotations. */
public class NullnessAnnotations {
Expand Down
Loading

0 comments on commit bfe69f8

Please sign in to comment.