Skip to content

Commit

Permalink
Migrate jimfs from @NullableDecl to @Nullable.
Browse files Browse the repository at this point in the history
This was prompted by the CI failure of #240, in which `maven-javadoc-plugin` had trouble finding `@NullableDecl`. I assumed that that was prompted by a change to Guava to remove its `checker-compat-qual` dependency, which presumably jimfs had been relying on. But:
- I think Guava had already removed that dep in an earlier release.
- jimfs does declare its own direct dep on `checker-compat-qual`.
- I had already had a CL out for the _real_ fix for the CI failure of #240, which is related to modules. That said, I have no idea why the failure hasn't already been happening at head. Maybe it has something to do with a change in the version of Maven used by default in CI??? I'm not sure I believe that, but if that is the cause, then this is another reason to look at [Maven Wrapper](google/guava#6326).

Anyway, this change _still_ migrates us, since `@NullableDecl` is discouraged nowadays and we can finally replace our usages with usages of type-use annotations.

This change is generated automatically with some tooling that we've already used elsewhere, including testing it on Guava, so I'd expect it to be accurate. For future reference, I'd note that migrations off `@NullableDecl` _could_ be tricky at scale because `@NullableDecl` means "This thing is null _unless it's an array (including varargs), in which case it's an array that can **contain** nulls_" and, to make things worse, the Checker Framework is (probably) the only tool that treats it properly.... Luckily, that doesn't appear to come up here.

RELNOTES=n/a
PiperOrigin-RevId: 536392264
  • Loading branch information
cpovirk authored and Jimfs Team committed May 31, 2023
1 parent 45fc51d commit 7b67fb1
Show file tree
Hide file tree
Showing 39 changed files with 134 additions and 178 deletions.
2 changes: 1 addition & 1 deletion jimfs/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
</dependency>
<dependency>
<groupId>org.checkerframework</groupId>
<artifactId>checker-compat-qual</artifactId>
<artifactId>checker-qual</artifactId>
<optional>true</optional>
</dependency>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Abstract implementation of {@link WatchService}. Provides the means for registering and managing
Expand Down Expand Up @@ -91,16 +91,14 @@ ImmutableList<WatchKey> queuedKeys() {
return ImmutableList.copyOf(queue);
}

