From 6b868494b55eb91f322abd1af41deb434fbe133d Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Thu, 1 Oct 2026 02:11:03 -0700 Subject: [PATCH] Add --max-line-length and --style options, and programmatic tab support, to google-java-format. This adds support for configurable code styles to google-java-format, including: - `--max-line-length`: configures the column limit (default 100). - `--style=google|aosp`: selects the code style, preserving any configured line length or tab settings. If multiple style options are given, the last one wins. - `JavaFormatterOptions.Style.Builder.useTabs(boolean)`: programmatic support for tab-based indentation. Related issues: * Line lengths: https://github.com/google/google-java-format/issues/425 * Tab-based indents: https://github.com/google/google-java-format/issues/1320 * `--style=` flag: https://github.com/google/google-java-format/pull/1415 PiperOrigin-RevId: 991524639 --- .../java/CommandLineOptions.java | 16 +- .../java/CommandLineOptionsParser.java | 23 ++- .../java/FormatFileCallable.java | 2 +- .../googlejavaformat/java/Formatter.java | 10 +- .../googlejavaformat/java/ImportOrderer.java | 18 +- .../java/JavaCommentsHelper.java | 15 +- .../java/JavaFormatterOptions.java | 131 +++++++++++- .../googlejavaformat/java/JavaOutput.java | 21 +- .../google/googlejavaformat/java/Main.java | 6 +- .../googlejavaformat/java/StringWrapper.java | 43 ++-- .../googlejavaformat/java/UsageException.java | 5 + .../java/javadoc/JavadocFormatter.java | 21 +- .../java/javadoc/JavadocWriter.java | 8 +- .../java/CommandLineOptionsParserTest.java | 52 +++++ .../googlejavaformat/java/FormatterTest.java | 187 ++++++++++++++++++ .../googlejavaformat/java/MainTest.java | 32 +++ .../java/StringWrapperTest.java | 35 ++++ 17 files changed, 558 insertions(+), 67 deletions(-) diff --git a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java index 698a3ea9f..873e310c9 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java +++ b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java @@ -28,7 +28,7 @@ * @param lines Line ranges to format. * @param offsets Character offsets for partial formatting, paired with {@code lengths}. * @param lengths Partial formatting region lengths, paired with {@code offsets}. - * @param aosp Use AOSP style instead of Google Style (4-space indentation). + * @param style Code style configuration. * @param version Print the version. * @param help Print usage information. * @param stdin Format input from stdin. @@ -47,7 +47,7 @@ record CommandLineOptions( ImmutableRangeSet lines, ImmutableList offsets, ImmutableList lengths, - boolean aosp, + JavaFormatterOptions.Style style, boolean version, boolean help, boolean stdin, @@ -61,6 +61,14 @@ record CommandLineOptions( boolean formatJavadoc, boolean reorderModifiers) { + boolean aosp() { + return style().isAosp(); + } + + int maxLineLength() { + return style().maxLineLength(); + } + /** Returns true if partial formatting was selected. */ boolean isSelection() { return !lines().isEmpty() || !offsets().isEmpty() || !lengths().isEmpty(); @@ -68,12 +76,12 @@ boolean isSelection() { static Builder builder() { return new AutoBuilder_CommandLineOptions_Builder() + .style(JavaFormatterOptions.Style.GOOGLE) .sortImports(true) .removeUnusedImports(true) .reflowLongStrings(true) .formatJavadoc(true) .reorderModifiers(true) - .aosp(false) .version(false) .help(false) .stdin(false) @@ -108,7 +116,7 @@ default Builder addLength(Integer length) { return this; } - Builder aosp(boolean aosp); + Builder style(JavaFormatterOptions.Style style); Builder version(boolean version); 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 9674a14a1..fe27840b7 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java +++ b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java @@ -42,6 +42,7 @@ final class CommandLineOptionsParser { /** Parses {@link CommandLineOptions}. */ static CommandLineOptions parse(Iterable options) { CommandLineOptions.Builder optionsBuilder = CommandLineOptions.builder(); + JavaFormatterOptions.Style.Builder styleBuilder = JavaFormatterOptions.Style.GOOGLE.toBuilder(); List expandedOptions = new ArrayList<>(); expandParamsFiles(options, expandedOptions); Iterator it = expandedOptions.iterator(); @@ -71,8 +72,17 @@ static CommandLineOptions parse(Iterable options) { parseRangeSet(linesBuilder, getValue(flag, it, value)); case "--offset", "-offset" -> optionsBuilder.addOffset(parseInteger(it, flag, value)); case "--length", "-length" -> optionsBuilder.addLength(parseInteger(it, flag, value)); - case "--google-style", "-google-style" -> optionsBuilder.aosp(false); - case "--aosp", "-aosp", "-a" -> optionsBuilder.aosp(true); + case "--google-style", "-google-style" -> styleBuilder.google(); + case "--aosp", "-aosp", "-a" -> styleBuilder.aosp(); + case "--style" -> { + String style = getValue(flag, it, value); + switch (style) { + case "google" -> styleBuilder.google(); + case "aosp" -> styleBuilder.aosp(); + default -> + throw new IllegalArgumentException(String.format("invalid style value: %s", style)); + } + } case "--version", "-version", "-v" -> optionsBuilder.version(true); case "--help", "-help", "-h" -> optionsBuilder.help(true); case "--fix-imports-only" -> optionsBuilder.fixImportsOnly(true); @@ -81,6 +91,14 @@ static CommandLineOptions parse(Iterable options) { case "--skip-reflowing-long-strings" -> optionsBuilder.reflowLongStrings(false); case "--skip-javadoc-formatting" -> optionsBuilder.formatJavadoc(false); case "--skip-reordering-modifiers" -> optionsBuilder.reorderModifiers(false); + case "--max-line-length" -> { + int length = parseInteger(it, flag, value); + if (length <= 0) { + throw new IllegalArgumentException( + String.format("invalid max-line-length: %d (must be positive)", length)); + } + styleBuilder.maxLineLength(length); + } case "-" -> optionsBuilder.stdin(true); case "-n", "--dry-run" -> optionsBuilder.dryRun(true); case "--set-exit-if-changed" -> optionsBuilder.setExitIfChanged(true); @@ -89,6 +107,7 @@ static CommandLineOptions parse(Iterable options) { default -> throw new IllegalArgumentException("unexpected flag: " + flag); } } + optionsBuilder.style(styleBuilder.build()); optionsBuilder.lines(ImmutableRangeSet.copyOf(linesBuilder)); return optionsBuilder.build(); } diff --git a/core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java b/core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java index ce63efe95..d6e3cc793 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java +++ b/core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java @@ -75,7 +75,7 @@ public Result call() { String formatted = formatter.formatSource(input, characterRanges(input).asRanges()); formatted = fixImports(formatted); if (parameters.reflowLongStrings()) { - formatted = StringWrapper.wrap(Formatter.MAX_LINE_LENGTH, formatted, formatter); + formatted = StringWrapper.wrap(options.maxLineLength(), formatted, formatter); } return Result.create(path, input, formatted, /* exception= */ null); } catch (FormatterException e) { diff --git a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java index 6f021b6a2..e8ca411d1 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Formatter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Formatter.java @@ -14,7 +14,6 @@ package com.google.googlejavaformat.java; - import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterators; @@ -90,6 +89,10 @@ public Formatter(JavaFormatterOptions options) { this.options = options; } + JavaFormatterOptions options() { + return options; + } + /** * Construct a {@code Formatter} given a Java compilation unit. Parses the code; builds a {@link * JavaInput} and the corresponding {@link JavaOutput}. @@ -124,7 +127,7 @@ static void format(final JavaInput javaInput, JavaOutput javaOutput, JavaFormatt Newlines.guessLineSeparator(javaInput.getText()), options, markdownJavadocPositions.build()); - doc.computeBreaks(commentsHelper, MAX_LINE_LENGTH, new Doc.State(+0, 0)); + doc.computeBreaks(commentsHelper, options.maxLineLength(), new Doc.State(+0, 0)); doc.write(javaOutput); javaOutput.flush(); } @@ -221,7 +224,8 @@ public ImmutableList getFormatReplacements( new JavaOutput( lineSeparator, javaInput, - new JavaCommentsHelper(lineSeparator, options, ImmutableSet.of())); + new JavaCommentsHelper(lineSeparator, options, ImmutableSet.of()), + options::indentString); try { format(javaInput, javaOutput, options); } catch (FormattingError e) { 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 70611cd21..7fef64f32 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java +++ b/core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java @@ -177,14 +177,16 @@ private ImportOrderer(String text, ImmutableList toks, Style style) { this.text = text; this.toks = toks; this.lineSeparator = Newlines.guessLineSeparator(text); - if (style.equals(Style.GOOGLE)) { - this.importComparator = GOOGLE_IMPORT_COMPARATOR; - this.shouldInsertBlankLineFn = ImportOrderer::shouldInsertBlankLineGoogle; - } else if (style.equals(Style.AOSP)) { - this.importComparator = AOSP_IMPORT_COMPARATOR; - this.shouldInsertBlankLineFn = ImportOrderer::shouldInsertBlankLineAosp; - } else { - throw new IllegalArgumentException("Unsupported code style: " + style); + switch (style.importOrder()) { + case GOOGLE -> { + this.importComparator = GOOGLE_IMPORT_COMPARATOR; + this.shouldInsertBlankLineFn = ImportOrderer::shouldInsertBlankLineGoogle; + } + case AOSP -> { + this.importComparator = AOSP_IMPORT_COMPARATOR; + this.shouldInsertBlankLineFn = ImportOrderer::shouldInsertBlankLineAosp; + } + default -> throw new IllegalArgumentException("Unsupported code style: " + style); } } 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 8c9acedcc..846d12034 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaCommentsHelper.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaCommentsHelper.java @@ -51,10 +51,10 @@ public String rewrite(Tok tok, int maxWidth, int column0) { if (tok.isJavadocComment() && options.formatJavadoc()) { if (text.startsWith("///")) { if (markdownJavadocPositions.contains(tok.getPosition())) { - return JavadocFormatter.formatJavadoc(text, column0); + return JavadocFormatter.formatJavadoc(text, column0, options.maxLineLength()); } } else { - text = JavadocFormatter.formatJavadoc(text, column0); + text = JavadocFormatter.formatJavadoc(text, column0, options.maxLineLength()); } } List lines = new ArrayList<>(); @@ -95,8 +95,9 @@ private String preserveIndentation(List lines, int column0) { builder.append(lines.get(0)); // output all trailing lines with plausible indentation + String indentString = options.indentString(column0); for (int i = 1; i < lines.size(); ++i) { - builder.append(lineSeparator).repeat(" ", column0); + builder.append(lineSeparator).append(indentString); // check that startCol is valid index, e.g. for blank lines if (lines.get(i).length() >= startCol) { builder.append(lines.get(i).substring(startCol)); @@ -112,7 +113,7 @@ private String indentLineComments(Tok tok, List lines, int column0) { lines = wrapLineComments(tok, lines, column0); StringBuilder builder = new StringBuilder(); builder.append(lines.get(0).trim()); - String indentString = " ".repeat(column0); + String indentString = options.indentString(column0); for (int i = 1; i < lines.size(); ++i) { builder.append(lineSeparator).append(indentString).append(lines.get(i).trim()); } @@ -146,8 +147,8 @@ private List wrapLineComments(Tok tok, List lines, int column0) result.add(line); continue; } - while (line.length() + column0 > Formatter.MAX_LINE_LENGTH) { - int idx = Formatter.MAX_LINE_LENGTH - column0; + while (line.length() + column0 > options.maxLineLength()) { + int idx = options.maxLineLength() - column0; // only break on whitespace characters, and ignore the leading `// ` while (idx >= 2 && !CharMatcher.whitespace().matches(line.charAt(idx))) { idx--; @@ -169,7 +170,7 @@ private String indentJavadoc(List lines, int column0) { StringBuilder builder = new StringBuilder(); builder.append(lines.get(0).trim()); int indent = column0 + 1; - String indentString = " ".repeat(indent); + String indentString = options.indentString(indent); for (int i = 1; i < lines.size(); ++i) { builder.append(lineSeparator).append(indentString); String line = lines.get(i).trim(); diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java b/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java index 509e0d33d..4a75d3aa0 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java @@ -17,6 +17,7 @@ import static java.util.Objects.requireNonNull; import com.google.auto.value.AutoBuilder; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.Immutable; /** @@ -37,29 +38,143 @@ public record JavaFormatterOptions(boolean formatJavadoc, boolean reorderModifie requireNonNull(style, "style"); } - public enum Style { + /** Code style configuration for layout and imports. */ + @Immutable + public record Style( + int indentationMultiplier, int maxLineLength, boolean useTabs, ImportOrder importOrder) { + public Style { + if (maxLineLength <= 0) { + throw new IllegalArgumentException( + String.format("maxLineLength must be positive, was: %d", maxLineLength)); + } + requireNonNull(importOrder, "importOrder"); + } + /** The default Google Java Style configuration. */ - GOOGLE(1), + public static final Style GOOGLE = builder().google().build(); /** The AOSP-compliant configuration. */ - AOSP(2); + public static final Style AOSP = builder().aosp().build(); + + /** + * Returns the visual column width of a tab stop. + * + *

