Skip to content

Commit 68e5bdf

Browse files
committed
Reflow only the strings in the code that formatting the ranges rewrites
--output-replacements, which IntelliJ IDEA and formatDiff with the native binary run, reflowed the whole file once a line in the ranges needed it, and long strings on lines nobody touched changed with it (#219). StringWrapper.wrap takes the ranges and reflows only the literals and text blocks that overlap them. Its passes format and replace text, so the ranges move along with each of them, growing to take in what a replacement rewrites. The ranges it gets are those of the code that formatting the given ranges rewrote, and they also decide whether to reflow at all; that check used to hold the ranges of the input against the lines of the formatted text. Without ranges nothing changes: the whole file is reflowed as before. The method that formats the ranges and reflows them moves to Formatter, for the in-process service to use next.
1 parent edce6f5 commit 68e5bdf

4 files changed

Lines changed: 190 additions & 34 deletions

File tree

‎open-java-format/src/main/java/com/palantir/javaformat/java/FormatFileCallable.java‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import com.palantir.javaformat.Utils;
2525
import java.io.UncheckedIOException;
2626
import java.util.List;
27-
import java.util.Set;
2827
import java.util.concurrent.Callable;
2928

3029
/** Encapsulates information about a file to be formatted, including which parts of the file to format. */
@@ -51,23 +50,10 @@ public String call() throws FormatterException {
5150
Formatter formatter = Formatter.createFormatter(options);
5251
if (parameters.outputReplacements()) {
5352
RangeSet<Integer> characterRanges = characterRanges(input);
54-
Set<Range<Integer>> rangesToChange = characterRanges.asRanges();
55-
List<Replacement> replacements = formatter.getFormatReplacements(input, rangesToChange);
56-
if (parameters.reflowLongStrings()) {
57-
String formattedText = Utils.applyReplacements(input, replacements);
58-
// avoid trying to wrap the input unless the lines needing wrapping are part of the "characterRanges"
59-
if (!StringWrapper.linesNeedWrapping(options.maxLineLength(), formattedText, characterRanges)) {
60-
return writeFormatReplacements(replacements);
61-
}
62-
String wrappedFormattedText = StringWrapper.wrap(options.maxLineLength(), formattedText, formatter);
63-
// if no wrapping change happened, return the initial replacements
64-
if (wrappedFormattedText.equals(formattedText)) {
65-
return writeFormatReplacements(replacements);
66-
}
67-
return writeFormatReplacements(List.of(Replacement.create(0, input.length(), wrappedFormattedText)));
68-
} else {
69-
return writeFormatReplacements(replacements);
70-
}
53+
return writeFormatReplacements(
54+
parameters.reflowLongStrings()
55+
? formatter.getFormatAndReflowReplacements(input, characterRanges)
56+
: formatter.getFormatReplacements(input, characterRanges.asRanges()));
7157
}
7258
return formatFile(formatter);
7359
}

‎open-java-format/src/main/java/com/palantir/javaformat/java/Formatter.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,4 +366,27 @@ public List<Replacement> getFormatReplacements(String input, Collection<Range<In
366366
RangeSet<Integer> tokenRangeSet = javaInput.characterRangesToTokenRanges(characterRanges);
367367
return javaOutput.getFormatReplacements(tokenRangeSet);
368368
}
369+
370+
/**
371+
* Emit the {@link Replacement}s that format the given character ranges and reflow the long strings in the code
372+
* that formatting them rewrites; the imports, and the strings elsewhere, stay as they are. The command line prints
373+
* these for {@code --output-replacements}, and {@link FormatterServiceImpl#getFormatReplacements} returns them,
374+
* so that {@code formatDiff} and IntelliJ IDEA format the same on the JVM and with the native binary.
375+
*/
376+
List<Replacement> getFormatAndReflowReplacements(String input, RangeSet<Integer> characterRanges)
377+
throws FormatterException {
378+
List<Replacement> replacements = getFormatReplacements(input, characterRanges.asRanges());
379+
String formatted = Utils.applyReplacements(input, replacements);
380+
RangeSet<Integer> formattedRanges = StringWrapper.moveRanges(characterRanges, replacements);
381+
// avoid trying to wrap the input unless the lines needing wrapping are part of the ranges
382+
if (!StringWrapper.linesNeedWrapping(options.maxLineLength(), formatted, formattedRanges)) {
383+
return replacements;
384+
}
385+
String wrapped = StringWrapper.wrap(options.maxLineLength(), formatted, formattedRanges, this);
386+
// if no wrapping change happened, return the initial replacements
387+
if (wrapped.equals(formatted)) {
388+
return replacements;
389+
}
390+
return List.of(Replacement.create(0, input.length(), wrapped));
391+
}
369392
}