@NullableDecl
@Override
public WatchKey poll() {
public @Nullable WatchKey poll() {
checkOpen();
return check(queue.poll());
}

@NullableDecl
@Override
public WatchKey poll(long timeout, TimeUnit unit) throws InterruptedException {
public @Nullable WatchKey poll(long timeout, TimeUnit unit) throws InterruptedException {
checkOpen();
return check(queue.poll(timeout, unit));
}
Expand All @@ -112,8 +110,7 @@ public WatchKey take() throws InterruptedException {
}

/** Returns the given key, throwing an exception if it's the poison. */
@NullableDecl
private WatchKey check(@NullableDecl WatchKey key) {
private @Nullable WatchKey check(@Nullable WatchKey key) {
if (key == poison) {
// ensure other blocking threads get the poison
queue.offer(poison);
Expand Down Expand Up @@ -143,9 +140,9 @@ static final class Event<T> implements WatchEvent<T> {
private final Kind<T> kind;
private final int count;

@NullableDecl private final T context;
private final @Nullable T context;

public Event(Kind<T> kind, int count, @NullableDecl T context) {
public Event(Kind<T> kind, int count, @Nullable T context) {
this.kind = checkNotNull(kind);
checkArgument(count >= 0, "count (%s) must be non-negative", count);
this.count = count;
Expand All @@ -162,9 +159,8 @@ public int count() {
return count;
}

@NullableDecl
@Override
public T context() {
public @Nullable T context() {
return context;
}

Expand Down Expand Up @@ -215,7 +211,7 @@ private static WatchEvent<Object> overflowEvent(int count) {

public Key(
AbstractWatchService watcher,
@NullableDecl Watchable watchable,
@Nullable Watchable watchable,
Iterable<? extends WatchEvent.Kind<?>> subscribedTypes) {
this.watcher = checkNotNull(watcher);
this.watchable = watchable; // nullable for Watcher poison
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import java.nio.file.attribute.UserPrincipal;
import java.util.List;
import java.util.Map;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Attribute provider that provides the {@link AclFileAttributeView} ("acl").
Expand Down Expand Up @@ -71,9 +71,8 @@ public ImmutableSet<String> fixedAttributes() {
return ImmutableMap.of("acl:acl", acl);
}

@NullableDecl
@Override
public Object get(File file, String attribute) {
public @Nullable Object get(File file, String attribute) {
if (attribute.equals("acl")) {
return file.getAttribute("acl", "acl");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import java.nio.file.attribute.FileAttributeView;
import java.util.Arrays;
import java.util.Map;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Abstract provider for handling a specific file attribute view.
Expand Down Expand Up @@ -87,8 +87,7 @@ public ImmutableSet<String> attributes(File file) {
* Returns the value of the given attribute in the given file or null if the attribute is not
* supported by this provider.
*/
@NullableDecl
public abstract Object get(File file, String attribute);
public abstract @Nullable Object get(File file, String attribute);

/**
* Sets the value of the given attribute in the given file object. The {@code create} parameter
Expand All @@ -108,8 +107,7 @@ public ImmutableSet<String> attributes(File file) {
* Returns the type of file attributes object this provider supports, or null if it doesn't
* support reading its attributes as an object.
*/
@NullableDecl
public Class<? extends BasicFileAttributes> attributesType() {
public @Nullable Class<? extends BasicFileAttributes> attributesType() {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Service providing all attribute related operations for a file store. One piece of the file store
Expand Down Expand Up @@ -204,8 +204,7 @@ public Object getAttribute(File file, String view, String attribute) {
return value;
}

@NullableDecl
private Object getAttributeInternal(File file, String view, String attribute) {
private @Nullable Object getAttributeInternal(File file, String view, String attribute) {
AttributeProvider provider = providersByName.get(view);
if (provider == null) {
return null;
Expand Down Expand Up @@ -259,8 +258,8 @@ private void setAttributeInternal(
* if the view type is not supported.
*/
@SuppressWarnings("unchecked")
@NullableDecl
public <V extends FileAttributeView> V getFileAttributeView(FileLookup lookup, Class<V> type) {
public <V extends FileAttributeView> @Nullable V getFileAttributeView(
FileLookup lookup, Class<V> type) {
AttributeProvider provider = providersByViewType.get(type);

if (provider != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileAttributeView;
import java.nio.file.attribute.FileTime;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Attribute provider that provides attributes common to all file systems, the {@link
Expand Down Expand Up @@ -56,9 +56,8 @@ public ImmutableSet<String> fixedAttributes() {
return ATTRIBUTES;
}

@NullableDecl
@Override
public Object get(File file, String attribute) {
public @Nullable Object get(File file, String attribute) {
switch (attribute) {
case "size":
return file.size();
Expand Down Expand Up @@ -149,9 +148,9 @@ public BasicFileAttributes readAttributes() throws IOException {

@Override
public void setTimes(
@NullableDecl FileTime lastModifiedTime,
@NullableDecl FileTime lastAccessTime,
@NullableDecl FileTime createTime)
@Nullable FileTime lastModifiedTime,
@Nullable FileTime lastAccessTime,
@Nullable FileTime createTime)
throws IOException {
File file = lookupFile();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Immutable configuration for an in-memory file system. A {@code Configuration} is passed to a
Expand Down Expand Up @@ -442,7 +442,7 @@ private ImmutableSet<PathNormalization> checkNormalizations(
}

private static void checkNormalizationNotSet(
PathNormalization n, @NullableDecl PathNormalization set) {
PathNormalization n, @Nullable PathNormalization set) {
if (set != null) {
throw new IllegalArgumentException(
"can't set normalization " + n + ": normalization " + set + " already set");
Expand Down
7 changes: 3 additions & 4 deletions jimfs/src/main/java/com/google/common/jimfs/Directory.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.nio.file.attribute.FileTime;
import java.util.Iterator;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* A table of {@linkplain DirectoryEntry directory entries}.
Expand Down Expand Up @@ -105,8 +105,7 @@ public boolean isEmpty() {
}

/** Returns the entry for the given name in this table or null if no such entry exists. */
@NullableDecl
public DirectoryEntry get(Name name) {
public @Nullable DirectoryEntry get(Name name) {
int index = bucketIndex(name, table.length);

DirectoryEntry entry = table[index];
Expand Down Expand Up @@ -337,7 +336,7 @@ DirectoryEntry remove(Name name) {
public Iterator<DirectoryEntry> iterator() {
return new AbstractIterator<DirectoryEntry>() {
int index;
@NullableDecl DirectoryEntry entry;
@Nullable DirectoryEntry entry;

@Override
protected DirectoryEntry computeNext() {
Expand Down
11 changes: 5 additions & 6 deletions jimfs/src/main/java/com/google/common/jimfs/DirectoryEntry.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.nio.file.NotLinkException;
import java.nio.file.Path;
import java.util.Objects;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Entry in a directory, containing references to the directory itself, the file the entry links to
Expand All @@ -41,11 +41,11 @@ final class DirectoryEntry {
private final Directory directory;
private final Name name;

@NullableDecl private final File file;
private final @Nullable File file;

@NullableDecl DirectoryEntry next; // for use in Directory
@Nullable DirectoryEntry next; // for use in Directory

DirectoryEntry(Directory directory, Name name, @NullableDecl File file) {
DirectoryEntry(Directory directory, Name name, @Nullable File file) {
this.directory = checkNotNull(directory);
this.name = checkNotNull(name);
this.file = file;
Expand Down Expand Up @@ -140,8 +140,7 @@ public File file() {
}

/** Returns the file this entry links to or {@code null} if the file does not exist */
@NullableDecl
public File fileOrNull() {
public @Nullable File fileOrNull() {
return file;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.nio.file.attribute.FileAttributeView;
import java.nio.file.attribute.FileTime;
import java.util.Map;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Attribute provider that provides the {@link DosFileAttributeView} ("dos") and allows the reading
Expand Down Expand Up @@ -75,9 +75,8 @@ private static Boolean getDefaultValue(String attribute, Map<String, ?> userProv
return false;
}

@NullableDecl
@Override
public Object get(File file, String attribute) {
public @Nullable Object get(File file, String attribute) {
if (ATTRIBUTES.contains(attribute)) {
return file.getAttribute("dos", attribute);
}
Expand Down
14 changes: 6 additions & 8 deletions jimfs/src/main/java/com/google/common/jimfs/File.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import java.io.IOException;
import java.nio.file.attribute.FileTime;
import java.util.concurrent.locks.ReadWriteLock;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* A file object, containing both the file's metadata and content.
Expand All @@ -43,8 +43,8 @@ public abstract class File {
private FileTime lastAccessTime;
private FileTime lastModifiedTime;

@NullableDecl // null when only the basic view is used (default)
private Table<String, String, Object> attributes;
// null when only the basic view is used (default)
private @Nullable Table<String, String, Object> attributes;

File(int id, FileTime creationTime) {
this.id = id;
Expand Down Expand Up @@ -102,8 +102,7 @@ void copyContentTo(File file) throws IOException {}
* Returns the read-write lock for this file's content, or {@code null} if there is no content
* lock.
*/
@NullableDecl
ReadWriteLock contentLock() {
@Nullable ReadWriteLock contentLock() {
return null;
}

Expand Down Expand Up @@ -210,8 +209,7 @@ final synchronized ImmutableSet<String> getAttributeKeys() {
}

/** Gets the value of the given attribute in the given view. */
@NullableDecl
public final synchronized Object getAttribute(String view, String attribute) {
public final synchronized @Nullable Object getAttribute(String view, String attribute) {
if (attributes == null) {
return null;
}
Expand Down Expand Up @@ -251,7 +249,7 @@ final synchronized void copyAttributes(File target) {
target.putAll(attributes);
}

private synchronized void putAll(@NullableDecl Table<String, String, Object> attributes) {
private synchronized void putAll(@Nullable Table<String, String, Object> attributes) {
if (attributes != null && this.attributes != attributes) {
if (this.attributes == null) {
this.attributes = HashBasedTable.create();
Expand Down
12 changes: 5 additions & 7 deletions jimfs/src/main/java/com/google/common/jimfs/FileSystemView.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
import java.util.Set;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* View of a file system with a specific working directory. As all file system operations need to
Expand Down Expand Up @@ -313,8 +313,7 @@ public RegularFile getOrCreateRegularFile(
* Looks up the regular file at the given path, throwing an exception if the file isn't a regular
* file. Returns null if the file did not exist.
*/
@NullableDecl
private RegularFile lookUpRegularFile(JimfsPath path, Set<OpenOption> options)
private @Nullable RegularFile lookUpRegularFile(JimfsPath path, Set<OpenOption> options)
throws IOException {
store.readLock().lock();
try {
Expand Down Expand Up @@ -700,14 +699,13 @@ private void unlockSourceAndCopy(File sourceFile, File copyFile) {
}

/** Returns a file attribute view using the given lookup callback. */
@NullableDecl
public <V extends FileAttributeView> V getFileAttributeView(FileLookup lookup, Class<V> type) {
public <V extends FileAttributeView> @Nullable V getFileAttributeView(
FileLookup lookup, Class<V> type) {
return store.getFileAttributeView(lookup, type);
}

/** Returns a file attribute view for the given path in this view. */
@NullableDecl
public <V extends FileAttributeView> V getFileAttributeView(
public <V extends FileAttributeView> @Nullable V getFileAttributeView(
final JimfsPath path, Class<V> type, final Set<? super LinkOption> options) {
return store.getFileAttributeView(
new FileLookup() {
Expand Down
Loading

0 comments on commit 7b67fb1

Please sign in to comment.