Skip to content

Commit

Permalink
Stop using floats to represent line widths, which must be integers.
Browse files Browse the repository at this point in the history
Float was used so that forced breaks could be modeled with POSITIVE_INFINISTY, but an arbitrary large int works just as well.

PiperOrigin-RevId: 620329267
  • Loading branch information
nreid260 authored and google-java-format Team committed Mar 30, 2024
1 parent 3ee6e2a commit f9d92d2
Showing 1 changed file with 36 additions and 27 deletions.
63 changes: 36 additions & 27 deletions core/src/main/java/com/google/googlejavaformat/Doc.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ public enum FillMode {
FORCED
}

/**
* The maximum supported line width.
*
* <p>This can be used as a sentinel/threshold for {@code Doc}s that break unconditionally.
*
* <p>The value was selected to be obviously too large for any practical line, but small enough to
* prevent accidental overflow.
*/
public static final int MAX_LINE_WIDTH = 1000;

/** State for writing. */
public static final class State {
final int lastIndent;
Expand Down Expand Up @@ -103,8 +113,7 @@ public String toString() {
private static final Range<Integer> EMPTY_RANGE = Range.closedOpen(-1, -1);
private static final DiscreteDomain<Integer> INTEGERS = DiscreteDomain.integers();

// Memoized width; Float.POSITIVE_INFINITY if contains forced breaks.
private final Supplier<Float> width = Suppliers.memoize(this::computeWidth);
private final Supplier<Integer> width = Suppliers.memoize(this::computeWidth);

// Memoized flat; not defined (and never computed) if contains forced breaks.
private final Supplier<String> flat = Suppliers.memoize(this::computeFlat);
Expand All @@ -113,16 +122,16 @@ public String toString() {
private final Supplier<Range<Integer>> range = Suppliers.memoize(this::computeRange);

/**
* Return the width of a {@code Doc}, or {@code Float.POSITIVE_INFINITY} if it must be broken.
* Return the width of a {@code Doc}.
*
* @return the width
*/
final float getWidth() {
final int getWidth() {
return width.get();
}

/**
* Return a {@code Doc}'s flat-string value; not defined (and never called) if the (@code Doc}
* Return a {@code Doc}'s flat-string value; not defined (and never called) if the {@code Doc}
* contains forced breaks.
*
* @return the flat-string value
Expand All @@ -143,9 +152,9 @@ final Range<Integer> range() {
/**
* Compute the {@code Doc}'s width.
*
* @return the width, or {@code Float.POSITIVE_INFINITY} if it must be broken
* @return the width
*/
abstract float computeWidth();
abstract int computeWidth();

/**
* Compute the {@code Doc}'s flat value. Not defined (and never called) if contains forced breaks.
Expand Down Expand Up @@ -202,12 +211,8 @@ void add(Doc doc) {
}

@Override
float computeWidth() {
float thisWidth = 0.0F;
for (Doc doc : docs) {
thisWidth += doc.getWidth();
}
return thisWidth;
int computeWidth() {
return getWidth(docs);
}

@Override
Expand Down Expand Up @@ -246,10 +251,10 @@ Range<Integer> computeRange() {

@Override
public State computeBreaks(CommentsHelper commentsHelper, int maxWidth, State state) {
float thisWidth = getWidth();
int thisWidth = getWidth();
if (state.column + thisWidth <= maxWidth) {
oneLine = true;
return state.withColumn(state.column + (int) thisWidth);
return state.withColumn(state.column + thisWidth);
}
State broken =
computeBroken(
Expand Down Expand Up @@ -295,8 +300,8 @@ private static State computeBreakAndSplit(
State state,
Optional<Break> optBreakDoc,
List<Doc> split) {
float breakWidth = optBreakDoc.isPresent() ? optBreakDoc.get().getWidth() : 0.0F;
float splitWidth = getWidth(split);
int breakWidth = optBreakDoc.isPresent() ? optBreakDoc.get().getWidth() : 0;
int splitWidth = getWidth(split);
boolean shouldBreak =
(optBreakDoc.isPresent() && optBreakDoc.get().fillMode == FillMode.UNIFIED)
|| state.mustBreak
Expand Down Expand Up @@ -348,12 +353,16 @@ private void writeFilled(Output output) {
* Get the width of a sequence of {@link Doc}s.
*
* @param docs the {@link Doc}s
* @return the width, or {@code Float.POSITIVE_INFINITY} if any {@link Doc} must be broken
* @return the width
*/
static float getWidth(List<Doc> docs) {
float width = 0.0F;
static int getWidth(List<Doc> docs) {
int width = 0;
for (Doc doc : docs) {
width += doc.getWidth();

if (width >= MAX_LINE_WIDTH) {
return MAX_LINE_WIDTH; // Paranoid overflow protection
}
}
return width;
}
Expand Down Expand Up @@ -455,7 +464,7 @@ public void add(DocBuilder builder) {
}

@Override
float computeWidth() {
int computeWidth() {
return token.getTok().length();
}

Expand Down Expand Up @@ -512,8 +521,8 @@ public void add(DocBuilder builder) {
}

@Override
float computeWidth() {
return 1.0F;
int computeWidth() {
return 1;
}

@Override
Expand Down Expand Up @@ -615,8 +624,8 @@ public void add(DocBuilder builder) {
}

@Override
float computeWidth() {
return isForced() ? Float.POSITIVE_INFINITY : (float) flat.length();
int computeWidth() {
return isForced() ? MAX_LINE_WIDTH : flat.length();
}

@Override
Expand Down Expand Up @@ -705,7 +714,7 @@ public void add(DocBuilder builder) {
}

@Override
float computeWidth() {
int computeWidth() {
int idx = Newlines.firstBreak(tok.getOriginalText());
// only count the first line of multi-line block comments
if (tok.isComment()) {
Expand All @@ -718,7 +727,7 @@ float computeWidth() {
return reformatParameterComment(tok).map(String::length).orElse(tok.length());
}
}
return idx != -1 ? Float.POSITIVE_INFINITY : (float) tok.length();
return idx != -1 ? MAX_LINE_WIDTH : tok.length();
}

@Override
Expand Down

0 comments on commit f9d92d2

Please sign in to comment.