From 1fe23b7547eef00f38b924adcc5be63783c1ca37 Mon Sep 17 00:00:00 2001 From: Liam Miller-Cushon Date: Thu, 1 Oct 2026 08:27:19 -0700 Subject: [PATCH] Limit `--max-line-length` in google-java-format to at most 999. The formatter assumes lines won't be longer than `Doc.MAX_LINE_WIDTH` (1000), this limits the value of `--max-line-length` to avoid layout bugs. PiperOrigin-RevId: 991692634 --- .../java/CommandLineOptionsParser.java | 10 +++++---- .../java/JavaFormatterOptions.java | 18 +++++++++++---- .../google/googlejavaformat/java/Main.java | 3 --- .../java/CommandLineOptionsParserTest.java | 22 ++++++++++++++++--- .../googlejavaformat/java/FormatterTest.java | 13 ++++++++--- 5 files changed, 49 insertions(+), 17 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 fe27840b7..35e5a8e00 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java +++ b/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptionsParser.java @@ -14,6 +14,7 @@ package com.google.googlejavaformat.java; +import static com.google.common.base.Preconditions.checkArgument; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.CharMatcher; @@ -93,10 +94,11 @@ static CommandLineOptions parse(Iterable options) { 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)); - } + checkArgument( + length > 0 && length <= JavaFormatterOptions.Style.MAX_LINE_LENGTH_LIMIT, + "invalid max-line-length: %s (must be between 1 and %s)", + length, + JavaFormatterOptions.Style.MAX_LINE_LENGTH_LIMIT); styleBuilder.maxLineLength(length); } case "-" -> optionsBuilder.stdin(true); 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 c2cdf3d03..65fcce4eb 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java +++ b/core/src/main/java/com/google/googlejavaformat/java/JavaFormatterOptions.java @@ -14,11 +14,13 @@ package com.google.googlejavaformat.java; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; import com.google.auto.value.AutoBuilder; import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.Immutable; +import com.google.googlejavaformat.Doc; /** * Options for a google-java-format invocation. @@ -43,13 +45,21 @@ public record JavaFormatterOptions(boolean formatJavadoc, boolean reorderModifie 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)); - } + checkArgument( + maxLineLength > 0 && maxLineLength <= MAX_LINE_LENGTH_LIMIT, + "maxLineLength must be between 1 and %s, was: %s", + MAX_LINE_LENGTH_LIMIT, + maxLineLength); requireNonNull(importOrder, "importOrder"); } + /** + * The largest supported {@link #maxLineLength}. Layout widths saturate at {@link + * Doc#MAX_LINE_WIDTH}, which is also how forced breaks are represented, so the line length must + * be smaller than that. + */ + static final int MAX_LINE_LENGTH_LIMIT = Doc.MAX_LINE_WIDTH - 1; + /** The default Google Java Style configuration. */ public static final Style GOOGLE = builder().google().build(); 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 5efa2d185..19165b24a 100644 --- a/core/src/main/java/com/google/googlejavaformat/java/Main.java +++ b/core/src/main/java/com/google/googlejavaformat/java/Main.java @@ -278,9 +278,6 @@ 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/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java b/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java index 4ed61d1b4..d28ee9f3c 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/CommandLineOptionsParserTest.java @@ -249,18 +249,34 @@ public void maxLineLength() { } @Test - public void maxLineLengthNonPositive() { + public void maxLineLengthOutOfRange() { 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)"); + assertThat(e) + .hasMessageThat() + .contains("invalid max-line-length: 0 (must be between 1 and 999)"); e = assertThrows( IllegalArgumentException.class, () -> CommandLineOptionsParser.parse(Arrays.asList("--max-line-length=-1"))); - assertThat(e).hasMessageThat().contains("invalid max-line-length: -1 (must be positive)"); + assertThat(e) + .hasMessageThat() + .contains("invalid max-line-length: -1 (must be between 1 and 999)"); + + e = + assertThrows( + IllegalArgumentException.class, + () -> CommandLineOptionsParser.parse(Arrays.asList("--max-line-length=1000"))); + assertThat(e) + .hasMessageThat() + .contains("invalid max-line-length: 1000 (must be between 1 and 999)"); + + assertThat( + CommandLineOptionsParser.parse(Arrays.asList("--max-line-length=999")).maxLineLength()) + .isEqualTo(999); } @Test 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 7414e4e50..71a286f50 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java @@ -851,15 +851,22 @@ public void maxLineLengthExactBoundary() throws Exception { } @Test - public void maxLineLengthNonPositive() { + public void maxLineLengthOutOfRange() { IllegalArgumentException e = assertThrows( IllegalArgumentException.class, () -> Style.builder().maxLineLength(0).build()); - assertThat(e).hasMessageThat().contains("maxLineLength must be positive, was: 0"); + assertThat(e).hasMessageThat().contains("maxLineLength must be between 1 and 999, was: 0"); e = assertThrows( IllegalArgumentException.class, () -> Style.builder().maxLineLength(-1).build()); - assertThat(e).hasMessageThat().contains("maxLineLength must be positive, was: -1"); + assertThat(e).hasMessageThat().contains("maxLineLength must be between 1 and 999, was: -1"); + + e = + assertThrows( + IllegalArgumentException.class, () -> Style.builder().maxLineLength(1000).build()); + assertThat(e).hasMessageThat().contains("maxLineLength must be between 1 and 999, was: 1000"); + + assertThat(Style.builder().maxLineLength(999).build().maxLineLength()).isEqualTo(999); } }