spotify / fmt-maven-plugin

Opinionated Maven Plugin that formats your Java code.
MIT License
326 stars 62 forks source link

Different behaviour to CLI when formatting long strings #159

Closed cgoeller closed 1 year ago

cgoeller commented 1 year ago

Describe the bug The CLI formatter reflows long strings by default (can be deactivated) The maven plugin does not reflow long strings by default (cannot be configured) IntelliJ Plugin also does not reflow long strings by default (cannot be configured)

To Reproduce See example project: https://github.com/cgoeller/fmt-example

Expected behavior The default behavior should be identical.

Additional context Version of CLI 1.15.0 Maven Plugin Verison 2.19 (uses 1.15.0) IntelliJ Plugin Version 1.15.0

I am not sure if this is an issue of the maven plugin or of google-java-format. By looking at the code the new reflow option is enabled by default and not explicitly disabled by the maven-plugin. So why is it not active in the maven plugin?

klaraward commented 1 year ago

Looking at the GJF code it looks like the codepath where the reflowLongString setting is used it only via the GJF main method, and not via the Formatter API. The default value true is set in https://github.com/google/google-java-format/blob/master/core/src/main/java/com/google/googlejavaformat/java/CommandLineOptions.java#L195 and used in https://github.com/google/google-java-format/blob/master/core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java#L47 I would say this is a bug in GJF

cgoeller commented 1 year ago

I created a new issue in GJF. Thanks for your investigation @klaraward

tbroyer commented 1 year ago

Is this really different from import reordering and removing unused imports? It's an additional call to StringWrapper.wrap() on the result from formatter.formatSource()

https://github.com/spotify/fmt-maven-plugin/blob/1cf19c0cb5a88272a50bfafed8935c79b946c62f/src/main/java/com/spotify/fmt/Formatter.java#L130-L134

klaraward commented 1 year ago

Is this really different from import reordering and removing unused imports? It's an additional call to StringWrapper.wrap() on the result from formatter.formatSource()

https://github.com/spotify/fmt-maven-plugin/blob/1cf19c0cb5a88272a50bfafed8935c79b946c62f/src/main/java/com/spotify/fmt/Formatter.java#L130-L134

Agreed, not sure why they put it in the CLI code path only...

cgoeller commented 1 year ago

Things would be easier if there were no configuration options as stated in the project description of GJF.

Note: There is no configurability as to the formatter's algorithm for formatting. This is a deliberate design decision to unify our code formatting on a single format.