Repository navigation
Add maxLineLength option to googleJavaFormat step (requires gjf 1.37.0+) - #3152
xzxiaoshan wants to merge 1 commit 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
There was a problem hiding this comment.
+1, would love to get this merged and released.
Fixes #3151
Summary
Exposes the column-limit setting that google-java-format v1.37.0 added via
JavaFormatterOptions.Style.builder().maxLineLength(int)(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 a gjf version older than 1.37.0 fails at configuration time with a clear message (maxLineLength requires google-java-format 1.37.0 or later (you are using 1.36.1)).Compatibility fix included
Upstream 1.37.0 turned
JavaFormatterOptions.Stylefrom an enum into a record, which removedStyle.valueOf(String)— so the existing glue code would crash withNoSuchMethodErroras soon as a user pins gjf 1.37.0 (and would also break #3130, the default-version bump, if merged as-is). This PR resolves style names against theGOOGLE/AOSPconstants, which exist as static fields in both the enum and the record, so the step works on all supported gjf versions again. There is a new regression test (behavior137) covering gjf 1.37.0 with default settings.Implementation notes
maxLineLengthis set.JavaFormatterOptions$Style$Buildertype rather than the builder's runtime class — gjf 1.37.0 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
GoogleJavaFormatStepTest: newbehaviorWithMaxLineLength,behavior137(gjf 1.37.0 with defaults), andmaxLineLengthRequiresVersion1Dot37OrLater(config-time error) — all pass on JDK 21GoogleJavaFormatStepTestcases still pass (no behavior change whenmaxLineLengthis unset)GoogleJavaFormatIntegrationTest.integrationWithMaxLineLengthpasses (Gradle plugin,spotlessApply+ up-to-date checks)mvn applyoutput matches byte-for-byte, including line endings; the old-version error message renders correctly). The in-repoGoogleJavaFormatTest.specificVersionMaxLineLengthis included for CI (gated on JVM 21+), but couldn't run on my Windows machine because of a pre-existing harness issue there (cmd /c mvnwfails to resolvemvnw.cmdwhen an extension-lessmvnwexists — unrelated to this change; it reproduces on a clean checkout too).spotlessCheckandspotbugsMain/spotbugsGoogleJavaFormatpass