Repository navigation
Add maxLineLength option to googleJavaFormat step (requires gjf 1.37.0+) - #3152
xzxiaoshan wants to merge 2 commits into
Conversation
Expose the column-limit setting that google-java-format 1.37.0 added via
JavaFormatterOptions.Style.builder().maxLineLength(int):
- Gradle: googleJavaFormat('1.37.0').maxLineLength(240)
- Maven: <googleJavaFormat><version>1.37.0</version><maxLineLength>240</maxLineLength></googleJavaFormat>
The step rejects maxLineLength with gjf older than 1.37.0 at
configuration time with a clear error message. The glue code keeps
compiling against gjf 1.28.0 (so older versions and JDK 17 keep
working) and calls the 1.37.0 builder API reflectively.
google-java-format 1.37.0 turned JavaFormatterOptions.Style from an
enum into a record, which removed Style.valueOf(String) and broke the
existing glue with a NoSuchMethodError. Style names are now resolved
against the GOOGLE/AOSP constants, which exist as static fields in
both shapes, so the step works again on 1.37.0+ regardless of
maxLineLength.
Fixes diffplug#3151
|
On the surface this is a fairly ordinary property addition, but it fulfills something the community has been hoping for over the long term — the underlying upstream request (google/google-java-format#425) stayed open for nearly seven years, and the earlier equivalent request here (#1563) was closed back in 2023 as impossible to implement until upstream made the limit configurable. So finally being able to expose it through Spotless is genuinely meaningful for the many developers who'd like a column limit other than 100. If this PR looks good and can be merged, it would be great to see it included in a release relatively soon so users can start taking advantage of it. Thank you very much for maintaining Spotless and for taking the time to review this contribution! |
jawadshaikst
left a comment
jawadshaikst
left a comment
There was a problem hiding this comment.
+1, would love to get this merged and released.
Archilobe
left a comment
Archilobe
left a comment
There was a problem hiding this comment.
This feature is very important, and I hope it can be merged and released as soon as possible.
| /** | ||
| * Parses a style name without calling {@code Style.valueOf}, which existed only while | ||
| * {@code Style} was an enum (google-java-format {@literal <=} 1.36); since 1.37 it is a record, | ||
| * but the {@code GOOGLE} and {@code AOSP} constants are present in both. | ||
| */ | ||
| @Nonnull | ||
| private static Style toStyle(@Nonnull String style) { | ||
| switch (Objects.requireNonNull(style)) { | ||
| case "GOOGLE": | ||
| return Style.GOOGLE; | ||
| case "AOSP": | ||
| return Style.AOSP; | ||
| default: | ||
| throw new IllegalArgumentException("style must be GOOGLE or AOSP, but was: " + style); | ||
| } | ||
| } |
|
Thanks for the pointer! To make sure I read it right: gjf #1477 (already merged, so it will land in the next gjf release, 1.38.0) restores the enum-compatible Style.valueOf(String)/values()/name() methods precisely so that callers compiled against the old enum — like this glue — keep working, which would also resolve #3126 from the upstream side. Two notes on how this PR interacts with that: toStyle() deliberately resolves names against the GOOGLE/AOSP static constants instead of valueOf(), and those constants exist in every supported version (enum ≤1.36, record 1.37.0, and 1.38+ with #1477), so this PR is forward-compatible with 1.38.0 and is not blocked by it — it also keeps the already-published 1.37.0 usable from Spotless (fixing #3126 for users pinned to it, no need to wait for 1.38). That said, if you would rather standardize on 1.38.0 (gate maxLineLength on it and drop toStyle() in favor of the restored valueOf()), I'm happy to rework the PR that way — just let me know which direction you prefer. |
….0 workaround google/google-java-format#1477 (merged, to be released as gjf 1.38.0) restores the enum-compatible Style.valueOf(String)/values()/name() methods on the record Style, so the glue no longer needs toStyle() to parse style names. Revert the glue to Style.valueOf and raise the maxLineLength gate to 1.38.0, the first version that combines a configurable Style with those restored methods. The short-lived 1.37.0 remains unsupported for Spotless, in line with the upstream fix. The behavior tests now target gjf 1.38.0 and will turn green in CI once that version is published.
|
Reworked per your feedback in ab9a64d: the glue is back to plain Style.valueOf() (toStyle() is gone), and the maxLineLength gate is raised to 1.38.0, the first version that combines a configurable Style with the enum-compatible methods restored by #1477. The behavior/integration tests now target gjf 1.38.0 and will go green in CI once that version ships. Thanks for the pointer! |
|
Please be paitent for the release of the blocker fix. |
Fixes #3151
Summary
Exposes the column-limit setting that google-java-format added via
JavaFormatterOptions.Style.builder().maxLineLength(int)(landed in gjf v1.37.0, resolving google/google-java-format#425, open since Dec 2019 — this was previously impossible because the limit was a compile-time-inlined constant, which is why #1563 was closed back in 2023).Gradle
Maven
Setting
maxLineLengthtogether with an older gjf version fails at configuration time with a clear message (maxLineLength requires google-java-format 1.38.0 or later (you are using 1.37.0)).Based on the final upstream shape (gjf 1.38.0)
As flagged by @Goooler in this review: google/google-java-format#1477 (merged; will be released as gjf 1.38.0) restores the enum-compatible
Style.valueOf(String)/values()/name()methods on the recordStyleexactly so that callers compiled against the old enum — like this glue — keep working, and it closes #3126 from the upstream side. Accordingly this PR now:Style.valueOf(...)(no spotless-side compatibility shim for the short-lived broken 1.37.0), andmaxLineLengthon 1.38.0, the first version that combines a configurableStylewith the restored methods.The reflection for
maxLineLengthremains necessary because the glue still compiles against the pinned gjf 1.28.0 (needed for the JDK-17 tier; 1.37+ is class-file v65 which JDK 17 cannot even read). Reflection methods are looked up on the publicJavaFormatterOptions$Style$Buildertype rather than the builder's runtime class — gjf implements the builder via AutoBuilder and the generated class is package-private, which makesbuilder.getClass().getMethod(...).invoke(...)throwIllegalAccessException.maxLineLengthis part of the step's equality state, so Gradle/Maven correctly re-run when it changes.Testing
maxLineLengthRequiresVersion1Dot38OrLater(config-time error, verified locally on JDK 21)GoogleJavaFormatStepTestcases still pass with the glue back onStyle.valueOf(default gjf path unchanged)RemoveUnusedImportsStep_withGoogleJavaFormatTest,spotlessCheck,spotbugsMain/spotbugsGoogleJavaFormatpassStyle$BuilderAPI as post-Add spotless check and build with tests #1477 master) with a locally published plugin:mvn applyoutput matched byte-for-byte, and the Gradle integration test passedbehaviorWithMaxLineLength/ Gradle / Maven integration tests now target gjf 1.38.0 and will turn green in CI once that version is published (they cannot resolve the dependency before then)