‎open-java-format/src/main/java/com/palantir/javaformat/java/StringWrapper.java‎

Lines changed: 108 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import com.google.common.base.CharMatcher;
2121
import com.google.common.base.Verify;
22+
import com.google.common.collect.DiscreteDomain;
2223
import com.google.common.collect.Range;
2324
import com.google.common.collect.RangeSet;
2425
import com.google.common.collect.TreeRangeMap;
@@ -41,6 +42,7 @@
4142
import com.sun.tools.javac.util.Position.LineMap;
4243
import java.util.ArrayDeque;
4344
import java.util.ArrayList;
45+
import java.util.Collection;
4446
import java.util.Collections;
4547
import java.util.Comparator;
4648
import java.util.Deque;
@@ -72,53 +74,67 @@ public final class StringWrapper {
7274
* adjacent string literals that share a line.
7375
*/
7476
static String wrap(final int columnLimit, String input, Formatter formatter) throws FormatterException {
75-
String result = wrapOnce(columnLimit, input, formatter);
76-
for (int round = 1; round < MAX_ROUNDS && !result.equals(input); round++) {
77+
return wrap(columnLimit, new Source(input, null), formatter).text();
78+
}
79+
80+
/**
81+
* Reflows, as {@link #wrap(int, String, Formatter)} does, only the string literals and text blocks that overlap
82+
* the given character ranges: when part of a file is formatted, the long strings in the rest of it stay as they
83+
* are.
84+
*/
85+
static String wrap(final int columnLimit, String input, RangeSet<Integer> characterRanges, Formatter formatter)
86+
throws FormatterException {
87+
return wrap(columnLimit, new Source(input, characterRanges), formatter).text();
88+
}
89+
90+
private static Source wrap(final int columnLimit, Source input, Formatter formatter) throws FormatterException {
91+
Source result = wrapOnce(columnLimit, input, formatter);
92+
for (int round = 1; round < MAX_ROUNDS && !result.text().equals(input.text()); round++) {
7793
input = result;
7894
result = wrapOnce(columnLimit, input, formatter);
7995
}
8096
return result;
8197
}
8298

83-
private static String wrapOnce(final int columnLimit, String input, Formatter formatter) throws FormatterException {
84-
if (!needWrapping(columnLimit, input)) {
99+
private static Source wrapOnce(final int columnLimit, Source input, Formatter formatter) throws FormatterException {
100+
if (!needWrapping(columnLimit, input.text())) {
85101
// fast path
86102
return input;
87103
}
88104

89105
// The pieces continue the way the formatter continues a chain of +: four columns times the multiplier of its
90106
// style, eight in OJF.
91107
var continuationIndent = 4 * formatter.options().indentationMultiplier();
92-
TreeRangeMap<Integer, String> replacements = getReflowReplacements(columnLimit, continuationIndent, input);
108+
TreeRangeMap<Integer, String> replacements = input.reflowReplacements(columnLimit, continuationIndent);
93109
if (replacements.asMapOfRanges().isEmpty()) {
94110
// needWrapping is an over-approximation, so there may be nothing to do after all.
95111
return input;
96112
}
97-
String firstPass =
98-
formatter.formatSource(input, replacements.asMapOfRanges().keySet());
113+
Source firstPass = input.format(formatter, replacements.asMapOfRanges().keySet());
99114

100-
if (!firstPass.equals(input)) {
115+
if (!firstPass.text().equals(input.text())) {
101116
// If formatting the replacement ranges resulted in a change, recalculate the replacements on
102117
// the updated input.
103118
input = firstPass;
104-
replacements = getReflowReplacements(columnLimit, continuationIndent, input);
119+
replacements = input.reflowReplacements(columnLimit, continuationIndent);
105120
}
106121

107-
String result = applyReplacements(input, replacements);
122+
Source result = input.apply(replacements);
108123

109124
// Format again, because broken strings might now fit on the first line in case of assignments
110-
String secondPass = formatter.formatSource(result, rangesAfterAppliedReplacements(replacements));
125+
Source secondPass = result.format(formatter, rangesAfterAppliedReplacements(replacements));
111126

112-
if (!secondPass.equals(result)) {
113-
replacements = getReflowReplacements(columnLimit, continuationIndent, secondPass);
114-
result = applyReplacements(secondPass, replacements);
127+
if (!secondPass.text().equals(result.text())) {
128+
replacements = secondPass.reflowReplacements(columnLimit, continuationIndent);
129+
result = secondPass.apply(replacements);
115130
}
116131

117132
{
118133
// We really don't want bugs in this pass to change the behaviour of programs we're
119134
// formatting, so check that the pretty-printed AST is the same before and after reformatting.
120-
String expected = parse(input, /* allowStringFolding= */ true).toString();
121-
String actual = parse(result, /* allowStringFolding= */ true).toString();
135+
String expected =
136+
parse(input.text(), /* allowStringFolding= */ true).toString();
137+
String actual = parse(result.text(), /* allowStringFolding= */ true).toString();
122138
if (!expected.equals(actual)) {
123139
throw new FormatterException(String.format(
124140
"Something has gone terribly wrong. Please file a bug: "
@@ -131,6 +147,82 @@ private static String wrapOnce(final int columnLimit, String input, Formatter fo
131147
return result;
132148
}
133149

150+
/**
151+
* The source between two passes of {@link #wrapOnce}, with the character ranges of it that the passes may change:
152+
* the ranges being formatted, moved along with every change, or {@code null} when the whole file is.
153+
*/
154+
private record Source(String text, @Nullable RangeSet<Integer> ranges) {
155+
156+
/** The replacements of {@link #getReflowReplacements}, less those for literals outside the ranges. */
157+
TreeRangeMap<Integer, String> reflowReplacements(int columnLimit, int continuationIndent)
158+
throws FormatterException {
159+
TreeRangeMap<Integer, String> replacements = getReflowReplacements(columnLimit, continuationIndent, text);
160+
RangeSet<Integer> within = ranges;
161+
if (within == null) {
162+
return replacements;
163+
}
164+
TreeRangeMap<Integer, String> inRanges = TreeRangeMap.create();
165+
replacements.asMapOfRanges().forEach((literal, replacement) -> {
166+
if (within.intersects(literal)) {
167+
inRanges.put(literal, replacement);
168+
}
169+
});
170+
return inRanges;
171+
}
172+
173+
/** Formats the given ranges, as {@link Formatter#formatSource(String, Collection)} does. */
174+
Source format(Formatter formatter, Collection<Range<Integer>> rangesToFormat) throws FormatterException {
175+
List<Replacement> replacements = formatter.getFormatReplacements(text, rangesToFormat);
176+
return new Source(Utils.applyReplacements(text, replacements), moved(replacements));
177+
}
178+
179+
Source apply(TreeRangeMap<Integer, String> replacements) {
180+
List<Replacement> applied = replacements.asMapOfRanges().entrySet().stream()
181+
.map(entry -> Replacement.create(
182+
entry.getKey().lowerEndpoint(), entry.getKey().upperEndpoint(), entry.getValue()))
183+
.toList();
184+
return new Source(applyReplacements(text, replacements), moved(applied));
185+
}
186+
187+
private @Nullable RangeSet<Integer> moved(List<Replacement> replacements) {
188+
return ranges == null ? null : moveRanges(ranges, replacements);
189+
}
190+
}
191+
192+
/**
193+
* Where the given character ranges are once the replacements, sorted and without overlaps, have been applied. A
194+
* range grows to take in all of a replacement that overlaps it, or that inserts text at its start.
195+
*/
196+
static RangeSet<Integer> moveRanges(RangeSet<Integer> ranges, List<Replacement> replacements) {
197+
RangeSet<Integer> moved = TreeRangeSet.create();
198+
for (Range<Integer> range : ranges.asRanges()) {
199+
Range<Integer> canonical = range.canonical(DiscreteDomain.integers());
200+
moved.add(Range.closedOpen(
201+
movePosition(canonical.lowerEndpoint(), replacements, /* start= */ true),
202+
movePosition(canonical.upperEndpoint(), replacements, /* start= */ false)));
203+
}
204+
return moved;
205+
}
206+
207+
private static int movePosition(int position, List<Replacement> replacements, boolean start) {
208+
int shift = 0;
209+
for (Replacement replacement : replacements) {
210+
int lower = replacement.getReplaceRange().lowerEndpoint();
211+
int upper = replacement.getReplaceRange().upperEndpoint();
212+
int length = replacement.getReplacementString().length();
213+
if (upper < position || (upper == position && lower < position)) {
214+
// Before the position.
215+
shift += length - (upper - lower);
216+
} else if (lower < position) {
217+
// Around the position, which moves to the replacement's start or end.
218+
return lower + shift + (start ? 0 : length);
219+
} else {
220+
break;
221+
}
222+
}
223+
return position + shift;
224+
}
225+
134226
private static Set<Range<Integer>> rangesAfterAppliedReplacements(TreeRangeMap<Integer, String> replacements) {
135227
var outputRanges = new LinkedHashSet<Range<Integer>>();
136228
int offset = 0;

‎open-java-format/src/test/java/com/palantir/javaformat/java/StringWrapperTest.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
import com.google.common.base.Joiner;
2121
import com.google.common.collect.Iterables;
22+
import com.google.common.collect.Range;
23+
import com.google.common.collect.RangeSet;
24+
import com.google.common.collect.TreeRangeSet;
2225
import com.sun.source.tree.BinaryTree;
2326
import com.sun.source.tree.ClassTree;
2427
import com.sun.source.tree.CompilationUnitTree;
@@ -29,6 +32,7 @@
2932
import java.io.IOException;
3033
import java.net.URI;
3134
import java.util.List;
35+
import java.util.Set;
3236
import javax.tools.JavaFileObject;
3337
import javax.tools.SimpleJavaFileObject;
3438
import javax.tools.ToolProvider;
@@ -94,6 +98,57 @@ void wrappingKeepsTheValueOfAStringWithEscapes() throws Exception {
9498
assertThat(stringValue(output)).isEqualTo(stringValue(input));
9599
}
96100

101+
@Test
102+
void wrapsOnlyTheStringsInTheRanges() throws Exception {
103+
// As when one line of the file is formatted: the long string on the other line stays as it is.
104+
String input = """
105+
class T {
106+
String outside = "one long incredibly unbroken sentence moving from topic to topic so that no one had a chance";
107+
String inside = "one long incredibly unbroken sentence moving from topic to topic so that no one had a chance";
108+
}
109+
""";
110+
String output = """
111+
class T {
112+
String outside = "one long incredibly unbroken sentence moving from topic to topic so that no one had a chance";
113+
String inside =
114+
"one long incredibly unbroken sentence moving from topic to topic so that no one had a"
115+
+ " chance";
116+
}
117+
""";
118+
int inside = input.indexOf(" String inside");
119+
RangeSet<Integer> insideLine =
120+
TreeRangeSet.create(List.of(Range.closedOpen(inside, input.indexOf('\n', inside))));
121+
RangeSet<Integer> wholeFile = TreeRangeSet.create(List.of(Range.closedOpen(0, input.length())));
122+
123+
assertThat(StringWrapper.wrap(100, input, insideLine, Formatter.create()))
124+
.isEqualTo(output);
125+
assertThat(StringWrapper.wrap(100, input, wholeFile, Formatter.create()))
126+
.isEqualTo(StringWrapper.wrap(100, input, Formatter.create()));
127+
}
128+
129+
@Test
130+
void movesRangesAlongWithTheReplacements() {
131+
// Characters 4 to 8 become two, and two are inserted at 12.
132+
List<Replacement> replacements = List.of(Replacement.create(4, 8, "ab"), Replacement.create(12, 12, "cd"));
133+
134+
// Before the replacements, and after the first one.
135+
assertThat(moved(Range.closedOpen(0, 2), replacements)).containsExactly(Range.closedOpen(0, 2));
136+
assertThat(moved(Range.closedOpen(9, 11), replacements)).containsExactly(Range.closedOpen(7, 9));
137+
// Over the start or the end of a replacement: the range takes in all of it.
138+
assertThat(moved(Range.closedOpen(2, 5), replacements)).containsExactly(Range.closedOpen(2, 6));
139+
assertThat(moved(Range.closedOpen(6, 10), replacements)).containsExactly(Range.closedOpen(4, 8));
140+
// Text inserted at the start of a range goes in, at its end it does not.
141+
assertThat(moved(Range.closedOpen(12, 14), replacements)).containsExactly(Range.closedOpen(10, 14));
142+
assertThat(moved(Range.closedOpen(10, 12), replacements)).containsExactly(Range.closedOpen(8, 10));
143+
// A closed range moves as the half-open one with the same characters.
144+
assertThat(moved(Range.closed(9, 10), replacements)).containsExactly(Range.closedOpen(7, 9));
145+
}
146+
147+
private static Set<Range<Integer>> moved(Range<Integer> range, List<Replacement> replacements) {
148+
return StringWrapper.moveRanges(TreeRangeSet.create(List.of(range)), replacements)
149+
.asRanges();
150+
}
151+
97152
/** The value javac gives the initializer of the only field: a string literal, or literals joined with +. */
98153
private static String stringValue(String source) throws IOException {
99154
JavaFileObject file = new SimpleJavaFileObject(URI.create("string:///T.java"), JavaFileObject.Kind.SOURCE) {

0 commit comments

Comments
 (0)