From ad2c3780979101a32561163c2b5b189d20a93ca2 Mon Sep 17 00:00:00 2001 From: Bishad Deb Roy Utsha Date: Sat, 23 Nov 2024 18:32:25 -0400 Subject: [PATCH] fix-#1199/refactor-implementation-smells --- .../java/CommandLineOptionsParser.java | 24 ++- .../googlejavaformat/java/ImportOrderer.java | 140 +++++++++++------- .../java/JavaCommentsHelper.java | 57 ++++++- 3 files changed, 153 insertions(+), 68 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java index 52a5e05d4..5278f34e3 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java +++ b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java @@ -168,17 +168,27 @@ private static void parseRangeSet(RangeSet result, String ranges) { * converted here to {@code 0}-based. */ private static Range parseRange(String arg) { + final int SINGLE_LINE = 1; + final int RANGE_LINE = 2; + final int BASE_OFFSET = 1; + List args = COLON_SPLITTER.splitToList(arg); + switch (args.size()) { - case 1: - int line = Integer.parseInt(args.get(0)) - 1; + case SINGLE_LINE: + // Handle single line case (e.g., "42") + int line = Integer.parseInt(args.get(0)) - BASE_OFFSET; return Range.closedOpen(line, line + 1); - case 2: - int line0 = Integer.parseInt(args.get(0)) - 1; - int line1 = Integer.parseInt(args.get(1)) - 1; - return Range.closedOpen(line0, line1 + 1); + + case RANGE_LINE: + // Handle line range case (e.g., "1:12") + int startLine = Integer.parseInt(args.get(0)) - BASE_OFFSET; + int endLine = Integer.parseInt(args.get(1)) - BASE_OFFSET; + return Range.closedOpen(startLine, endLine + 1); + default: - throw new IllegalArgumentException(arg); + throw new IllegalArgumentException("Invalid range format: " + arg + + ". Expected format: 'lineNumber' or 'startLine:endLine'"); } } diff --git a/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java b/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java index dcbaea172..fe0d13043 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java +++ b/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java @@ -296,70 +296,104 @@ private static class ImportsAndIndex { private ImportsAndIndex scanImports(int i) throws FormatterException { int afterLastImport = i; ImmutableSortedSet.Builder imports = ImmutableSortedSet.orderedBy(importComparator); - // JavaInput.buildToks appends a zero-width EOF token after all tokens. It won't match any - // of our tests here and protects us from running off the end of the toks list. Since it is - // zero-width it doesn't matter if we include it in our string concatenation at the end. + while (i < toks.size() && tokenAt(i).equals("import")) { - i++; - if (isSpaceToken(i)) { - i++; - } - boolean isStatic = tokenAt(i).equals("static"); - if (isStatic) { - i++; - if (isSpaceToken(i)) { - i++; - } - } - if (!isIdentifierToken(i)) { - throw new FormatterException("Unexpected token after import: " + tokenAt(i)); - } - StringAndIndex imported = scanImported(i); - String importedName = imported.string; - i = imported.index; - if (isSpaceToken(i)) { - i++; - } - if (!tokenAt(i).equals(";")) { - throw new FormatterException("Expected ; after import"); - } - while (tokenAt(i).equals(";")) { - // Extra semicolons are not allowed by the JLS but are accepted by javac. + ImportAndIndex result = scanSingleImport(i); + imports.add(result.importStatement); + i = result.index; + afterLastImport = i; + + // Skip any whitespace but preserve comment positions + while (isNewlineToken(i) || isSpaceToken(i)) { i++; } - StringBuilder trailing = new StringBuilder(); + } + + return new ImportsAndIndex(imports.build(), afterLastImport); + } + + private static class ImportAndIndex { + final Import importStatement; + final int index; + + ImportAndIndex(Import importStatement, int index) { + this.importStatement = importStatement; + this.index = index; + } + } + + private ImportAndIndex scanSingleImport(int i) throws FormatterException { + // Skip 'import' and following space + i++; + if (isSpaceToken(i)) { + i++; + } + + // Handle static keyword + boolean isStatic = tokenAt(i).equals("static"); + if (isStatic) { + i++; if (isSpaceToken(i)) { - trailing.append(tokenAt(i)); i++; } + } + + if (!isIdentifierToken(i)) { + throw new FormatterException("Unexpected token after import: " + tokenAt(i)); + } + + StringAndIndex imported = scanImported(i); + String importedName = imported.string; + i = imported.index; + + // Handle semicolon and spaces + if (isSpaceToken(i)) { + i++; + } + if (!tokenAt(i).equals(";")) { + throw new FormatterException("Expected ; after import"); + } + + // Collect all trailing content (including comments) + StringBuilder trailing = new StringBuilder(); + i = collectTrailingContent(i, trailing); + + Import importStatement = new Import(importedName, trailing.toString(), isStatic); + return new ImportAndIndex(importStatement, i); + } + + private int collectTrailingContent(int i, StringBuilder trailing) { + // Collect all semicolons + while (tokenAt(i).equals(";")) { + i++; + } + + // Collect whitespace and newline + if (isSpaceToken(i)) { + trailing.append(tokenAt(i)); + i++; + } + if (isNewlineToken(i)) { + trailing.append(tokenAt(i)); + i++; + } + + // Collect comments and their newlines + while (isSlashSlashCommentToken(i)) { + trailing.append(tokenAt(i)); + i++; if (isNewlineToken(i)) { trailing.append(tokenAt(i)); i++; } - // Gather (if any) all single line comments and accompanied line terminators following this - // import - while (isSlashSlashCommentToken(i)) { - trailing.append(tokenAt(i)); - i++; - if (isNewlineToken(i)) { - trailing.append(tokenAt(i)); - i++; - } - } - while (tokenAt(i).equals(";")) { - // Extra semicolons are not allowed by the JLS but are accepted by javac. - i++; - } - imports.add(new Import(importedName, trailing.toString(), isStatic)); - // Remember the position just after the import we just saw, before skipping blank lines. - // If the next thing after the blank lines is not another import then we don't want to - // include those blank lines in the text to be replaced. - afterLastImport = i; - while (isNewlineToken(i) || isSpaceToken(i)) { - i++; - } } - return new ImportsAndIndex(imports.build(), afterLastImport); + + // Collect any remaining semicolons + while (tokenAt(i).equals(";")) { + i++; + } + + return i; } // Produces the sorted output based on the imports we have scanned. diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaCommentsHelper.java b/core/src/main/java/com/google/googlejavaformat/java/JavaCommentsHelper.java index 9526b892c..3f0947bc0 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaCommentsHelper.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaCommentsHelper.java @@ -42,28 +42,69 @@ public String rewrite(Tok tok, int maxWidth, int column0) { if (!tok.isComment()) { return tok.getOriginalText(); } + + String text = formatJavadocIfNeeded(tok, column0); + List lines = parseCommentLines(tok, text); + + return formatComment(tok, lines, column0); + } + + /** + * Format javadoc comment if needed + */ + private String formatJavadocIfNeeded(Tok tok, int column0) { String text = tok.getOriginalText(); if (tok.isJavadocComment() && options.formatJavadoc()) { - text = JavadocFormatter.formatJavadoc(text, column0); + return JavadocFormatter.formatJavadoc(text, column0); } + return text; + } + + /** + * Parse comment into lines with appropriate trimming + */ + private List parseCommentLines(Tok tok, String text) { List lines = new ArrayList<>(); Iterator it = Newlines.lineIterator(text); + while (it.hasNext()) { + String line = it.next(); if (tok.isSlashSlashComment()) { - lines.add(CharMatcher.whitespace().trimFrom(it.next())); + lines.add(CharMatcher.whitespace().trimFrom(line)); } else { - lines.add(CharMatcher.whitespace().trimTrailingFrom(it.next())); + lines.add(CharMatcher.whitespace().trimTrailingFrom(line)); } } + return lines; + } + + /** + * Format the comment based on its type + */ + private String formatComment(Tok tok, List lines, int column0) { if (tok.isSlashSlashComment()) { return indentLineComments(lines, column0); } + + return getFormattedBlockComment(tok, lines, column0); + } + + /** + * Get formatted block comment, either as parameter comment or regular block comment + */ + private String getFormattedBlockComment(Tok tok, List lines, int column0) { return CommentsHelper.reformatParameterComment(tok) - .orElseGet( - () -> - javadocShaped(lines) - ? indentJavadoc(lines, column0) - : preserveIndentation(lines, column0)); + .orElseGet(() -> formatBlockComment(lines, column0)); + } + + /** + * Format block comment based on whether it is javadoc-shaped or not + */ + private String formatBlockComment(List lines, int column0) { + if (javadocShaped(lines)) { + return indentJavadoc(lines, column0); + } + return preserveIndentation(lines, column0); } // For non-javadoc-shaped block comments, shift the entire block to the correct