Skip to content

Commit

Permalink
Make nullWriter().append(...) (both overloads) accept a null CharSequ…
Browse files Browse the repository at this point in the history
…ence.

The docs of Writer are misleading:

"""
An invocation of this method of the form out.append(csq) behaves in exactly the same way as the invocation
     out.write(csq.toString())
"""
https://docs.oracle.com/javase/7/docs/api/java/io/Writer.html#append%28java.lang.CharSequence%29

But that's not true, as the docs go on to say:

"""
csq - The character sequence to append. If csq is null, then the four characters "null" are appended to this writer.
"""

Accepting null in the 2-arg method is arguably even weirder, but at least the docs call it out more prominently:
https://docs.oracle.com/javase/7/docs/api/java/io/Writer.html#append%28java.lang.CharSequence,%20int,%20int%29

Credit to the Checker Framework for identifying this bug.

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=298388993
  • Loading branch information
cpovirk authored and cgdecker committed Mar 2, 2020
1 parent a94114b commit a8107fa
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 8 deletions.
15 changes: 15 additions & 0 deletions android/guava-tests/test/com/google/common/io/CharStreamsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,21 @@ public void testNullWriter() throws Exception {
String test = "Test string for NullWriter";
nullWriter.write(test);
nullWriter.write(test, 2, 10);
nullWriter.append(null);
nullWriter.append(null, 0, 4);

try {
nullWriter.append(null, -1, 4);
fail();
} catch (IndexOutOfBoundsException expected) {
}

try {
nullWriter.append(null, 0, 5);
fail();
} catch (IndexOutOfBoundsException expected) {
}

// nothing really to assert?
assertSame(CharStreams.nullWriter(), CharStreams.nullWriter());
}
Expand Down
8 changes: 4 additions & 4 deletions android/guava/src/com/google/common/io/CharStreams.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.nio.CharBuffer;
import java.util.ArrayList;
import java.util.List;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;

/**
* Provides utility methods for working with character streams.
Expand Down Expand Up @@ -306,14 +307,13 @@ public void write(String str, int off, int len) {
}

@Override
public Writer append(CharSequence csq) {
checkNotNull(csq);
public Writer append(@NullableDecl CharSequence csq) {
return this;
}

@Override
public Writer append(CharSequence csq, int start, int end) {
checkPositionIndexes(start, end, csq.length());
public Writer append(@NullableDecl CharSequence csq, int start, int end) {
checkPositionIndexes(start, end, csq == null ? "null".length() : csq.length());
return this;
}

Expand Down
15 changes: 15 additions & 0 deletions guava-tests/test/com/google/common/io/CharStreamsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,21 @@ public void testNullWriter() throws Exception {
String test = "Test string for NullWriter";
nullWriter.write(test);
nullWriter.write(test, 2, 10);
nullWriter.append(null);
nullWriter.append(null, 0, 4);

try {
nullWriter.append(null, -1, 4);
fail();
} catch (IndexOutOfBoundsException expected) {
}

try {
nullWriter.append(null, 0, 5);
fail();
} catch (IndexOutOfBoundsException expected) {
}

// nothing really to assert?
assertSame(CharStreams.nullWriter(), CharStreams.nullWriter());
}
Expand Down
8 changes: 4 additions & 4 deletions guava/src/com/google/common/io/CharStreams.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.nio.CharBuffer;
import java.util.ArrayList;
import java.util.List;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Provides utility methods for working with character streams.
Expand Down Expand Up @@ -306,14 +307,13 @@ public void write(String str, int off, int len) {
}

@Override
public Writer append(CharSequence csq) {
checkNotNull(csq);
public Writer append(@Nullable CharSequence csq) {
return this;
}

@Override
public Writer append(CharSequence csq, int start, int end) {
checkPositionIndexes(start, end, csq.length());
public Writer append(@Nullable CharSequence csq, int start, int end) {
checkPositionIndexes(start, end, csq == null ? "null".length() : csq.length());
return this;
}

Expand Down

0 comments on commit a8107fa

Please sign in to comment.