Skip to content

Commit 482d63d

Browse files
cushongoogle-java-format Team
authored andcommitted
Remove redundant semicolons in the cleanup pass.
Remove redundant semicolons at top level, in class and enum bodies, and in statements and blocks during the cleanup pass. PiperOrigin-RevId: 1004558693
1 parent 5eb4f96 commit 482d63d

5 files changed

Lines changed: 373 additions & 5 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.
29+
/// AST cleanups: missing control-flow braces and redundant semicolons.
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: 210 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@
1717
import static com.google.common.collect.ImmutableList.toImmutableList;
1818
import static com.google.googlejavaformat.java.Trees.getEndPosition;
1919
import static com.google.googlejavaformat.java.Trees.getStartPosition;
20+
import static com.sun.tools.javac.code.Flags.ENUM;
21+
import static com.sun.tools.javac.code.Flags.GENERATED_MEMBER;
22+
import static java.lang.Math.max;
23+
import static java.lang.Math.min;
2024
import static java.util.Comparator.comparingInt;
2125

2226
import com.google.common.collect.ImmutableList;
@@ -26,16 +30,24 @@
2630
import com.google.common.collect.TreeRangeSet;
2731
import com.google.googlejavaformat.java.JavaInput.Tok;
2832
import com.sun.source.tree.BlockTree;
33+
import com.sun.source.tree.CaseTree;
34+
import com.sun.source.tree.ClassTree;
35+
import com.sun.source.tree.CompilationUnitTree;
2936
import com.sun.source.tree.DoWhileLoopTree;
3037
import com.sun.source.tree.EmptyStatementTree;
3138
import com.sun.source.tree.EnhancedForLoopTree;
3239
import com.sun.source.tree.ForLoopTree;
3340
import com.sun.source.tree.IfTree;
41+
import com.sun.source.tree.ImportTree;
42+
import com.sun.source.tree.NewClassTree;
3443
import com.sun.source.tree.StatementTree;
3544
import com.sun.source.tree.Tree;
45+
import com.sun.source.tree.VariableTree;
3646
import com.sun.source.tree.WhileLoopTree;
3747
import com.sun.source.util.TreePathScanner;
48+
import com.sun.tools.javac.tree.JCTree;
3849
import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
50+
import com.sun.tools.javac.tree.TreeInfo;
3951
import java.util.ArrayList;
4052
import java.util.Collection;
4153
import java.util.List;
@@ -44,18 +56,21 @@
4456
import java.util.TreeMap;
4557
import org.jspecify.annotations.Nullable;
4658