This matches the standard block indentation width for the style: 2 columns for Google + * Style and 4 columns for AOSP. + */ + public int tabWidth() { + return 2 * indentationMultiplier(); + } + + /** Returns the indentation string for the given visual column width. */ + public String indentString(int indent) { + if (!useTabs()) { + return " ".repeat(indent); + } + int tabWidth = tabWidth(); + return "\t".repeat(indent / tabWidth) + " ".repeat(indent % tabWidth); + } + + /** Returns the visual column width of the given character sequence. */ + public int visualLength(CharSequence input) { + return visualLength(input, 0, input.length()); + } - private final int indentationMultiplier; + /** Returns the visual column width of the given subsequence. */ + public int visualLength(CharSequence input, int start, int end) { + if (!useTabs()) { + return end - start; + } + int tabWidth = tabWidth(); + int column = 0; + for (int i = start; i < end; i++) { + if (input.charAt(i) == '\t') { + column += tabWidth - (column % tabWidth); + } else { + column++; + } + } + return column; + } - Style(int indentationMultiplier) { - this.indentationMultiplier = indentationMultiplier; + public boolean isAosp() { + return importOrder() == ImportOrder.AOSP; } - int indentationMultiplier() { - return indentationMultiplier; + public static Builder builder() { + return new AutoBuilder_JavaFormatterOptions_Style_Builder() + .maxLineLength(100) + .useTabs(false) + .google(); + } + + public Builder toBuilder() { + return new AutoBuilder_JavaFormatterOptions_Style_Builder() + .indentationMultiplier(indentationMultiplier()) + .maxLineLength(maxLineLength()) + .useTabs(useTabs()) + .importOrder(importOrder()); + } + + /** A builder for {@link Style}. */ + @AutoBuilder + public abstract static class Builder { + public abstract Builder indentationMultiplier(int indentationMultiplier); + + public abstract Builder maxLineLength(int maxLineLength); + + public abstract Builder useTabs(boolean useTabs); + + public abstract Builder importOrder(ImportOrder importOrder); + + @CanIgnoreReturnValue + public Builder aosp() { + return indentationMultiplier(2).importOrder(ImportOrder.AOSP); + } + + @CanIgnoreReturnValue + public Builder google() { + return indentationMultiplier(1).importOrder(ImportOrder.GOOGLE); + } + + public abstract Style build(); } } + /** The import order to use. */ + public enum ImportOrder { + GOOGLE, + AOSP, + } + /** Returns the multiplier for the unit of indent. */ public int indentationMultiplier() { return style().indentationMultiplier(); } + public int maxLineLength() { + return style().maxLineLength(); + } + + public boolean useTabs() { + return style().useTabs(); + } + + /** Returns the indentation string for the given visual column width. */ + public String indentString(int indent) { + return style().indentString(indent); + } + + /** Returns the visual column width of the given character sequence. */ + public int visualLength(CharSequence input) { + return style().visualLength(input); + } + + /** Returns the visual column width of the given subsequence. */ + public int visualLength(CharSequence input, int start, int end) { + return style().visualLength(input, start, end); + } + /** Returns the default formatting options. */ public static JavaFormatterOptions defaultOptions() { return builder().build(); diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaOutput.java b/core/src/main/java/com/google/googlejavaformat/java/JavaOutput.java index 497f8cff6..498de6622 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaOutput.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaOutput.java @@ -19,7 +19,6 @@ import com.google.common.base.CharMatcher; import com.google.common.base.MoreObjects; -import com.google.common.base.Strings; import com.google.common.collect.DiscreteDomain; import com.google.common.collect.ImmutableList; import com.google.common.collect.Range; @@ -35,6 +34,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.IntFunction; /* * Throughout this file, {@code i} is an index for input lines, {@code j} is an index for output @@ -50,6 +50,7 @@ public final class JavaOutput extends Output { private final String lineSeparator; private final Input javaInput; // Used to follow along while emitting the output. private final CommentsHelper commentsHelper; // Used to re-flow comments. + private final IntFunction indentFunction; private final Map blankLines = new HashMap<>(); // Info on blank lines. private final RangeSet partialFormatRanges = TreeRangeSet.create(); @@ -68,9 +69,25 @@ public final class JavaOutput extends Output { * @param commentsHelper the {@link CommentsHelper}, used to rewrite comments */ public JavaOutput(String lineSeparator, Input javaInput, CommentsHelper commentsHelper) { + this(lineSeparator, javaInput, commentsHelper, indent -> " ".repeat(Math.max(0, indent))); + } + + /** + * {@code JavaOutput} constructor. + * + * @param javaInput the {@link Input}, used to match up blank lines in the output + * @param commentsHelper the {@link CommentsHelper}, used to rewrite comments + * @param indentFunction function mapping a visual column indent to an indentation string + */ + public JavaOutput( + String lineSeparator, + Input javaInput, + CommentsHelper commentsHelper, + IntFunction indentFunction) { this.lineSeparator = lineSeparator; this.javaInput = javaInput; this.commentsHelper = commentsHelper; + this.indentFunction = indentFunction; kN = javaInput.getkN(); } @@ -179,7 +196,7 @@ public void append(String text, Range range) { @Override public void indent(int indent) { - spacesPending.append(Strings.repeat(" ", indent)); + spacesPending.append(indentFunction.apply(indent)); } /** Flush any incomplete last line, then add the EOF token into our data structures. */ diff --git a/core/src/main/java/com/google/googlejavaformat/java/Main.java b/core/src/main/java/com/google/googlejavaformat/java/Main.java index bfe1ff51f..5efa2d185 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Main.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Main.java @@ -20,7 +20,6 @@ import com.google.common.io.ByteStreams; import com.google.common.util.concurrent.MoreExecutors; -import com.google.googlejavaformat.java.JavaFormatterOptions.Style; import java.io.IOError; import java.io.IOException; import java.io.InputStream; @@ -118,7 +117,7 @@ public int format(String... args) throws UsageException { JavaFormatterOptions options = JavaFormatterOptions.builder() - .style(parameters.aosp() ? Style.AOSP : Style.GOOGLE) + .style(parameters.style()) .formatJavadoc(parameters.formatJavadoc()) .reorderModifiers(parameters.reorderModifiers()) .build(); @@ -279,6 +278,9 @@ static CommandLineOptions processArgs(String... args) throws UsageException { if (parameters.dryRun() && parameters.inPlace()) { throw new UsageException("cannot use --dry-run and --in-place at the same time"); } + if (parameters.maxLineLength() <= 0) { + throw new UsageException("--max-line-length must be positive"); + } return parameters; } } diff --git a/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java b/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java index 52d55adba..449644312 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java +++ b/core/src/main/java/com/google/googlejavaformat/java/StringWrapper.java @@ -22,7 +22,6 @@ import static java.util.stream.Collectors.joining; import com.google.common.base.CharMatcher; -import com.google.common.base.Strings; import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; import com.google.common.collect.Range; @@ -55,7 +54,7 @@ public final class StringWrapper { /** Reflows long string literals in the given Java source code. */ public static String wrap(String input, Formatter formatter) throws FormatterException { - return StringWrapper.wrap(Formatter.MAX_LINE_LENGTH, input, formatter); + return StringWrapper.wrap(formatter.options().maxLineLength(), input, formatter); } /** @@ -63,19 +62,20 @@ public static String wrap(String input, Formatter formatter) throws FormatterExc */ static String wrap(final int columnLimit, String input, Formatter formatter) throws FormatterException { - if (!needWrapping(columnLimit, input)) { + JavaFormatterOptions options = formatter.options(); + if (!needWrapping(columnLimit, input, options)) { // fast path return input; } - TreeRangeMap replacements = getReflowReplacements(columnLimit, input); + TreeRangeMap replacements = getReflowReplacements(columnLimit, input, options); String firstPass = formatter.formatSource(input, replacements.asMapOfRanges().keySet()); if (!firstPass.equals(input)) { // If formatting the replacement ranges resulted in a change, recalculate the replacements on // the updated input. input = firstPass; - replacements = getReflowReplacements(columnLimit, input); + replacements = getReflowReplacements(columnLimit, input, options); } String result = applyReplacements(input, replacements); @@ -101,26 +101,34 @@ static String wrap(final int columnLimit, String input, Formatter formatter) } private static TreeRangeMap getReflowReplacements( - int columnLimit, final String input) throws FormatterException { - return new Reflower(columnLimit, input).getReflowReplacements(); + int columnLimit, final String input, JavaFormatterOptions options) throws FormatterException { + return new Reflower(columnLimit, input, options).getReflowReplacements(); } private static class Reflower { private final String input; private final int columnLimit; + private final JavaFormatterOptions options; private final String separator; private final JCTree.JCCompilationUnit unit; private final Position.LineMap lineMap; - Reflower(int columnLimit, String input) throws FormatterException { + Reflower(int columnLimit, String input, JavaFormatterOptions options) + throws FormatterException { this.columnLimit = columnLimit; this.input = input; + this.options = options; this.separator = Newlines.guessLineSeparator(input); this.unit = parse(input, /* allowStringFolding= */ false); this.lineMap = unit.getLineMap(); } + private int visualColumn(int position) { + int lineStart = lineMap.getStartPosition(lineMap.getLineNumber(position)); + return options.visualLength(input, lineStart, position); + } + TreeRangeMap getReflowReplacements() { // Paths to string literals that extend past the column limit. List longStringLiterals = new ArrayList<>(); @@ -164,7 +172,7 @@ public Void visitLiteral(LiteralTree literalTree, Void aVoid) { while (Newlines.hasNewlineAt(input, lineEnd) == -1) { lineEnd++; } - if (lineMap.getColumnNumber(lineEnd) - 1 <= columnLimit) { + if (visualColumn(lineEnd) <= columnLimit) { return null; } longStringLiterals.add(getCurrentPath()); @@ -190,7 +198,8 @@ private void indentTextBlocks( getLast(initialLines).stripTrailing().length() == getLast(lines).stripTrailing().length(); - String prefix = deindent ? "" : " ".repeat(leadingWhitespace); + String prefix = + deindent ? "" : options.indentString(visualColumn(startPosition + leadingWhitespace)); StringBuilder output = new StringBuilder(prefix).append(initialLines.get(0).stripLeading()); for (int i = 0; i < lines.size(); i++) { @@ -238,7 +247,7 @@ private void wrapLongStrings( // to be wrapped. List flat = flatten(input, unit, path, enclosing, first); // Zero-indexed start column - int startColumn = lineMap.getColumnNumber(getStartPosition(flat.get(0))) - 1; + int startColumn = visualColumn(getStartPosition(flat.get(0))); // Handling leaving trailing non-string tokens at the end of the literal, // e.g. the trailing `);` in `foo("...");`. @@ -253,7 +262,8 @@ private void wrapLongStrings( ImmutableList components = stringComponents(input, unit, flat); replacements.put( Range.closedOpen(getStartPosition(flat.get(0)), getEndPosition(getLast(flat), unit)), - reflow(separator, columnLimit, startColumn, trailing, components, first.get())); + reflow( + separator, columnLimit, startColumn, trailing, components, first.get(), options)); } } } @@ -337,7 +347,8 @@ private static String reflow( int startColumn, int trailing, ImmutableList components, - boolean first0) { + boolean first0, + JavaFormatterOptions options) { // We have space between the start column and the limit to output the first line. // Reserve two spaces for the start and end quotes. int width = columnLimit - startColumn - 2; @@ -375,7 +386,7 @@ private static String reflow( return lines.stream() .collect( joining( - "\"" + separator + Strings.repeat(" ", startColumn + (first0 ? 4 : -2)) + "+ \"", + "\"" + separator + options.indentString(startColumn + (first0 ? 4 : -2)) + "+ \"", "\"", "\"")); } @@ -451,12 +462,12 @@ private static boolean noComments( * Returns true if any lines in the given Java source exceed the column limit, or contain a {@code * """} that could indicate a text block. */ - private static boolean needWrapping(int columnLimit, String input) { + private static boolean needWrapping(int columnLimit, String input, JavaFormatterOptions options) { // TODO(cushon): consider adding Newlines.lineIterable? Iterator it = Newlines.lineIterator(input); while (it.hasNext()) { String line = it.next(); - if (line.length() > columnLimit || line.contains(TEXT_BLOCK_DELIMITER)) { + if (options.visualLength(line) > columnLimit || line.contains(TEXT_BLOCK_DELIMITER)) { return true; } } diff --git a/core/src/main/java/com/google/googlejavaformat/java/UsageException.java b/core/src/main/java/com/google/googlejavaformat/java/UsageException.java index 04652e1b0..a51151ee6 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/UsageException.java +++ b/core/src/main/java/com/google/googlejavaformat/java/UsageException.java @@ -39,6 +39,11 @@ File name to use for diagnostics when formatting standard input (default is true; @@ -99,15 +97,16 @@ public static String formatJavadoc(String input, int blockIndent) { } catch (LexException e) { return input; } - String result = render(tokens, blockIndent, classicJavadoc); + String result = render(tokens, blockIndent, classicJavadoc, maxLineLength); if (classicJavadoc) { - result = makeSingleLineIfPossible(blockIndent, result); + result = makeSingleLineIfPossible(blockIndent, result, maxLineLength); } return result; } - private static String render(List input, int blockIndent, boolean classicJavadoc) { - JavadocWriter output = new JavadocWriter(blockIndent, classicJavadoc); + private static String render( + List input, int blockIndent, boolean classicJavadoc, int maxLineLength) { + JavadocWriter output = new JavadocWriter(blockIndent, classicJavadoc, maxLineLength); for (Token token : input) { switch (token) { case BeginJavadoc unused -> output.writeBeginJavadoc(); @@ -182,21 +181,21 @@ private static T standardize(T token, T standardToken) { * Returns the given string or a one-line version of it (e.g., "∕✱✱ Tests for foos. ✱∕") if it * fits on one line. */ - private static String makeSingleLineIfPossible(int blockIndent, String input) { + private static String makeSingleLineIfPossible(int blockIndent, String input, int maxLineLength) { Matcher matcher = ONE_CONTENT_LINE_PATTERN.matcher(input); if (matcher.matches()) { String line = matcher.group(1); if (line.isEmpty()) { return "/** */"; - } else if (oneLineJavadoc(line, blockIndent)) { + } else if (oneLineJavadoc(line, blockIndent, maxLineLength)) { return "/** " + line + " */"; } } return input; } - private static boolean oneLineJavadoc(String line, int blockIndent) { - int oneLinerContentLength = MAX_LINE_LENGTH - "/** */".length() - blockIndent; + private static boolean oneLineJavadoc(String line, int blockIndent, int maxLineLength) { + int oneLinerContentLength = maxLineLength - "/** */".length() - blockIndent; if (line.length() > oneLinerContentLength) { return false; } diff --git a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java index 67b2eb195..38e1332e9 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java +++ b/core/src/main/java/com/google/googlejavaformat/java/javadoc/JavadocWriter.java @@ -64,6 +64,7 @@ final class JavadocWriter { private final int blockIndent; private final boolean classicJavadoc; + private final int maxLineLength; private final StringBuilder output = new StringBuilder(); /** @@ -83,9 +84,10 @@ final class JavadocWriter { private String indentForMoeEndStripComment = ""; private boolean wroteAnythingSignificant; - JavadocWriter(int blockIndent, boolean classicJavadoc) { + JavadocWriter(int blockIndent, boolean classicJavadoc, int maxLineLength) { this.blockIndent = blockIndent; this.classicJavadoc = classicJavadoc; + this.maxLineLength = maxLineLength; } /** @@ -133,7 +135,7 @@ void writeBeginJavadoc() { writeNewline(); } else { output.append("/// "); - remainingOnLine = JavadocFormatter.MAX_LINE_LENGTH - blockIndent - 4; + remainingOnLine = maxLineLength - blockIndent - 4; } } @@ -545,7 +547,7 @@ private void writeNewline() { private void writeNewline(AutoIndent autoIndent) { writeNewlineStart(); appendSpaces(1); - remainingOnLine = JavadocFormatter.MAX_LINE_LENGTH - blockIndent - (classicJavadoc ? 3 : 4); + remainingOnLine = maxLineLength - blockIndent - (classicJavadoc ? 3 : 4); if (autoIndent == AUTO_INDENT) { String indent = innerIndentString(); output.append(indent); diff --git a/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java b/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java index fd88a6d6b..4ed61d1b4 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java @@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.Range; @@ -54,6 +55,7 @@ public void defaults() { assertThat(options.reflowLongStrings()).isTrue(); assertThat(options.formatJavadoc()).isTrue(); assertThat(options.reorderModifiers()).isTrue(); + assertThat(options.maxLineLength()).isEqualTo(100); } @Test @@ -91,6 +93,17 @@ public void lastStyleWins() { .isTrue(); } + @Test + public void style() { + assertThat(CommandLineOptionsParser.parse(Arrays.asList("--style=google")).aosp()).isFalse(); + assertThat(CommandLineOptionsParser.parse(Arrays.asList("--style", "google")).aosp()).isFalse(); + assertThat(CommandLineOptionsParser.parse(Arrays.asList("--style=aosp")).aosp()).isTrue(); + assertThat(CommandLineOptionsParser.parse(Arrays.asList("--style", "aosp")).aosp()).isTrue(); + assertThrows( + IllegalArgumentException.class, + () -> CommandLineOptionsParser.parse(Arrays.asList("--style=invalid"))); + } + @Test public void help() { assertThat(CommandLineOptionsParser.parse(Arrays.asList("-help")).help()).isTrue(); @@ -223,4 +236,43 @@ public void skipReorderingModifiers() { .reorderModifiers()) .isFalse(); } + + @Test + public void maxLineLength() { + assertThat( + CommandLineOptionsParser.parse(Arrays.asList("--max-line-length", "80")) + .maxLineLength()) + .isEqualTo(80); + assertThat( + CommandLineOptionsParser.parse(Arrays.asList("--max-line-length=120")).maxLineLength()) + .isEqualTo(120); + } + + @Test + public void maxLineLengthNonPositive() { + IllegalArgumentException e = + assertThrows( + IllegalArgumentException.class, + () -> CommandLineOptionsParser.parse(Arrays.asList("--max-line-length=0"))); + assertThat(e).hasMessageThat().contains("invalid max-line-length: 0 (must be positive)"); + + e = + assertThrows( + IllegalArgumentException.class, + () -> CommandLineOptionsParser.parse(Arrays.asList("--max-line-length=-1"))); + assertThat(e).hasMessageThat().contains("invalid max-line-length: -1 (must be positive)"); + } + + @Test + public void styleFlagOrder() { + CommandLineOptions opt1 = + CommandLineOptionsParser.parse(Arrays.asList("--max-line-length=120", "--aosp")); + assertThat(opt1.aosp()).isTrue(); + assertThat(opt1.maxLineLength()).isEqualTo(120); + + CommandLineOptions opt2 = + CommandLineOptionsParser.parse(Arrays.asList("--aosp", "--max-line-length=120")); + assertThat(opt2.aosp()).isTrue(); + assertThat(opt2.maxLineLength()).isEqualTo(120); + } } diff --git a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java index 9ba460428..c6862b6ca 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java @@ -669,4 +669,191 @@ void f() { } """); } + + @Test + public void testI1205() throws Exception { + String input = + """ + public interface Foo { + + private static String foo = + \"\"\" + foo\\ + bar \"\"\"; + } + """; + String formatted = new Formatter().formatSource(input); + assertThat(formatted).isEqualTo(input); + } + + @Test + public void maxLineLength() throws Exception { + String input = + """ + class T { + /** A javadoc comment that is longer than forty columns. */ + void f(int aaaaaaaaaa, int bbbbbbbbbb, int cccccccccc) { + // A line comment that is longer than forty columns. + int x = aaaaaaaaaa + bbbbbbbbbb + cccccccccc; + } + } + """; + Formatter formatter = + new Formatter( + JavaFormatterOptions.builder() + .style(Style.GOOGLE.toBuilder().maxLineLength(40).build()) + .build()); + assertThat(formatter.formatSource(input)) + .isEqualTo( + """ + class T { + /** + * A javadoc comment that is longer + * than forty columns. + */ + void f( + int aaaaaaaaaa, + int bbbbbbbbbb, + int cccccccccc) { + // A line comment that is longer + // than forty columns. + int x = + aaaaaaaaaa + + bbbbbbbbbb + + cccccccccc; + } + } + """); + } + + @Test + public void useTabsGoogleStyle() throws Exception { + String input = + """ + class T { + /** + * Multi-line javadoc + * comment. + */ + void f(int a, int b) { + // multi-line + // comment + int x = aaaaaaaaaa + bbbbbbbbbb + cccccccccc; + } + } + """; + Formatter formatter = + new Formatter( + JavaFormatterOptions.builder() + .style(Style.GOOGLE.toBuilder().useTabs(true).maxLineLength(30).build()) + .build()); + assertThat(formatter.formatSource(input)) + .isEqualTo( + """ + class T { + \t/** + \t * Multi-line javadoc + \t * comment. + \t */ + \tvoid f(int a, int b) { + \t\t// multi-line + \t\t// comment + \t\tint x = + \t\t\t\taaaaaaaaaa + \t\t\t\t\t\t+ bbbbbbbbbb + \t\t\t\t\t\t+ cccccccccc; + \t} + } + """); + } + + @Test + public void useTabsAospStyle() throws Exception { + String input = + """ + class T { + /** + * Multi-line javadoc + * comment. + */ + void f(int a, int b) { + int x = aaaaaaaaaa + bbbbbbbbbb + cccccccccc; + } + } + """; + Formatter formatter = + new Formatter( + JavaFormatterOptions.builder() + .style(Style.AOSP.toBuilder().useTabs(true).maxLineLength(30).build()) + .build()); + assertThat(formatter.formatSource(input)) + .isEqualTo( + """ + class T { + \t/** + \t * Multi-line javadoc + \t * comment. + \t */ + \tvoid f(int a, int b) { + \t\tint x = + \t\t\t\taaaaaaaaaa + \t\t\t\t\t\t+ bbbbbbbbbb + \t\t\t\t\t\t+ cccccccccc; + \t} + } + """); + } + + @Test + public void styleVisualLengthAndIndentString() { + Style google = Style.GOOGLE; + assertThat(google.tabWidth()).isEqualTo(2); + assertThat(google.visualLength("abcd")).isEqualTo(4); + assertThat(google.visualLength("\tab")).isEqualTo(3); + + Style googleTabs = Style.GOOGLE.toBuilder().useTabs(true).build(); + assertThat(googleTabs.tabWidth()).isEqualTo(2); + assertThat(googleTabs.indentString(5)).isEqualTo("\t\t "); + assertThat(googleTabs.visualLength(googleTabs.indentString(5))).isEqualTo(5); + assertThat(googleTabs.visualLength("\t")).isEqualTo(2); + assertThat(googleTabs.visualLength("\t\t")).isEqualTo(4); + assertThat(googleTabs.visualLength(" \t")).isEqualTo(2); + assertThat(googleTabs.visualLength("\tab")).isEqualTo(4); + + Style aospTabs = Style.AOSP.toBuilder().useTabs(true).build(); + assertThat(aospTabs.tabWidth()).isEqualTo(4); + assertThat(aospTabs.indentString(10)).isEqualTo("\t\t "); + assertThat(aospTabs.visualLength(aospTabs.indentString(10))).isEqualTo(10); + assertThat(aospTabs.visualLength("\t")).isEqualTo(4); + assertThat(aospTabs.visualLength(" \t")).isEqualTo(4); + assertThat(google.indentString(0)).isEmpty(); + assertThat(googleTabs.indentString(0)).isEmpty(); + assertThat(aospTabs.indentString(0)).isEmpty(); + } + + @Test + public void maxLineLengthExactBoundary() throws Exception { + // "class T extends S {}" is exactly 20 characters. At column 0 it fits within maxLineLength=20. + // If Doc.State column was initialized > 0, it would break before 'extends'. + String input = "class T extends S {}\n"; + Formatter formatter = + new Formatter( + JavaFormatterOptions.builder() + .style(Style.GOOGLE.toBuilder().maxLineLength(20).build()) + .build()); + assertThat(formatter.formatSource(input)).isEqualTo("class T extends S {}\n"); + } + + @Test + public void maxLineLengthNonPositive() { + IllegalArgumentException e = + assertThrows( + IllegalArgumentException.class, () -> Style.builder().maxLineLength(0).build()); + assertThat(e).hasMessageThat().contains("maxLineLength must be positive, was: 0"); + + e = + assertThrows( + IllegalArgumentException.class, () -> Style.builder().maxLineLength(-1).build()); + assertThat(e).hasMessageThat().contains("maxLineLength must be positive, was: -1"); + } } diff --git a/core/src/test/java/com/google/googlejavaformat/java/MainTest.java b/core/src/test/java/com/google/googlejavaformat/java/MainTest.java index 76b99baaf..08c366f43 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/MainTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/MainTest.java @@ -711,4 +711,36 @@ public void syntaxErrorBeginning() throws Exception { .replace("\n", System.lineSeparator()); assertThat(err.toString()).isEqualTo(expected); } + + @Test + public void maxLineLength() throws Exception { + String input = + """ + class Test { + void f() { + int x = aaaaaaaaaa + bbbbbbbbbb + cccccccccc; + } + } + """; + String expected = + """ + class Test { + void f() { + int x = + aaaaaaaaaa + + bbbbbbbbbb + + cccccccccc; + } + } + """; + InputStream in = new ByteArrayInputStream(input.getBytes(UTF_8)); + StringWriter out = new StringWriter(); + Main main = + new Main( + new PrintWriter(out, true), + new PrintWriter(new BufferedWriter(new OutputStreamWriter(System.err, UTF_8)), true), + in); + assertThat(main.format("--max-line-length=30", "-")).isEqualTo(0); + assertThat(out.toString()).isEqualTo(expected); + } } diff --git a/core/src/test/java/com/google/googlejavaformat/java/StringWrapperTest.java b/core/src/test/java/com/google/googlejavaformat/java/StringWrapperTest.java index afc1533f4..302d4681b 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/StringWrapperTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/StringWrapperTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assume.assumeTrue; +import com.google.googlejavaformat.java.JavaFormatterOptions.Style; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -209,4 +210,38 @@ public class T { String actual = StringWrapper.wrap(100, input, new Formatter()); assertThat(actual).isEqualTo(expected); } + + @Test + public void wrapWithMaxLineLengthAndTabs() throws Exception { + String input = + """ + class T { + String s = "one two three four five six seven eight"; + String tb = + \""" + hello + world + \"""; + } + """; + Formatter formatter = + new Formatter( + JavaFormatterOptions.builder() + .style(Style.GOOGLE.toBuilder().maxLineLength(35).useTabs(true).build()) + .build()); + assertThat(formatter.formatSourceAndFixImports(input)) + .isEqualTo( + """ + class T { + \tString s = + \t\t\t"one two three four five six" + \t\t\t\t\t+ " seven eight"; + \tString tb = + \t\t\t\""" + \t\t\thello + \t\t\tworld + \t\t\t\"""; + } + """); + } }