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); } }