palantir-java-format IntelliJ plugin ignores @Formatter:off/on

[SebHo0]

What happened?

The IntelliJ plugin ignores formatter guards (e.g. @Formatter:off/on) even though IntelliJ itself is setup to respect it.

What did you want to happen?

Do not overwrite the formatter guard settings (or at least provide a settings option to respect those).

[obilliard]

Any plan on fixing this?
This indeed breaks the expected IntelliJ setup, as well as the potential Spotless config when combined.

[iamdanfox]

One of the guiding principles of this project was to minimize configurability in order to avoid endless discussions/arguments between developers about 'which formatting looks better'. I would be concerned that building support for special @formatter:off / on comments would allow/encourage folks to demarcate sections of their code where they want to use their own artisanal formatting, which re-opens the door to endless subjective discussions about what looks better.

So consequently I'd actually vote to not implement support for these (unless there's a super compelling use-case I'm not aware of). I think this is consistent with the google-java-format approach (diffplug/spotless#275), gofmt. rustfmt does actually support these kinds of comments though (rustfmt_skip).

Are there snippets or examples where you feel it's essential to use the // @formatter:off comments?

[querdenker2k]

I need this as well. I am having code like this

new ResponseVerifier.ProductionRecordBuilder()
            .verifyArtifact("MyArtefact")
            .verifyDataFields()
            .verifyDataField("DataField1").mustExist().back()
            .verifyDataField("DataField2").mustNotExist().back()
            ...

The palantir plugin will currently put this all in single lines so this is not readable anymore.
Why not taking respect to the IntellJ formatting guards.
I can use the turn formatting-off/on feature with the maven-plugin as well.

[iamdanfox]

@querdenker2k I appreciate the snippet of code you've pasted is arguably better than the p-j-f formatted version:

new ResponseVerifier.ProductionRecordBuilder()
                .verifyArtifact("MyArtefact")
                .verifyDataFields()
                .verifyDataField("DataField1")
                .mustExist()
                .back()
                .verifyDataField("DataField2")
                .mustNotExist()
                .back()

but I'm afraid I don't find this example compelling enough to open the door to // @formatter:off. My concern is that adding this feature opens the door to people arguing about style within those blocks. For example, if you received a pull request with the snippet I pasted above, would you ask the contributor to use the // @formatter:off blocks and chain up the calls like .verifyDataField("DataField2").mustNotExist().back()?

We found somewhat similar cases internally, which led to us calling out the 'downside' of strongly opinionated formatters on the README:

if you don't like how the formatter laid out your code, you may need to introduce new functions/variables

In this case, I'd suggest maybe the verifyDataField() function could take two arguments - a string name and a lambda, within which you could configure the criteria. Then it might look something like the following snippet which palantir-java-format will automatically format:

new ResponseVerifier.ProductionRecordBuilder()
                .verifyArtifact("MyArtefact")
                .verifyDataFields()
                .verifyDataField("DataField1", field -> {
                    field.mustExist();
                })
                .verifyDataField("DataField2", field -> {
                    field.mustNotExist();
                })

[andrej-urvantsev]

One of the arguments to support // @formatter:off could be to make a temporary workarounds for cases like #782

[jamesdh]

I think if a team is aware enough to be using a tool such as PJF, then when they see a code block that has formatting disabled via // @formatter:off then they'll also be aware enough to understand when it's helpful and permitted, and not worth arguing about the semantics of the chosen format. Anytime that's not the case, then the solution is likely to remove the format marker, but let the team decide for themselves.

[jamesdh]

The bigger concern is that this plugin breaks from IntelliJ default behavior, without any option to restore it or even a hint that it does so. The implication is that it modifies how IntelliJ does its formatting, when in fact it seems to replace the formatting function in its entirety.

[jamesdh]

FWIW, diffplug/spotless#275 was also referenced along with the Google Java Formatter earlier as partial justification for not supporting this, but Spotless actually added the ability to disable formatting regardless of the fact GJF doesn't support it.

[philip-rx]

