Skip to content

Commit ad2c378

Browse files
committed
fix-google#1199/refactor-implementation-smells
1 parent 20c526c commit ad2c378

File tree

3 files changed

+153
-68
lines changed

3 files changed

+153
-68
lines changed

Diff for: core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java

+17-7
Original file line numberDiff line numberDiff line change
@@ -168,17 +168,27 @@ private static void parseRangeSet(RangeSet<Integer> result, String ranges) {
168168
* converted here to {@code 0}-based.
169169
*/
170170
private static Range<Integer> parseRange(String arg) {
171+
final int SINGLE_LINE = 1;
172+
final int RANGE_LINE = 2;
173+
final int BASE_OFFSET = 1;
174+
171175
List<String> args = COLON_SPLITTER.splitToList(arg);
176+
172177
switch (args.size()) {
173-
case 1:
174-
int line = Integer.parseInt(args.get(0)) - 1;
178+
case SINGLE_LINE:
179+
// Handle single line case (e.g., "42")
180+
int line = Integer.parseInt(args.get(0)) - BASE_OFFSET;
175181
return Range.closedOpen(line, line + 1);
176-
case 2:
177-
int line0 = Integer.parseInt(args.get(0)) - 1;
178-
int line1 = Integer.parseInt(args.get(1)) - 1;
179-
return Range.closedOpen(line0, line1 + 1);
182+
183+
case RANGE_LINE:
184+
// Handle line range case (e.g., "1:12")
185+
int startLine = Integer.parseInt(args.get(0)) - BASE_OFFSET;
186+
int endLine = Integer.parseInt(args.get(1)) - BASE_OFFSET;
187+
return Range.closedOpen(startLine, endLine + 1);
188+
180189
default:
181-
throw new IllegalArgumentException(arg);
190+
throw new IllegalArgumentException("Invalid range format: " + arg
191+
+ ". Expected format: 'lineNumber' or 'startLine:endLine'");
182192
}
183193
}
184194

Diff for: core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java

+87-53
Original file line numberDiff line numberDiff line change
@@ -296,70 +296,104 @@ private static class ImportsAndIndex {
296296
private ImportsAndIndex scanImports(int i) throws FormatterException {
297297
int afterLastImport = i;
298298
ImmutableSortedSet.Builder<Import> imports = ImmutableSortedSet.orderedBy(importComparator);
299-
// JavaInput.buildToks appends a zero-width EOF token after all tokens. It won't match any
300-
// of our tests here and protects us from running off the end of the toks list. Since it is
301-
// zero-width it doesn't matter if we include it in our string concatenation at the end.
299+
302300
while (i < toks.size() && tokenAt(i).equals("import")) {
303-
i++;
304-
if (isSpaceToken(i)) {
305-
i++;
306-
}
307-
boolean isStatic = tokenAt(i).equals("static");
308-
if (isStatic) {
309-
i++;
310-
if (isSpaceToken(i)) {
311-
i++;
312-
}
313-
}
314-
if (!isIdentifierToken(i)) {
315-
throw new FormatterException("Unexpected token after import: " + tokenAt(i));
316-
}
317-
StringAndIndex imported = scanImported(i);
318-
String importedName = imported.string;
319-
i = imported.index;
320-
if (isSpaceToken(i)) {
321-
i++;
322-
}
323-
if (!tokenAt(i).equals(";")) {
324-
throw new FormatterException("Expected ; after import");
325-
}
326-
while (tokenAt(i).equals(";")) {
327-
// Extra semicolons are not allowed by the JLS but are accepted by javac.
301+
ImportAndIndex result = scanSingleImport(i);
302+
imports.add(result.importStatement);
303+
i = result.index;
304+
afterLastImport = i;
305+
306+
// Skip any whitespace but preserve comment positions
307+
while (isNewlineToken(i) || isSpaceToken(i)) {
328308
i++;
329309
}
330-
StringBuilder trailing = new StringBuilder();
310+
}
311+
312+
return new ImportsAndIndex(imports.build(), afterLastImport);
313+
}
314+
315+
private static class ImportAndIndex {
316+
final Import importStatement;
317+
final int index;
318+
319+
ImportAndIndex(Import importStatement, int index) {
320+
this.importStatement = importStatement;
321+
this.index = index;
322+
}
323+
}
324+
325+
private ImportAndIndex scanSingleImport(int i) throws FormatterException {
326+
// Skip 'import' and following space
327+
i++;
328+
if (isSpaceToken(i)) {
329+
i++;
330+
}
331+
332+
// Handle static keyword
333+
boolean isStatic = tokenAt(i).equals("static");
334+
if (isStatic) {
335+
i++;
331336
if (isSpaceToken(i)) {
332-
trailing.append(tokenAt(i));
333337
i++;
334338
}
339+
}
340+
341+
if (!isIdentifierToken(i)) {
342+
throw new FormatterException("Unexpected token after import: " + tokenAt(i));
343+
}
344+
345+
StringAndIndex imported = scanImported(i);
346+
String importedName = imported.string;
347+
i = imported.index;
348+
349+
// Handle semicolon and spaces
350+
if (isSpaceToken(i)) {
351+
i++;
352+
}
353+
if (!tokenAt(i).equals(";")) {
354+
throw new FormatterException("Expected ; after import");
355+
}
356+
357+
// Collect all trailing content (including comments)
358+
StringBuilder trailing = new StringBuilder();
359+
i = collectTrailingContent(i, trailing);
360+
361+
Import importStatement = new Import(importedName, trailing.toString(), isStatic);
362+
return new ImportAndIndex(importStatement, i);
363+
}
364+
365+
private int collectTrailingContent(int i, StringBuilder trailing) {
366+
// Collect all semicolons
367+
while (tokenAt(i).equals(";")) {
368+
i++;
369+
}
370+
371+
// Collect whitespace and newline
372+
if (isSpaceToken(i)) {
373+
trailing.append(tokenAt(i));
374+
i++;
375+
}
376+
if (isNewlineToken(i)) {
377+
trailing.append(tokenAt(i));
378+
i++;
379+
}
380+
381+
// Collect comments and their newlines
382+
while (isSlashSlashCommentToken(i)) {
383+
trailing.append(tokenAt(i));
384+
i++;
335385
if (isNewlineToken(i)) {
336386
trailing.append(tokenAt(i));
337387
i++;
338388
}
339-
// Gather (if any) all single line comments and accompanied line terminators following this
340-
// import
341-
while (isSlashSlashCommentToken(i)) {
342-
trailing.append(tokenAt(i));
343-
i++;
344-
if (isNewlineToken(i)) {
345-
trailing.append(tokenAt(i));
346-
i++;
347-
}
348-
}
349-
while (tokenAt(i).equals(";")) {
350-
// Extra semicolons are not allowed by the JLS but are accepted by javac.
351-
i++;
352-
}
353-
imports.add(new Import(importedName, trailing.toString(), isStatic));
354-
// Remember the position just after the import we just saw, before skipping blank lines.
355-
// If the next thing after the blank lines is not another import then we don't want to
356-
// include those blank lines in the text to be replaced.
357-
afterLastImport = i;
358-
while (isNewlineToken(i) || isSpaceToken(i)) {
359-
i++;
360-
}
361389
}
362-
return new ImportsAndIndex(imports.build(), afterLastImport);
390+
391+
// Collect any remaining semicolons
392+
while (tokenAt(i).equals(";")) {
393+
i++;
394+
}
395+
396+
return i;
363397
}
364398

365399
// Produces the sorted output based on the imports we have scanned.

Diff for: core/src/main/java/com/google/googlejavaformat/java/JavaCommentsHelper.java

+49-8
Original file line numberDiff line numberDiff line change
@@ -42,28 +42,69 @@ public String rewrite(Tok tok, int maxWidth, int column0) {
4242
if (!tok.isComment()) {
4343
return tok.getOriginalText();
4444
}
45+
46+
String text = formatJavadocIfNeeded(tok, column0);
47+
List<String> lines = parseCommentLines(tok, text);
48+
49+
return formatComment(tok, lines, column0);
50+
}
51+
52+
/**
53+
* Format javadoc comment if needed
54+
*/
55+
private String formatJavadocIfNeeded(Tok tok, int column0) {
4556
String text = tok.getOriginalText();
4657
if (tok.isJavadocComment() && options.formatJavadoc()) {
47-
text = JavadocFormatter.formatJavadoc(text, column0);
58+
return JavadocFormatter.formatJavadoc(text, column0);
4859
}
60+
return text;
61+
}
62+
63+
/**
64+
* Parse comment into lines with appropriate trimming
65+
*/
66+
private List<String> parseCommentLines(Tok tok, String text) {
4967
List<String> lines = new ArrayList<>();
5068
Iterator<String> it = Newlines.lineIterator(text);
69+
5170
while (it.hasNext()) {
71+
String line = it.next();
5272
if (tok.isSlashSlashComment()) {
53-
lines.add(CharMatcher.whitespace().trimFrom(it.next()));
73+
lines.add(CharMatcher.whitespace().trimFrom(line));
5474
} else {
55-
lines.add(CharMatcher.whitespace().trimTrailingFrom(it.next()));
75+
lines.add(CharMatcher.whitespace().trimTrailingFrom(line));
5676
}
5777
}
78+
return lines;
79+
}
80+
81+
/**
82+
* Format the comment based on its type
83+
*/
84+
private String formatComment(Tok tok, List<String> lines, int column0) {
5885
if (tok.isSlashSlashComment()) {
5986
return indentLineComments(lines, column0);
6087
}
88+
89+
return getFormattedBlockComment(tok, lines, column0);
90+
}
91+
92+
/**
93+
* Get formatted block comment, either as parameter comment or regular block comment
94+
*/
95+
private String getFormattedBlockComment(Tok tok, List<String> lines, int column0) {
6196
return CommentsHelper.reformatParameterComment(tok)
62-
.orElseGet(
63-
() ->
64-
javadocShaped(lines)
65-
? indentJavadoc(lines, column0)
66-
: preserveIndentation(lines, column0));
97+
.orElseGet(() -> formatBlockComment(lines, column0));
98+
}
99+
100+
/**
101+
* Format block comment based on whether it is javadoc-shaped or not
102+
*/
103+
private String formatBlockComment(List<String> lines, int column0) {
104+
if (javadocShaped(lines)) {
105+
return indentJavadoc(lines, column0);
106+
}
107+
return preserveIndentation(lines, column0);
67108
}
68109

69110
// For non-javadoc-shaped block comments, shift the entire block to the correct

0 commit comments

Comments
 (0)