Detect mismatching `spotless:off` / `spotless:on` comments

[Marcono1234]

Description

It appears currently Spotless is not validating whether the spotless:off and spotless:on comments are matching. The following erroneous usages can exist, and are not detected / reported:

• Leading spotless:on without previous spotless:off
• Trailing spotless:off without subsequent spotless:on
• spotless:off followed by spotless:off
• spotless:on followed by spotless:on

The current behavior of Spotless in all these cases is most likely implementation dependent and not consistent / reliable in any way. Currently Spotless may, or may not format sections of code with such mismatching comments. For example in the following case where the second comment for m2 is spotless:off by accident, it only formats m1, but does not format m3:

    void m1() {}

    // spotless:off
    void m2() {}
    // spotless:off

    void m3() {}

    // spotless:off
    void m4() {}
    // spotless:on

It would be good if Spotless detected such mismatching comments before performing any task and reported them, failing the task before changing any of the source files.

Version

Gradle: 7.4.2 Spotless plugin: 6.4.2

[nedtwigg]

That's a good idea. The place to do the check is here:

https://github.com/diffplug/spotless/blob/f1aba173af53b87802b51d49f20b2a3fc1373d49/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java#L157

The tricky part is that the off/on tags actually build a regex, and we already have public API which allows people to use custom regex.

https://github.com/diffplug/spotless/blob/f1aba173af53b87802b51d49f20b2a3fc1373d49/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java#L62-L76

I expect 90%+ of users are just using spotless:off and spotless:on, so I think it would be a great idea to add nullable fields tokenOpen / tokenClose, and set them when appropriate and use them to implement the check. Happy to merge a PR for this.

[Marcono1234]

Happy to merge a PR for this.

(In case this is directed to me) Unfortunately I am not familiar with Spotless' code at all, and haven't really used Spotless much yet either. I mainly noticed the same issue pattern for Eclipse's @formatter comments 'in the wild' and checked whether Spotless is affected by this as well.

The tricky part is that the off/on tags actually build a regex, and we already have public API which allows people to use custom regex.

The check for removed comments in the following line could benefit from such tokenOpen and tokenClose fields as well then, right? https://github.com/diffplug/spotless/blob/f1aba173af53b87802b51d49f20b2a3fc1373d49/lib/src/main/java/com/diffplug/spotless/generic/PipeStepPair.java#L199

[nedtwigg]

I am not familiar with Spotless' code at all

Reporting a bug/enhancement is a great help, and it comes with no obligation to contribute a fix on the part of either the reporter or the maintainers. As a maintainer I try to unblock anyone (present or future) who wants to contribute, and also save someone's time from contributing something we wouldn't merge anyway. I say "PR welcome" only to make it clear that anyone who implements this will not do so in vain :)

the following line could benefit from such tokenOpen and tokenClose fields as well

Good call, that's a complicated regex and it would be better to remove it and just use the hypothetical token fields.