Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

licenseHeader treats address as copyright year (regression in plugin-gradle 5.5.0+, plugin-maven 2.3.0+) #716

Closed
smoothreggae opened this issue Oct 15, 2020 · 3 comments
Labels

Comments

@smoothreggae
Copy link

Summary of the problem

I recently upgraded the Spotless Gradle plugin in a Java project I work on from 5.1.0 to 5.6.1 and saw an unexpected change in the copyright header. My project uses a license header template with the $YEAR placeholder that I tell Spotless about using licenseHeaderFile. I had made some additional changes to the source file in question and ran ./gradlew spotlessApply to make sure these changes complied with the configured settings for formatting and indentation. I was surprised to see that the year (2020) copyright header had changed to a range. It looks like this happens if the template contains a number that has at least 4 digits (like a street number or a zip code). It seems like #690 which became available in version 5.5.0 may be the reason.

Here's what I did to see if this was the case:

  • I undid the changes Spotless had made to the file, downgraded the plugin to 5.4.0 and ran ./gradlew spotlessApply and the copyright header was untouched; I even added an errant space in the header and Spotless fixed that without changing the year in the header
  • I undid the changes Spotless made, upgraded the plugin to 5.5.0 and ran ./gradlew spotlessApply and saw the year in the header change to a range

Since the project I was working with is not open source, I worked on putting together a sample project to illustrate the problem. The repository is https://github.com/smoothreggae/spotless-license-header-bug and I have listed the steps to reproduce the problem in README.md

While doing this, I encountered another problem: if a Java source file did not have a copyright header at all and I ran ./gradlew spotlessApply, the spotlessJava task printed out the message "Can't parse copyright year '', defaulting to 2020" before adding the copyright header to the source file. The sample project accounts for this as well.

Is there some other setting that I need to configure now to get this to work like it used to before?

Gradle version

6.6.1

Spotless version

5.5.0

Operating system and version

Windows 10

public repository to demonstrate the problem

https://github.com/smoothreggae/spotless-license-header-bug

@nedtwigg
Copy link
Member

Thanks for the detailed bug report. Your test repository is very helpful, but I think the only command necessary to reproduce after cloning is:

git checkout -f test/5.5.0
./gradlew spotlessApply
git diff

The license header step already has a lot of modes, I'm hesitant to add more. I think it's better to make the modes it has "do the right thing" in more cases. Previous to #690, the step was very demanding in detecting an existing year, for the reason that we were worried about bugs exactly like this one which you have found. But a user came forward with I think is a very good case in #677 that it was very surprising that a license header with just an incorrect whitespace issue would completely reset the license date. So to avoid surprising behavior, I think we have no choice but to be make the year detection smarter.

Make regex more specific

One solution is to make this regex more specific:

private static final Pattern YYYY = Pattern.compile("[0-9]{4}");

here are two ways:

  • require a non-digit before and after (probably sufficient)
  • require regex to start with 19 or 20 (extra security on top of previous change)

Of course, if someone has an address like 1950 Main St, then we would have the same bug and set the copyright year to 2020-1950.

Workaround

One quick fix is something like this:

spotless { java {
  licenseHeader BLAH
  replace 'license header bug', '(C) 2020-1950', '(C) 2020'

As hacky as that is, it seems preferable to me over adding yet another mode.

Deeper fix (a tad tricky)

The deeper problem is that we're detecting a year inside the template. If a template has $YEAR, then we should never grab text outside that position and use it as a date.

One way to do this is something like this:

String licenseTemplate = ... // loaded from script
String existingHeader = ... // parsed from the file content
List<Edit> changes = diff(licenseTemplate, existingHeader) // try to match the existing header to the template
Edit changeWithYear = find edit which contains `$YEAR` on the template side;
String parseYearFromThis = changeWithYear.get(existing header side);

This is a bit harder to build, but it fixes the root problem and wouldn't require regex trickery.

Help wanted

Happy to take PRs for any of the fixes above, or I'm open to other solutions as well. If you want to add more modes, open a draft PR with only the documentation for the change you want to make. I think that documenting more modes will make it clear why we shouldn't add them, but I might be wrong!

@nedtwigg nedtwigg changed the title breaking changes in handling of license templates in Spotless 5.5.0 licenseHeaderStep treats address as copyright year (plugin-gradle 5.5.0+, plugin-maven 2.3.0+) Oct 15, 2020
@nedtwigg nedtwigg changed the title licenseHeaderStep treats address as copyright year (plugin-gradle 5.5.0+, plugin-maven 2.3.0+) licenseHeader treats address as copyright year (plugin-gradle 5.5.0+, plugin-maven 2.3.0+) Oct 15, 2020
@nedtwigg nedtwigg changed the title licenseHeader treats address as copyright year (plugin-gradle 5.5.0+, plugin-maven 2.3.0+) licenseHeader treats address as copyright year (regression in plugin-gradle 5.5.0+, plugin-maven 2.3.0+) Oct 15, 2020
@bugsaw
Copy link
Contributor

bugsaw commented Apr 13, 2021

Hi, I have another solution/workaround for You :)

Start using plugin-gradle 5.12.1 with improved support for range years in lincese, then you need update files:

--- a/build.gradle
+++ b/build.gradle
@@ -2,7 +2,7 @@ plugins {
     id "application"
     id "eclipse"
     id "java"
-    id "com.diffplug.spotless" version "5.1.0"
+    id "com.diffplug.spotless" version "5.12.1"
--- a/src/main/java/com/github/spotless/copyright/HelloWorld.java
+++ b/src/main/java/com/github/spotless/copyright/HelloWorld.java
@@ -1,5 +1,5 @@
 /*
- * Copyright &#169; 2020 FizzBin Inc.  All Rights Reserved.
+ * Copyright &#169; 2020-2020 FizzBin Inc.  All Rights Reserved.

and license template:

--- a/src/spotless/license-template.java
+++ b/src/spotless/license-template.java
@@ -1,5 +1,5 @@
 /*
- * Copyright &#169; $YEAR FizzBin Inc. All Rights Reserved.
+ * Copyright &#169; 2020-$YEAR FizzBin Inc. All Rights Reserved.

and then after spotlessApply:

gradle spotlessApply

updated file looks like this:

> head -12 src/main/java/com/github/spotless/copyright/HelloWorld.java
/*
 * Copyright &#169; 2020-2020 FizzBin Inc. All Rights Reserved.
 *
 * Use of this software is covered by inscrutable legal protection and
 * complex automation. Violaters of undisclosed terms must expect
 * unforeseen consequences.
 *
 * Ephemeral Concepts, Inc.
 * 5 Edelweiss Dr
 * Perry Heights, OH 44646 USA
 */

tada!

@nedtwigg
Copy link
Member

Fixed in plugin-gradle 5.12.2 and plugin-maven 2.10.2.

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

No branches or pull requests

3 participants