47-
/// Scans the compilation unit for non-whitespace cleanup opportunities, inserting missing
48-
/// control-flow braces.
59+
/// Scans the compilation unit for non-whitespace cleanup opportunities: inserting missing
60+
/// control-flow braces and removing redundant semicolons.
4961
final class CleanupScanner extends TreePathScanner<@Nullable Void, @Nullable Void> {
5062

63+
// Replace with Flags.IMPLICIT_CLASS once JDK 25 is the minimum supported version
64+
private static final int IMPLICIT_CLASS = 1 << 19;
65+
5166
static ImmutableList<Replacement> scan(
5267
JCCompilationUnit unit, String contents, Collection<Range<Integer>> ranges)
5368
throws FormatterException {
5469
RangeSet<Integer> targetRanges = TreeRangeSet.create(ranges);
5570
ImmutableList<Tok> toks = JavaInput.buildToks(contents, ImmutableSet.of());
5671
RangeSet<Integer> activeDeletions = TreeRangeSet.create();
5772
NavigableMap<Integer, StringBuilder> activeInsertions = new TreeMap<>();
58-
new CleanupScanner(unit, toks, targetRanges, activeDeletions, activeInsertions)
73+
new CleanupScanner(contents, unit, toks, targetRanges, activeDeletions, activeInsertions)
5974
.scan(unit, null);
6075
if (activeDeletions.isEmpty() && activeInsertions.isEmpty()) {
6176
return ImmutableList.of();
@@ -108,18 +123,21 @@ private static void verifyCommentsPreserved(
108123
}
109124
}
110125

126+
private final String contents;
111127
private final JCCompilationUnit unit;
112128
private final ImmutableList<Tok> toks;
113129
private final RangeSet<Integer> targetRanges;
114130
private final RangeSet<Integer> activeDeletions;
115131
private final NavigableMap<Integer, StringBuilder> activeInsertions;
116132

117133
CleanupScanner(
134+
String contents,
118135
JCCompilationUnit unit,
119136
ImmutableList<Tok> toks,
120137
RangeSet<Integer> targetRanges,
121138
RangeSet<Integer> activeDeletions,
122139
NavigableMap<Integer, StringBuilder> activeInsertions) {
140+
this.contents = contents;
123141
this.unit = unit;
124142
this.toks = toks;
125143
this.targetRanges = targetRanges;
@@ -135,6 +153,154 @@ private void insert(int pos, String text) {
135153
activeInsertions.computeIfAbsent(pos, k -> new StringBuilder()).append(text);
136154
}
137155

156+
private void deleteRedundantSemicolon(int semiPos) {
157+
int start = semiPos;
158+
while (start > 0 && isHorizontalWhitespace(contents.charAt(start - 1))) {
159+
start--;
160+
}
161+
int lineBegin = start;
162+
while (lineBegin > 0 && !isAtLineStart(contents, lineBegin)) {
163+
lineBegin--;
164+
}
165+
int end = semiPos + 1;
166+
while (end < contents.length() && isHorizontalWhitespace(contents.charAt(end))) {
167+
end++;
168+
}
169+
boolean nothingBeforeOnLine =
170+
start == lineBegin || activeDeletions.encloses(Range.closedOpen(lineBegin, start));
171+
if (nothingBeforeOnLine && end < contents.length()) {
172+
if (contents.charAt(end) == '\r') {
173+
end++;
174+
if (end < contents.length() && contents.charAt(end) == '\n') {
175+
end++;
176+
}
177+
} else if (contents.charAt(end) == '\n') {
178+
end++;
179+
}
180+
}
181+
activeDeletions.add(Range.closedOpen(start, end));
182+
}
183+
184+
private static boolean isHorizontalWhitespace(char c) {
185+
return c == ' ' || c == '\t';
186+
}
187+
188+
private static boolean isAtLineStart(String contents, int pos) {
189+
return pos == 0 || contents.charAt(pos - 1) == '\n' || contents.charAt(pos - 1) == '\r';
190+
}
191+
192+
@Override
193+
public @Nullable Void visitCompilationUnit(CompilationUnitTree node, @Nullable Void unused) {
194+
checkTopLevelSemicolons(node);
195+
return super.visitCompilationUnit(node, null);
196+
}
197+
198+
// Top-level semicolons between or after type declarations are represented as EmptyStatementTree
199+
// nodes in CompilationUnitTree.getTypeDecls(), but javac omits top-level semicolons from the AST
200+
// in some cases:
201+
// - In compilation units with no type declarations (e.g. only package and/or import declarations
202+
// followed by extra semicolons), JavacParser drops trailing semicolons at EOF without adding
203+
// them to getTypeDecls().
204+
// - In compact source files (implicitly declared classes), JavacParser.constructImplicitClass
205+
// strips all JCSkip (EmptyStatementTree) nodes when constructing the synthetic ClassDef.
206+
// Scan token gaps between top-level declarations so all redundant top-level semicolons are
207+
// handled uniformly.
208+
private void checkTopLevelSemicolons(CompilationUnitTree node) {
209+
RangeSet<Integer> validSpans = TreeRangeSet.create();
210+
if (node.getPackage() != null) {
211+
addNodeSpan(validSpans, node.getPackage());
212+
}
213+
if (node.getModule() != null) {
214+
addNodeSpan(validSpans, node.getModule());
215+
}
216+
for (ImportTree imp : node.getImports()) {
217+
addNodeSpan(validSpans, imp);
218+
}
219+
for (Tree decl : node.getTypeDecls()) {
220+
switch (decl) {
221+
case EmptyStatementTree unused -> {}
222+
case ClassTree classTree
223+
when (TreeInfo.flags((JCTree) classTree) & IMPLICIT_CLASS) == IMPLICIT_CLASS -> {
224+
for (Tree member : classTree.getMembers()) {
225+
if ((TreeInfo.flags((JCTree) member) & GENERATED_MEMBER) == 0) {
226+
addNodeSpan(validSpans, member);
227+
}
228+
}
229+
}
230+
default -> addNodeSpan(validSpans, decl);
231+
}
232+
}
233+
deleteSemicolonsInGaps(0, contents.length(), validSpans);
234+
}
235+
236+
private void addNodeSpan(RangeSet<Integer> spans, Tree tree) {
237+
spans.add(Range.closedOpen(getStartPosition(tree), getEndPosition(tree, unit)));
238+
}
239+
240+
@Override
241+
public @Nullable Void visitClass(ClassTree tree, @Nullable Void unused) {
242+
checkClassSemicolons(tree);
243+
return super.visitClass(tree, null);
244+
}
245+
246+
private void checkClassSemicolons(ClassTree tree) {
247+
if ((TreeInfo.flags((JCTree) tree) & IMPLICIT_CLASS) == IMPLICIT_CLASS
248+
|| tree.getMembers() == null) {
249+
return;
250+
}
251+
int classStart = getStartPosition(tree);
252+
int classEnd = getEndPosition(tree, unit);
253+
RangeSet<Integer> memberSpans = TreeRangeSet.create();
254+
// Record the spans of constructor arguments in `new MyClass(arg1, arg2) {...}` and enum
255+
// constants with class bodies `FOO(arg1, arg2) {...}` (where javac sets the anonymous
256+
// ClassTree's start position to the enum constant identifier before the argument list), so
257+
// semicolons inside argument expressions (such as lambdas) do not show up as gaps in the
258+
// class body.
259+
if (getCurrentPath().getParentPath().getLeaf() instanceof NewClassTree newClass) {
260+
for (Tree arg : newClass.getArguments()) {
261+
addNodeSpan(memberSpans, arg);
262+
}
263+
}
264+
int lastEnumConstantEnd = classStart;
265+
int firstNonEnumMemberStart = classEnd;
266+
for (Tree member : tree.getMembers()) {
267+
long flags = TreeInfo.flags((JCTree) member);
268+
if ((flags & GENERATED_MEMBER) != 0) {
269+
continue;
270+
}
271+
int mStart = getStartPosition(member);
272+
int mEnd = getEndPosition(member, unit);
273+
memberSpans.add(Range.closedOpen(mStart, mEnd));
274+
if (tree.getKind() == Tree.Kind.ENUM) {
275+
if (member instanceof VariableTree && (flags & ENUM) != 0) {
276+
lastEnumConstantEnd = max(lastEnumConstantEnd, mEnd);
277+
} else {
278+
firstNonEnumMemberStart = min(firstNonEnumMemberStart, mStart);
279+
}
280+
}
281+
}
282+
if (tree.getKind() == Tree.Kind.ENUM && !memberSpans.isEmpty()) {
283+
Tok enumSemi = findTokenAtOrAfter(lastEnumConstantEnd, firstNonEnumMemberStart, ";");
284+
if (enumSemi != null) {
285+
memberSpans.add(
286+
Range.closedOpen(enumSemi.getPosition(), enumSemi.getPosition() + enumSemi.length()));
287+
}
288+
}
289+
deleteSemicolonsInGaps(classStart, classEnd, memberSpans);
290+
}
291+
292+
private void deleteSemicolonsInGaps(int start, int end, RangeSet<Integer> validSpans) {
293+
for (Range<Integer> gap :
294+
validSpans.complement().subRangeSet(Range.closedOpen(start, end)).asRanges()) {
295+
for (Tok semi : semicolonTokensInRange(gap.lowerEndpoint(), gap.upperEndpoint())) {
296+
int pos = semi.getPosition();
297+
if (inRange(pos, pos + 1)) {
298+
deleteRedundantSemicolon(pos);
299+
}
300+
}
301+
}
302+
}
303+
138304
@Override
139305
public @Nullable Void visitIf(IfTree tree, @Nullable Void unused) {
140306
super.visitIf(tree, null);
@@ -214,6 +380,20 @@ private int closingBraceInsertionPos(StatementTree stmt) {
214380
return stmtEnd;
215381
}
216382

383+
@Override
384+
public @Nullable Void visitEmptyStatement(EmptyStatementTree tree, @Nullable Void unused) {
385+
// Include CaseTree for empty statements in colon-style switch statement groups
386+
// (e.g. `switch (x) { case FOO: ; case BAR: }`).
387+
Tree parent = getCurrentPath().getParentPath().getLeaf();
388+
if (parent instanceof BlockTree || parent instanceof CaseTree) {
389+
int pos = getStartPosition(tree);
390+
if (inRange(pos, pos + 1)) {
391+
deleteRedundantSemicolon(pos);
392+
}
393+
}
394+
return null;
395+
}
396+
217397
// Hand-rolled lower-bound binary search to avoid allocating a dummy Tok or wrapper list for
218398
// Collections.binarySearch.
219399
private int tokIndexAtOrAfter(int pos) {
@@ -239,4 +419,31 @@ private int tokIndexAtOrAfter(int pos) {
239419
}
240420
return null;
241421
}
422+
423+
private @Nullable Tok findTokenAtOrAfter(int startPos, int endPos, String text) {
424+
for (int i = tokIndexAtOrAfter(startPos); i < toks.size(); i++) {
425+
Tok tok = toks.get(i);
426+
if (tok.getPosition() >= endPos) {
427+
break;
428+
}
429+
if (tok.isToken() && tok.getText().equals(text)) {
430+
return tok;
431+
}
432+
}
433+
return null;
434+
}
435+
436+
private List<Tok> semicolonTokensInRange(int startPos, int endPos) {
437+
List<Tok> result = new ArrayList<>();
438+
for (int i = tokIndexAtOrAfter(startPos); i < toks.size(); i++) {
439+
Tok tok = toks.get(i);
440+
if (tok.getPosition() >= endPos) {
441+
break;
442+
}
443+
if (tok.isToken() && tok.getText().equals(";")) {
444+
result.add(tok);
445+
}
446+
}
447+
return result;
448+
}
242449
}

‎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 (default is true).
60+
Enables non-whitespace cleanups, such as inserting missing braces and 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)