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

add a $YEAR as a supported variable in license #179

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.io.Serializable;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.time.YearMonth;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -35,6 +36,11 @@ public final class LicenseHeaderStep implements Serializable {

private final String licenseHeader;
private final Pattern delimiterPattern;
private Pattern yearMatcherPattern;
private boolean hasYearToken;
private String licenseHeaderBeforeYEARToken;
private String licenseHeaderAfterYEARToken;
private String licenseHeaderWithYEARTokenReplaced;

/** Creates a FormatterStep which forces the start of each file to match a license header. */
public static FormatterStep createFromHeader(String licenseHeader, String delimiter) {
Expand Down Expand Up @@ -74,6 +80,14 @@ private LicenseHeaderStep(String licenseHeader, String delimiter) {
}
this.licenseHeader = licenseHeader;
this.delimiterPattern = Pattern.compile('^' + delimiter, Pattern.UNIX_LINES | Pattern.MULTILINE);
hasYearToken = licenseHeader.contains("$YEAR");
if (hasYearToken) {
int yearTokenIndex = licenseHeader.indexOf("$YEAR");
licenseHeaderBeforeYEARToken = licenseHeader.substring(0, yearTokenIndex);
licenseHeaderAfterYEARToken = licenseHeader.substring(yearTokenIndex + 5, licenseHeader.length());
licenseHeaderWithYEARTokenReplaced = licenseHeader.replace("$YEAR", String.valueOf(YearMonth.now().getYear()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor niggle: I'd ideally like to see instances of YEAR in field names turned into Year, to be consistent with the rest of the code.

Just to clarify, there's no need to change strings like $YEAR starting with a dollar sign to $Year - just field names. :)

this.yearMatcherPattern = Pattern.compile("[0-9]{4}(-[0-9]{4})?");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor niggle: I believe it would read a bit better if all field usages in this constructor were prepended with this..

For example, hasYearToken = ... refers to a field, but the hasYearToken part lacks a this., which makes it a bit inconsistent with the rest of the code in the constructor.

}
}

/** Reads the license file from the given file. */
Expand All @@ -87,7 +101,14 @@ public String format(String raw) {
if (!matcher.find()) {
throw new IllegalArgumentException("Unable to find delimiter regex " + delimiterPattern);
} else {
if (matcher.start() == licenseHeader.length() && raw.startsWith(licenseHeader)) {
if (hasYearToken) {
if (matchesLicenseWithYearToken(raw, matcher)) {
//that means we have the license like `licenseHeaderBeforeYEARToken 1990-2015 licenseHeaderAfterYEARToken`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor niggle: There should ideally be a space in between // and that to be consistent with the other comments.

return raw;
} else {
return licenseHeaderWithYEARTokenReplaced + raw.substring(matcher.start());
}
} else if (matcher.start() == licenseHeader.length() && raw.startsWith(licenseHeader)) {
// if no change is required, return the raw string without
// creating any other new strings for maximum performance
return raw;
Expand All @@ -97,4 +118,10 @@ public String format(String raw) {
}
}
}

private boolean matchesLicenseWithYearToken(String raw, Matcher matcher) {
int startOfTheSecondPart = raw.indexOf(licenseHeaderAfterYEARToken);
return (raw.startsWith(licenseHeaderBeforeYEARToken) && startOfTheSecondPart + licenseHeaderAfterYEARToken.length() == matcher.start())
&& yearMatcherPattern.matcher(raw.substring(licenseHeaderBeforeYEARToken.length(), startOfTheSecondPart)).matches();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again to clarify, usages of this. are not needed here, as I believe the unwritten convention in Spotless is that this. is only required when referring to fields in constructors.

}
}
26 changes: 26 additions & 0 deletions plugin-gradle/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,32 @@ spotless {
}
```

<a name="license-header"></a>

## License header options

The license header can contains a `$YEAR` variable that will be replaced by the current year.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this sentence would read better as something like the following:

If the string contents of a licenseHeader step or the file contents of a licenseHeaderFile step contains a $YEAR token, then in the end-result generated license headers which use this license header as a template, $YEAR will be replaced with the current year.


For example:
```
/* Licensed under Apache-2.0 $YEAR. */
```
will produce
```
/* Licensed under Apache-2.0 2017. */
```
if build is launched in 2017
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this would read better as:

if Spotless is launched in 2017.



The step will change the license according to the following rules
* It replace the license using the current year when
* The license is missing
* The license is not formatted correctly
* It will *not* replace the license when
* The year variable is already present and is a single year, e.g. `/* Licensed under Apache-2.0 1990. */`
* The year variable is already present and is a year span, e.g. `/* Licensed under Apache-2.0 1990-2003. */`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if these rules would read better as:

The `licenseHeader` and `licenseHeaderFile` steps will generate license headers with automatic years from the base license header according to the following rules:
* A generated license header will be updated with the current year when
    * the generated license header is missing
    * the generated license header is not formatted correctly
* A generated license header will _not_ be updated when
    * a single year is already present, e.g.
      `/* Licensed under Apache-2.0 1990. */`
    * a hyphen-separated year range is already present, e.g.
      `/* Licensed under Apache-2.0 1990-2003. */`
    * the `$YEAR` token is otherwise missing

@baptistemesta Is my understanding correct that no year substitution will happen if $YEAR is not present in the base license header?



<a name="custom"></a>

## Custom rules
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,28 @@
package com.diffplug.spotless.generic;

import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.time.YearMonth;

import org.junit.Assert;
import org.junit.Test;

import com.diffplug.spotless.FormatterStep;
import com.diffplug.spotless.ResourceHarness;
import com.diffplug.spotless.SerializableEqualityTester;
import com.diffplug.spotless.StepHarness;

public class LicenseHeaderStepTest extends ResourceHarness {
private static final String KEY_LICENSE = "license/TestLicense";
private static final String KEY_FILE_NOTAPPLIED = "license/MissingLicense.test";
private static final String KEY_FILE_APPLIED = "license/HasLicense.test";

// files to test $YEAR token replacement
private static final String KEY_LICENSE_WITH_YEAR_TOKEN = "license/LicenseHeaderWithYearToken";
private static final String KEY_FILE_WITHOUT_LICENSE = "license/FileWithoutLicenseHeader.test";
private static final String KEY_FILE_WITH_LICENSE_AND_PLACEHOLDER = "license/FileWithLicenseHeaderAndPlaceholder.test";

// If this constant changes, don't forget to change the similarly-named one in
// plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java as well
private static final String LICENSE_HEADER_DELIMITER = "package ";
Expand All @@ -46,6 +54,26 @@ public void fromFile() throws Throwable {
assertOnResources(step, KEY_FILE_NOTAPPLIED, KEY_FILE_APPLIED);
}

@Test
public void should_apply_license_containing_YEAR_token() throws Throwable {
FormatterStep step = LicenseHeaderStep.createFromFile(createTestFile(KEY_LICENSE_WITH_YEAR_TOKEN), StandardCharsets.UTF_8, LICENSE_HEADER_DELIMITER);

StepHarness.forStep(step)
.test(getTestResource(KEY_FILE_WITHOUT_LICENSE), fileWithPlaceholderContaining(currentYear()))
.testUnaffected(fileWithPlaceholderContaining(currentYear()))
.testUnaffected(fileWithPlaceholderContaining("2003"))
.testUnaffected(fileWithPlaceholderContaining("1990-2015"))
.test(fileWithPlaceholderContaining("not a year"), fileWithPlaceholderContaining(currentYear()));
}

private String fileWithPlaceholderContaining(String placeHolderContent) throws IOException {
return getTestResource(KEY_FILE_WITH_LICENSE_AND_PLACEHOLDER).replace("__PLACEHOLDER__", placeHolderContent);
}

private String currentYear() {
return String.valueOf(YearMonth.now().getYear());
}

@Test
public void efficient() throws Throwable {
FormatterStep step = LicenseHeaderStep.createFromHeader("LicenseHeader\n", "contentstart");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (C) __PLACEHOLDER__. ACME corp.
* This library is free software; you can redistribute it and/or modify it under the terms
* of the GNU Lesser General Public License as published by the Free Software Foundation
* version 2.1 of the License.
* This library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
* without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
* See the GNU Lesser General Public License for more details.
* You should have received a copy of the GNU Lesser General Public License along with this
* program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth
* Floor, Boston, MA 02110-1301, USA.
**/
package com.acme;

import java.util.function.Function;


public class Java8Test {
public void doStuff() throws Exception {
Function<String, Integer> example = Integer::parseInt;
example.andThen(val -> {
return val + 2;
} );
SimpleEnum val = SimpleEnum.A;
switch (val) {
case A:
break;
case B:
break;
case C:
break;
default:
throw new Exception();
}
}

public enum SimpleEnum {
A, B, C;
}
}
28 changes: 28 additions & 0 deletions testlib/src/test/resources/license/FileWithoutLicenseHeader.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.acme;

import java.util.function.Function;


public class Java8Test {
public void doStuff() throws Exception {
Function<String, Integer> example = Integer::parseInt;
example.andThen(val -> {
return val + 2;
} );
SimpleEnum val = SimpleEnum.A;
switch (val) {
case A:
break;
case B:
break;
case C:
break;
default:
throw new Exception();
}
}

public enum SimpleEnum {
A, B, C;
}
}
12 changes: 12 additions & 0 deletions testlib/src/test/resources/license/LicenseHeaderWithYearToken
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright (C) $YEAR. ACME corp.
* This library is free software; you can redistribute it and/or modify it under the terms
* of the GNU Lesser General Public License as published by the Free Software Foundation
* version 2.1 of the License.
* This library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
* without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
* See the GNU Lesser General Public License for more details.
* You should have received a copy of the GNU Lesser General Public License along with this
* program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth
* Floor, Boston, MA 02110-1301, USA.
**/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the concept of having a GNU licence anywhere in the source code is kinda a red flag.

Since the GNU license itself is licensed under GNU and we are "linking" (can be interpreted broadly) against the licence by reading in the file perhaps it would be better to pick a different licence with a year token.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I totally respect GNU I don't want it anywhere near my software. 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha, I will change that!