Skip to content

Commit b1fb22c

Browse files
cushongoogle-java-format Team
authored andcommitted
Remove control-flow brace insertion from the cleanup pass.
Stop inserting braces around single-statement control flow bodies (`if`, `else`, `for`, `do`, `while`) in `--cleanup`. Automatically adding braces can wrap unintended following statements during partial or in-progress edits (for example, formatting after typing `if (cond)` before typing the body), while `--cleanup` remains enabled by default for redundant semicolon removal. PiperOrigin-RevId: 1007177180
1 parent f7eeb77 commit b1fb22c

7 files changed

Lines changed: 44 additions & 299 deletions

File tree

‎core/src/main/java/com/google/googlejavaformat/java/CleanupPass.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ ImmutableList<Replacement> findReplacements(Cleanups cleanups) throws FormatterE
2626
cleanups.context(), cleanups.unit(), cleanups.contents());
2727
}
2828
},
29-
/// AST cleanups: missing control-flow braces and redundant semicolons.
29+
/// Non-whitespace AST cleanups.
3030
AST_CLEANUPS {
3131
@Override
3232
ImmutableList<Replacement> findReplacements(Cleanups cleanups) throws FormatterException {

‎core/src/main/java/com/google/googlejavaformat/java/CleanupScanner.java‎

Lines changed: 6 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import static com.sun.tools.javac.code.Flags.GENERATED_MEMBER;
2222
import static java.lang.Math.max;
2323
import static java.lang.Math.min;
24-
import static java.util.Comparator.comparingInt;
2524

2625
import com.google.common.collect.ImmutableList;
2726
import com.google.common.collect.ImmutableSet;
@@ -33,31 +32,22 @@
3332
import com.sun.source.tree.CaseTree;
3433
import com.sun.source.tree.ClassTree;
3534
import com.sun.source.tree.CompilationUnitTree;
36-
import com.sun.source.tree.DoWhileLoopTree;
3735
import com.sun.source.tree.EmptyStatementTree;
38-
import com.sun.source.tree.EnhancedForLoopTree;
39-
import com.sun.source.tree.ForLoopTree;
40-
import com.sun.source.tree.IfTree;
4136
import com.sun.source.tree.ImportTree;
4237
import com.sun.source.tree.NewClassTree;
43-
import com.sun.source.tree.StatementTree;
4438
import com.sun.source.tree.Tree;
4539
import com.sun.source.tree.VariableTree;
46-
import com.sun.source.tree.WhileLoopTree;
4740
import com.sun.source.util.TreePathScanner;
4841
import com.sun.tools.javac.tree.JCTree;
4942
import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
5043
import com.sun.tools.javac.tree.TreeInfo;
5144
import java.util.ArrayList;
5245
import java.util.Collection;
5346
import java.util.List;
54-
import java.util.Map;
55-
import java.util.NavigableMap;
56-
import java.util.TreeMap;
5747
import org.jspecify.annotations.Nullable;
5848

59-
/// Scans the compilation unit for non-whitespace cleanup opportunities: inserting missing
60-
/// control-flow braces and removing redundant semicolons.
49+
/// Scans the compilation unit for non-whitespace cleanup opportunities, such as removing redundant
50+
/// semicolons.
6151
final class CleanupScanner extends TreePathScanner<@Nullable Void, @Nullable Void> {
6252

6353
// Replace with Flags.IMPLICIT_CLASS once JDK 25 is the minimum supported version
@@ -69,27 +59,14 @@ static ImmutableList<Replacement> scan(
6959
RangeSet<Integer> targetRanges = TreeRangeSet.create(ranges);
7060
ImmutableList<Tok> toks = JavaInput.buildToks(contents, ImmutableSet.of());
7161
RangeSet<Integer> activeDeletions = TreeRangeSet.create();
72-
NavigableMap<Integer, StringBuilder> activeInsertions = new TreeMap<>();
73-
new CleanupScanner(contents, unit, toks, targetRanges, activeDeletions, activeInsertions)
74-
.scan(unit, null);
75-
if (activeDeletions.isEmpty() && activeInsertions.isEmpty()) {
62+
new CleanupScanner(contents, unit, toks, targetRanges, activeDeletions).scan(unit, null);
63+
if (activeDeletions.isEmpty()) {
7664
return ImmutableList.of();
7765
}
7866
List<Replacement> replacements = new ArrayList<>();
7967
for (Range<Integer> del : activeDeletions.asRanges()) {
80-
int dStart = del.lowerEndpoint();
81-
int dEnd = del.upperEndpoint();
82-
var subMap =
83-
activeInsertions.subMap(dStart, /* fromInclusive= */ true, dEnd, /* toInclusive= */ true);
84-
String rep = String.join("", subMap.values());
85-
subMap.clear();
86-
replacements.add(Replacement.create(dStart, dEnd, rep));
68+
replacements.add(Replacement.create(del.lowerEndpoint(), del.upperEndpoint(), ""));
8769
}
88-
for (Map.Entry<Integer, StringBuilder> entry : activeInsertions.entrySet()) {
89-
int pos = entry.getKey();
90-
replacements.add(Replacement.create(pos, pos, entry.getValue().toString()));
91-
}
92-
replacements.sort(comparingInt(r -> r.replaceRange().lowerEndpoint()));
9370
verifyCommentsPreserved(toks, contents, replacements);
9471
return ImmutableList.copyOf(replacements);
9572
}
@@ -128,31 +105,24 @@ private static void verifyCommentsPreserved(
128105
private final ImmutableList<Tok> toks;
129106
private final RangeSet<Integer> targetRanges;
130107
private final RangeSet<Integer> activeDeletions;
131-
private final NavigableMap<Integer, StringBuilder> activeInsertions;
132108

133109
CleanupScanner(
134110
String contents,
135111
JCCompilationUnit unit,
136112
ImmutableList<Tok> toks,
137113
RangeSet<Integer> targetRanges,
138-
RangeSet<Integer> activeDeletions,
139-
NavigableMap<Integer, StringBuilder> activeInsertions) {
114+
RangeSet<Integer> activeDeletions) {
140115
this.contents = contents;
141116
this.unit = unit;
142117
this.toks = toks;
143118
this.targetRanges = targetRanges;
144119
this.activeDeletions = activeDeletions;
145-
this.activeInsertions = activeInsertions;
146120
}
147121

148122
private boolean inRange(int startPos, int endPos) {
149123
return targetRanges.intersects(Range.closedOpen(startPos, endPos));
150124
}
151125

152-
private void insert(int pos, String text) {
153-
activeInsertions.computeIfAbsent(pos, k -> new StringBuilder()).append(text);
154-
}
155-
156126
private void deleteRedundantSemicolon(int semiPos) {
157127
int start = semiPos;
158128
while (start > 0 && isHorizontalWhitespace(contents.charAt(start - 1))) {
@@ -301,85 +271,6 @@ private void deleteSemicolonsInGaps(int start, int end, RangeSet<Integer> validS
301271
}
302272
}
303273

304-
@Override
305-
public @Nullable Void visitIf(IfTree tree, @Nullable Void unused) {
306-
super.visitIf(tree, null);
307-
checkBraces(tree, tree.getThenStatement());
308-
if (!(tree.getElseStatement() instanceof IfTree)) {
309-
checkBraces(tree, tree.getElseStatement());
310-
}
311-
return null;
312-
}
313-
314-
@Override
315-
public @Nullable Void visitForLoop(ForLoopTree tree, @Nullable Void unused) {
316-
super.visitForLoop(tree, null);
317-
checkBraces(tree, tree.getStatement());
318-
return null;
319-
}
320-
321-
@Override
322-
public @Nullable Void visitEnhancedForLoop(EnhancedForLoopTree tree, @Nullable Void unused) {
323-
super.visitEnhancedForLoop(tree, null);
324-
checkBraces(tree, tree.getStatement());
325-
return null;
326-
}
327-
328-
@Override
329-
public @Nullable Void visitWhileLoop(WhileLoopTree tree, @Nullable Void unused) {
330-
super.visitWhileLoop(tree, null);
331-
checkBraces(tree, tree.getStatement());
332-
return null;
333-
}
334-
335-
@Override
336-
public @Nullable Void visitDoWhileLoop(DoWhileLoopTree tree, @Nullable Void unused) {
337-
super.visitDoWhileLoop(tree, null);
338-
checkBraces(tree, tree.getStatement());
339-
return null;
340-
}
341-
342-
private void checkBraces(Tree owner, @Nullable StatementTree body) {
343-
if (body == null || body instanceof BlockTree) {
344-
return;
345-
}
346-
int rangeEnd =
347-
owner instanceof DoWhileLoopTree ? getEndPosition(owner, unit) : getEndPosition(body, unit);
348-
if (!inRange(getStartPosition(owner), rangeEnd)) {
349-
return;
350-
}
351-
Tok prev = prevToken(getStartPosition(body));
352-
if (prev == null) {
353-
return;
354-
}
355-
insert(prev.getPosition() + prev.length(), "{");
356-
insert(closingBraceInsertionPos(body), "}");
357-
if (body instanceof EmptyStatementTree) {
358-
activeDeletions.add(Range.closedOpen(getStartPosition(body), getEndPosition(body, unit)));
359-
}
360-
}
361-
362-
private int closingBraceInsertionPos(StatementTree stmt) {
363-
int stmtEnd = getEndPosition(stmt, unit);
364-
if (stmt instanceof EmptyStatementTree) {
365-
return stmtEnd;
366-
}
367-
boolean sawComment = false;
368-
for (int i = tokIndexAtOrAfter(stmtEnd); i < toks.size(); i++) {
369-
Tok tok = toks.get(i);
370-
if (tok.isToken()) {
371-
return stmtEnd;
372-
}
373-
if (tok.isNewline()) {
374-
return sawComment ? tok.getPosition() + tok.length() : stmtEnd;
375-
}
376-
if (tok.isComment()) {
377-
sawComment = true;
378-
}
379-
}
380-
return stmtEnd;
381-
}
382-
383274
@Override
384275
public @Nullable Void visitEmptyStatement(EmptyStatementTree tree, @Nullable Void unused) {
385276
// Include CaseTree for empty statements in colon-style switch statement groups
@@ -410,16 +301,6 @@ private int tokIndexAtOrAfter(int pos) {
410301
return lo;
411302
}
412303

413-
private @Nullable Tok prevToken(int pos) {
414-
for (int i = tokIndexAtOrAfter(pos) - 1; i >= 0; i--) {
415-
Tok tok = toks.get(i);
416-
if (tok.isToken() && !tok.getText().isEmpty()) {
417-
return tok;
418-
}
419-
}
420-
return null;
421-
}
422-
423304
private @Nullable Tok findTokenAtOrAfter(int startPos, int endPos, String text) {
424305
for (int i = tokIndexAtOrAfter(startPos); i < toks.size(); i++) {
425306
Tok tok = toks.get(i);

‎core/src/main/java/com/google/googlejavaformat/java/Formatter.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,9 +243,9 @@ public static String fixImports(
243243
/// Emit a list of [Replacement]s to convert from input to output.
244244
///
245245
/// This is a whitespace-only API that performs core layout and whitespace formatting on the
246-
/// requested ranges. It does not perform AST-based cleanups (such as brace insertion or modifier
247-
/// cleanup), nor does it modify unformatted code outside the requested ranges (such as file-wide
248-
/// blank line or trailing whitespace cleanup).
246+
/// requested ranges. It does not perform AST-based cleanups (such as removing redundant
247+
/// semicolons or reordering modifiers), nor does it modify unformatted code outside the requested
248+
/// ranges (such as file-wide blank line or trailing whitespace cleanup).
249249
///
250250
/// @param input the input compilation unit
251251
/// @param characterRanges the character ranges to reformat

‎core/src/main/java/com/google/googlejavaformat/java/UsageException.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ Maximum line length (default is 100).
5757
--skip-reordering-modifiers
5858
Do not reorder modifiers into the JLS-recommended order.
5959
--cleanup[=(true|false)]
60-
Enables non-whitespace cleanups, such as inserting missing braces and removing redundant semicolons (default is true).
60+
Enables non-whitespace cleanups, such as removing redundant semicolons (default is true).
6161
--dry-run, -n
6262
Prints the paths of the files whose contents would change if the formatter were run normally.
6363
--set-exit-if-changed

0 commit comments

Comments
 (0)