Skip to content

refactor(Joiner): rename internal toString helper to toCharSequence #8519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Derylfabiensyah wants to merge 2 commits into
base: master
Choose a base branch
from
+23 −25
Open

refactor(Joiner): rename internal toString helper to toCharSequence #8519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15b40ce
refactor(Joiner): rename internal toString helper to toCharSequence
Derylfabiensyah Jun 26, 2026
217a2a1
Merge branch 'master' into refactor/joiner-rename-tostring-helper
Derylfabiensyah Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions android/guava/src/com/google/common/base/Joiner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ public <A extends Appendable> A appendTo(A appendable, Iterable<?> parts) throws
public <A extends Appendable> A appendTo(A appendable, Iterator<?> parts) throws IOException {
checkNotNull(appendable);
if (parts.hasNext()) {
appendable.append(toString(parts.next()));
appendable.append(toCharSequence(parts.next()));
while (parts.hasNext()) {
appendable.append(separator);
appendable.append(toString(parts.next()));
appendable.append(toCharSequence(parts.next()));
}
}
return appendable;
Expand Down Expand Up @@ -241,8 +241,8 @@ public Joiner useForNull(String nullText) {
checkNotNull(nullText);
return new Joiner(this) {
@Override
CharSequence toString(@Nullable Object part) {
return (part == null) ? nullText : Joiner.this.toString(part);
CharSequence toCharSequence(@Nullable Object part) {
return (part == null) ? nullText : Joiner.this.toCharSequence(part);
}

@Override
Expand Down Expand Up @@ -276,15 +276,15 @@ public <A extends Appendable> A appendTo(A appendable, Iterator<?> parts) throws
while (parts.hasNext()) {
Object part = parts.next();
if (part != null) {
appendable.append(Joiner.this.toString(part));
appendable.append(Joiner.this.toCharSequence(part));
break;
}
}
while (parts.hasNext()) {
Object part = parts.next();
if (part != null) {
appendable.append(separator);
appendable.append(Joiner.this.toString(part));
appendable.append(Joiner.this.toCharSequence(part));
}
}
return appendable;
Expand Down Expand Up @@ -390,15 +390,15 @@ public <A extends Appendable> A appendTo(A appendable, Iterator<? extends Entry<
checkNotNull(appendable);
if (parts.hasNext()) {
Entry<?, ?> entry = parts.next();
appendable.append(joiner.toString(entry.getKey()));
appendable.append(joiner.toCharSequence(entry.getKey()));
appendable.append(keyValueSeparator);
appendable.append(joiner.toString(entry.getValue()));
appendable.append(joiner.toCharSequence(entry.getValue()));
while (parts.hasNext()) {
appendable.append(joiner.separator);
Entry<?, ?> e = parts.next();
appendable.append(joiner.toString(e.getKey()));
appendable.append(joiner.toCharSequence(e.getKey()));
appendable.append(keyValueSeparator);
appendable.append(joiner.toString(e.getValue()));
appendable.append(joiner.toCharSequence(e.getValue()));
}
}
return appendable;
Expand Down Expand Up @@ -470,8 +470,7 @@ public MapJoiner useForNull(String nullText) {
}
}

// TODO(cpovirk): Rename to "toCharSequence."
CharSequence toString(@Nullable Object part) {
CharSequence toCharSequence(@Nullable Object part) {

@ricardoofnl ricardoofnl Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as in the JRE flavor: the comment block further down in this method (around line 488) still says toString(Object) and should be updated to toCharSequence(Object) as part of this rename.

/*
* requireNonNull is not safe: Joiner.on(...).join(somethingThatContainsNull) will indeed throw.
* However, Joiner.on(...).useForNull(...).join(somethingThatContainsNull) *is* safe -- because
Expand Down
25 changes: 12 additions & 13 deletions guava/src/com/google/common/base/Joiner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ public <A extends Appendable> A appendTo(A appendable, Iterable<?> parts) throws
public <A extends Appendable> A appendTo(A appendable, Iterator<?> parts) throws IOException {
checkNotNull(appendable);
if (parts.hasNext()) {
appendable.append(toString(parts.next()));
appendable.append(toCharSequence(parts.next()));
while (parts.hasNext()) {
appendable.append(separator);
appendable.append(toString(parts.next()));
appendable.append(toCharSequence(parts.next()));
}
}
return appendable;
Expand Down Expand Up @@ -221,7 +221,7 @@ public String join(Iterable<?> parts) {
*/
toJoin = Arrays.copyOf(toJoin, expandedCapacity(toJoin.length, toJoin.length + 1));
}
toJoin[i++] = toString(part);
toJoin[i++] = toCharSequence(part);
}
// We might not have seen the expected number of elements, as discussed above.
if (i != toJoin.length) {
Expand Down Expand Up @@ -277,8 +277,8 @@ public Joiner useForNull(String nullText) {
checkNotNull(nullText);
return new Joiner(this) {
@Override
CharSequence toString(@Nullable Object part) {
return (part == null) ? nullText : Joiner.this.toString(part);
CharSequence toCharSequence(@Nullable Object part) {
return (part == null) ? nullText : Joiner.this.toCharSequence(part);
}

@Override
Expand Down Expand Up @@ -312,15 +312,15 @@ public <A extends Appendable> A appendTo(A appendable, Iterator<?> parts) throws
while (parts.hasNext()) {
Object part = parts.next();
if (part != null) {
appendable.append(Joiner.this.toString(part));
appendable.append(Joiner.this.toCharSequence(part));
break;
}
}
while (parts.hasNext()) {
Object part = parts.next();
if (part != null) {
appendable.append(separator);
appendable.append(Joiner.this.toString(part));
appendable.append(Joiner.this.toCharSequence(part));
}
}
return appendable;
Expand Down Expand Up @@ -426,15 +426,15 @@ public <A extends Appendable> A appendTo(A appendable, Iterator<? extends Entry<
checkNotNull(appendable);
if (parts.hasNext()) {
Entry<?, ?> entry = parts.next();
appendable.append(joiner.toString(entry.getKey()));
appendable.append(joiner.toCharSequence(entry.getKey()));
appendable.append(keyValueSeparator);
appendable.append(joiner.toString(entry.getValue()));
appendable.append(joiner.toCharSequence(entry.getValue()));
while (parts.hasNext()) {
appendable.append(joiner.separator);
Entry<?, ?> e = parts.next();
appendable.append(joiner.toString(e.getKey()));
appendable.append(joiner.toCharSequence(e.getKey()));
appendable.append(keyValueSeparator);
appendable.append(joiner.toString(e.getValue()));
appendable.append(joiner.toCharSequence(e.getValue()));
}
}
return appendable;
Expand Down Expand Up @@ -506,8 +506,7 @@ public MapJoiner useForNull(String nullText) {
}
}

// TODO(cpovirk): Rename to "toCharSequence."
CharSequence toString(@Nullable Object part) {
CharSequence toCharSequence(@Nullable Object part) {

@ricardoofnl ricardoofnl Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment block further down in this method (around line 524) still references the old name:

Its implementation avoids calling this toString(Object) method in the first place.

Since this PR renames the method, it would be good to update that mention to toCharSequence(Object) as well.

/*
* requireNonNull is not safe: Joiner.on(...).join(somethingThatContainsNull) will indeed throw.
* However, Joiner.on(...).useForNull(...).join(somethingThatContainsNull) *is* safe -- because
Expand Down