diffplug / spotless

Keep your code spotless
Apache License 2.0
4.55k stars 456 forks source link

more flexibility in keeping "sloppy" parts of existing license header #1611

Open planetf1 opened 1 year ago

planetf1 commented 1 year ago

If you are submitting a bug, please include the following:

If you're just submitting a feature request or question, no need for the above.

  1. Regex

Our large codebase has some variations in copyright specifically in terms of spaces between comment chars & the license text

So our checkstyle config (not merged, just trying to find a good tool to add this check since we migrated from maven) becomes

<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
        "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="TreeWalker">

        <module name="Regexp">
            <property name="format" value="/\*[ ]{1,}SPDX-License-Identifier: Apache-2\.0"/>
        </module>

        <module name="Regexp">
            <property name="format" value="/\*[ ]{1,}Copyright Contributors to the( ODPi)? Egeria project"/>
        </module>
    </module>
</module>

That seems good enough, so I'm wondering if I can use spotless instead, as it offers more functionality which we may use in future

Obviously we have have a preferred spacing (for auto fix) but want to tolerate additional spaces

I tried without the comments, but then all files fail the check

Interestingly I also wanted to only report warnings, and note that though multiple errors on a project will now be reported, in a multi module build, the build will terminate as that project build fails (unless --continue is used).

Finally, in the example above I'd need two license header steps, but this seems to not be allowed?

nedtwigg commented 1 year ago

I would read these two resources

To implement what you want, there would still be only one licenseHeader step, but that one step would specify a template which had some kind of regex inside it.

Currently, the license header step doesn't read very much in the existing header. It calculates "this is what the header ought to be", and either it exactly matches or it doesn't.

It does read some things, such as the copyright header. It does this so that it can preserve the existing copyright year and add new years on top.

So it would be possible to add some "softness" into the step, but it would be a new feature. We'd be happy to merge such a feature. I would start by updating the docs for the existing license header step to your dream scenario, and then building to that spec.

blacelle commented 1 year ago

Given https://github.com/diffplug/spotless/issues/1607, could this be achieved with 2 licenseHeader steps, each with the proper skipStep('XXX').forFiles('Blah.java') clause? This would argue in favor of steps having an alias (to seggregate between multiple steps with the same com.diffplug.spotless.FormatterStepImpl#getName).

planetf1 commented 1 year ago

@blacelle Thanks for the tip. that's useful for when any of us look again at these checks. For v4 we're likely leave as-is in the interests of time.

nedtwigg commented 1 year ago

@blacelle I agree that is possible, but it's such an advanced move I'm a bit worried about how to document it, versus adding something like $allow(regex) e.g. $allow(\s*) into the template.