Support license header for Java classes in default package

[sbrannen]

Overview

Disclaimer: I don't condone the use of the default package in Java at all.

However... having said that, we need to support test classes declared in the default package in JUnit 5. So I added a DefaultPackageTestCase to our test suite to verify our support for such use cases.

Consequently, when Spotless is applied during the JUnit 5 builds, we now see the following:

Unable to apply step LicenseHeader to src/test/java/DefaultPackageTestCase.java: Unable to find delimiter regex ^package

Since a Java class in the default package does not have a package declaration, it makes sense that your regular expression would fail to find ^package; however, just because a class exists in the default package shouldn't mean that it does not have the license header rule applied.

Proposal

If a class is in the default package, consider doing one of the following:

1. use a different means for determining the location of the license header file -- for example, prior to ^import, ^/**, or similar.
2. just make sure the license header exists at the top of the document.

[nedtwigg]

TL;DR: My gut is that supporting the default package is more effort and fragility than it's worth. A quick workaround for your usecase is this:

spotless {
    java {
       licenseHeaderFile 'spotless.license.java', '(package|import)'

Long version

When you do this

spotless {
    java {
       licenseHeaderFile 'spotless.license.java'

That's actually shorthand for

spotless {
    java {
       licenseHeaderFile 'spotless.license.java', 'package'

Where 'package' is compiled as a regex like this (^ means beginning of line):

this.delimiterPattern = Pattern.compile('^' + delimiter, Pattern.UNIX_LINES | Pattern.MULTILINE);

If you'd like to support the default package, that can be accomplished by something like this:

spotless {
    java {
       licenseHeaderFile 'spotless.license.java', '(package|import)'

The problem with this workaround is that it will wipe out any comments that happen to be above the first import. So it's tempting to say that it should be (package|import|/**) so that we can put the license above any comments that there might be in the default package. But now, if the user's license happens to have /**, the code will prepend over and over and over. Fixing that would require some fragile logic.

Does the workaround work for you?

[sbrannen]

Hi @nedtwigg,

Thanks for the super fast response!

Does the workaround work for you?

Well... yes and no. 😉

It gets rid of the annoying warning in the build. So that of course makes us happy! 👍

However, it does something odd with blank lines.

For example, it converts this:

/*
 * Copyright 2015-2016 the original author or authors.
 *
 * All rights reserved. This program and the accompanying materials are
 * made available under the terms of the Eclipse Public License v1.0 which
 * accompanies this distribution and is available at
 *
 * http://www.eclipse.org/legal/epl-v10.html
 */

import org.junit.jupiter.api.Test;

to this:

/*
 * Copyright 2015-2016 the original author or authors.
 *
 * All rights reserved. This program and the accompanying materials are
 * made available under the terms of the Eclipse Public License v1.0 which
 * accompanies this distribution and is available at
 *
 * http://www.eclipse.org/legal/epl-v10.html
 */
import org.junit.jupiter.api.Test;

Of course, you can't see the leading blank line in the above, so let me explain in words.

The new regex causes the following:

• a blank line is inserted before the existing license header.
• the existing, intentional blank after the license header is removed.

This behavior is, however, not the case for any Java classes that are not in the default package.

[sbrannen]

FYI: you can see the related changes to the JUnit 5 build here: junit-team/junit5@5ab6116

[nedtwigg]

I think your build looks like this:

spotless {
    licenseHeaderFile
    eclipseFormatFile
    ...

The license header step is putting the license where it's supposed to go, and then the eclipse formatter is screwing it up, because the eclipse formatter doesn't work very well with the default package either.

To fix this, just swap the order (apply the eclipse formatter first, and then the license header).

Also, I found some cases which further complicate the regex, which convinces me further that Spotless won't support the default package out-of-the-box. You might have a class with no imports at all, in which case the first line will either be a comment or the class definition, which might or might not be public. So the regex sorta needs to be (package|import|public|class|/\\*), and there's a lot that could go wrong...

[sbrannen]

Hmmmm.... swapping the eclipse formatter and license header steps ended up modifying every single Java file in the project, b/c it then removed the blank line we have between the license header and the package declaration.

So we'll just stick with the previous results from the '(package|import) ' tip you gave us. 😉

[sbrannen]

You might have a class with no imports at all,

Yeah, that was actually the case for us. That's why I added the unnecessary import for @Test in junit-team/junit5@5ab6116.

[sbrannen]

there's a lot that could go wrong...

Agreed.

So feel free to close this as "won't fix" if you like.

Thanks for all of your assistance!

[nedtwigg]

Thanks for raising the issue! It'll be a useful reference for anyone else who bumps into this :)