Skip to content
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.

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

Stop using floats to represent line widths, which must be integers. #1090

Merged
merged 1 commit into from
Mar 30, 2024
Merged
Changes from all commits
Commits
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
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
Loading