IMHO it makes no sense not to support the // @Formatter:off feature through at least a config setting on the plugin :

• If there's a global rule in a project/team/... to enforce the Spotless rules, then Spotless checks should/will be implemented in the CI/CD builds, and individual devs will have no benefit from turning the option on, since their commits will be refused anyway.
• If the project/team/... allows individual manual formatting, because in some cases the result is just simply a lot better than a "One Ring To Rule Them All" approach, then palantir-java-format does not support that, and becomes useless as a Spotless-enabled formatter. I rely on it heavily, since it's the only plugin I know of in IntelliJ which obeys the Spotless rules.
• There are use cases where Spotless essentially destroys all readability of the entire class. Specifically, in the use case I'm looking at, the combination of annotations and line splitting leads to an an indentation / EOL spaghetti in DTO classes using Java record which actively makes it really hard to spot the essence (the DTO properties).

Let me illustrate with this "anonimized" real-life example. I included 2 variations for the description args, one with multi-line Strings and one with "classic" String addition, which both have their issues in this forced formatting.

public record abcdef(
        @Schema(
                        description =
                                """
    xxxxxxxxxxxxxxxxxx xxxx xxx xxxx xxxx xxxxx xxxxx xxxx xxxx xxxx xxx xxxx xxxxx xxxxxx xxxxxx xxxx xx.
                                """)
                @NotNull
                String xxxxxxxxxx,
        @Schema(
                        description =
                                """
    xxxx xxx xxx xxx xxx xxx xxx xxx xxx xxx xxx xxx xxx x xx x xxxxxxxx xxxxxxx xxx.
                """)
                @NotNull
                UUID xxxxxxxxx,
        @Schema(
                        description = "xxxxxx xxxx xxxxx xxxxx xxxx xxxxx xxxx xxxxx xxxxxx xxxxxx xxxxx xx "
                                + "xxxx xxx xxx xxxx xxxx xxxx xxxx xxxx xxx.",
                        example = "xxxxxx")
                @NotNull
                String xxxxx,
        @Schema(description = "xxx  xxx xxxx xxxx xxxxx xxxx xxxxxxx xxxxxx xxxx.") @NotNull
                List<xxxxxxxxxx> xxxxxxxxxxxxxx),
        @Schema(description = "xxx  xxx.") @NotNull String xxxxxxx)
        implements Serializable {}

I see the following problems with it :

• The annotations and property definitions are not aligned systematically,. Sometimes the NotNull and / or property is inlined and sometimes it is on separate lines. When trying to interpret this code, the reader has to focus extremely hard because the formatting obscures the location of the info so much that the reader needs to jump between the end and start of lines unnecessarily. There is no way that I can see to quickly get a sense of what the properties are or what the meta-properties are.
• The second annotation, if on a new line, is indented extra, so even visually scanning for the annotations requires jumping left and right, which is fatiguing.
• The description property is forced onto a separate line, as are the opening """, so a lot of unnecessary lines are introduced, increasing the vertical spacing of information, which makes for slower reading IMHO.

I could format this in a much more readable way, aligning the annotations, properties etc in such a way that any scanning operation (i.e. quickly visually determining what the properties are, what the annotations are for a given property, ...) are facilitated and become trivial.

With the plugin as-is, pretty much whatever I do will be destroyed though.

One would think that the end goal of any formatting is to get readable maintainable code.
I'd argue that the use case below is completely sabotaged instead. I also get the sense that a lot of arguments pro the "One Ring" approach are based on improved formatting for individual methods/declarations, but I think the example shows that at class-level, the end result of so-called optimal individual formatting of each "element" results in a hot mess instead.

To add insult to injury, one of the promises of records and annotations is to create more declarative, very easy-to-read code.
IMHO, practically nothing is left of that in the example I provided.

[asibkamalsada]

I have to agree with @philip-rx , the auto formatting does not go well with records and annotations.

At least for "normal" classes, one can extract the longer description strings easily. But with records, you would need to extract them into a different file to be accessible... so not that elegant of a solution.