Skip to content

Commit

Permalink
Return a char from formatReleaseVersion
Browse files Browse the repository at this point in the history
Currently it always returns a `String` of length 1. This matches code in the JDK that is going to require updates for JDK 36, but I'm going to worry about that later.

The motivation is that this is the only place that `String#contains` is used in a context where string views could be used instead.

PiperOrigin-RevId: 689964455
  • Loading branch information
cushon authored and Javac Team committed Oct 28, 2024
1 parent fdeba1e commit 28094be
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 13 deletions.
21 changes: 13 additions & 8 deletions java/com/google/turbine/binder/CtSymClassBinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.google.turbine.binder;

import static com.google.common.base.Ascii.toUpperCase;
import static com.google.common.base.StandardSystemProperty.JAVA_HOME;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -70,10 +69,7 @@ public final class CtSymClassBinder {
return map.get(sym);
}
};
// ct.sym contains directories whose names are the concatentation of a list of target versions
// formatted as a single character 0-9 or A-Z (e.g. 789A) and which contain interface class
// files with a .sig extension.
String releaseString = formatReleaseVersion(version);
char releaseChar = formatReleaseVersion(version);
for (Zip.Entry ze : new Zip.ZipIterable(ctSym)) {
String name = ze.name();
if (!name.endsWith(".sig")) {
Expand All @@ -84,7 +80,7 @@ public final class CtSymClassBinder {
continue;
}
// check if the directory matches the desired release
if (!ze.name().substring(0, idx).contains(releaseString)) {
if (ze.name().substring(0, idx).indexOf(releaseChar) == -1) {
continue;
}
// JDK >= 12 includes the module name as a prefix
Expand Down Expand Up @@ -138,12 +134,21 @@ public byte[] get() {
});
}

// ct.sym contains directories whose names are the concatenation of a list of target versions
// formatted as a single character 0-9 or A-Z (e.g. 789A) and which contain interface class
// files with a .sig extension.
// This was updated to use 36 as a radix in https://bugs.openjdk.org/browse/JDK-8245544,
// it's not clear what the plan is for JDK 36.
@VisibleForTesting
static String formatReleaseVersion(int n) {
static char formatReleaseVersion(int n) {
if (n <= 4 || n >= 36) {
throw new IllegalArgumentException("invalid release version: " + n);
}
return toUpperCase(Integer.toString(n, 36));
if (n < 10) {
return (char) ('0' + n);
} else {
return (char) ('A' + n - 10);
}
}

private CtSymClassBinder() {}
Expand Down
12 changes: 7 additions & 5 deletions javatests/com/google/turbine/binder/CtSymClassBinderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@ public class CtSymClassBinderTest {
public void formatReleaseVersion() {
ImmutableList.of(5, 6, 7, 8, 9)
.forEach(
x -> assertThat(CtSymClassBinder.formatReleaseVersion(x)).isEqualTo(String.valueOf(x)));
x ->
assertThat(Character.toString(CtSymClassBinder.formatReleaseVersion(x)))
.isEqualTo(String.valueOf(x)));
ImmutableMap.of(
10, "A",
11, "B",
12, "C",
35, "Z")
10, 'A',
11, 'B',
12, 'C',
35, 'Z')
.forEach((k, v) -> assertThat(CtSymClassBinder.formatReleaseVersion(k)).isEqualTo(v));
ImmutableList.of(4, 36)
.forEach(
Expand Down

0 comments on commit 28094be

Please sign in to comment.