Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix spaceBeforeSeparator in Jackson Gradle plugin #2103

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

fandreuz
Copy link

@fandreuz fandreuz commented Apr 30, 2024

spaceBeforeSeparator setter in JsonExtension

Hi, I was trying to set a custom value for spaceBeforeSeparator in Gradle Kotlin DSL, as shown in the Gradle plugin example for the Jackson formatted. However, the relevant property from JacksonJsonConfig was not exposed to JsonExtension.

spaceBeforeSeparator always false

After tweaking the plugin with a local snapshot to add the relevant setter, I noticed that the rule that I was trying to achieve (i.e. spaceBeforeSeparator=true) was not respected by spotless. The effective behavior was that expected with spaceBeforeSeparator=false. I digged a little bit more:

at com.diffplug.spotless.glue.json.JacksonJsonFormatterFunc$SpotlessJsonPrettyPrinter.withSeparators(JacksonJsonFormatterFunc.java:107)
at com.fasterxml.jackson.core.util.DefaultPrettyPrinter.<init>(DefaultPrettyPrinter.java:128)
at com.fasterxml.jackson.core.util.DefaultPrettyPrinter.<init>(DefaultPrettyPrinter.java:104)
at com.diffplug.spotless.glue.json.JacksonJsonFormatterFunc$SpotlessJsonPrettyPrinter.<init>(JacksonJsonFormatterFunc.java:92)
at com.diffplug.spotless.glue.json.JacksonJsonFormatterFunc$SpotlessJsonPrettyPrinter.createInstance(JacksonJsonFormatterFunc.java:100)
at com.diffplug.spotless.glue.json.JacksonJsonFormatterFunc$SpotlessJsonPrettyPrinter.createInstance(JacksonJsonFormatterFunc.java:88)
at com.fasterxml.jackson.databind.ObjectWriter$GeneratorSettings.initialize(ObjectWriter.java:1438)
at com.fasterxml.jackson.databind.ObjectWriter._configureGenerator(ObjectWriter.java:1312)
at com.fasterxml.jackson.databind.ObjectWriter.createGenerator(ObjectWriter.java:751)
at com.fasterxml.jackson.databind.ObjectWriter.writeValueAsString(ObjectWriter.java:1140)
...

SpotlessJsonPrettyPrinter#withSeparators(Separators) is responsible for tweaking _objectFieldValueSeparatorWithSpaces according to the user-provided value of spaceBeforeSeparator. However, this method is called by the default constructor of DefaultPrettyPrinter, before we have a chance to set the value of the field spaceBeforeSeparator. Thus, SpotlessJsonPrettyPrinter#spaceBeforeSeparator is always false when SpotlessJsonPrettyPrinter#withSeparators(Separators) is called.

PR content

In this PR, I address the following:

  • The problem reported in the first paragraph (changes in JsonExtension);
  • The problem reported in the remaining paragraphs (changes in JacksonJsonFormatterFunc).

Let me know if I'm missing something, I've tried the tweaked plugin and I observed the expected behavior.

@nedtwigg
Copy link
Member

Thanks for the PR!

  • needs a spotlessApply
  • needs to update all changelogs: root, plugin-gradle, plugin-maven

@fandreuz
Copy link
Author

fandreuz commented Jul 1, 2024

Hi @nedtwigg, thanks for the feedback! Could you have another look now?

@fandreuz
Copy link
Author

@nedtwigg I'm a little bit confused by the test results, for example:
image

which is caused by my change, and by the fact that spaceBeforeSeparator in JacksonJsonConfig is false by default. It seems like the expected test result aren't accurate, am I missing something @nedtwigg ?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants