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

Support multiple license headers #872

Closed
rnett opened this issue Jun 1, 2021 · 6 comments · Fixed by #990
Closed

Support multiple license headers #872

rnett opened this issue Jun 1, 2021 · 6 comments · Fixed by #990

Comments

@rnett
Copy link

rnett commented Jun 1, 2021

We have a situation where we need to support multiple license headers (not in the same file, but some files will have one, some the other), because some commiters have different requirements from their companies. Spotless doesn't currently have any support for this that I could find, but it would be nice if it was able to accept multiple licenses as OK, and probably designate one as the default, to add to new files without a license header.

@nedtwigg
Copy link
Member

nedtwigg commented Jun 1, 2021

Related: #650 (disable license rule for specific file)

It's already possible (under the hood) to compose a step with a filename-specific filter, e.g. this little snippet which keeps the licenseHeader step from clobbering package-info.java files:

return step.filterByFile(LicenseHeaderStep.unsupportedJvmFilesFilter());

It would be pretty easy to use this mechanism to expose something like this in the Gradle DSL:

licenseHeader('foo').skipIfPathEndsWith('src/main/thirdparty/**/*.java')
licenseHeader('bar').onlyIfPathEndsWith('src/main/thirdparty/**/*.java')

The problem is that LicenseHeaderStep has its name hardcoded, and you can't have multiple steps with the same name because we use that name to determine where errors come from.

int existingIdx = getExistingStepIdx(newStep.getName());
if (existingIdx != -1) {
throw new GradleException("Multiple steps with name '" + newStep.getName() + "' for spotless format '" + formatName() + "'");
}

Two different solutions come to mind. Happy to take a PR for either.

Named license header

One solution is something like this:

licenseHeader('foo').named('licenseFoo').skipIfPathEndsWith('src/main/thirdparty/**/*.java')
licenseHeader('bar').named('licenseBar').onlyIfPathEndsWith('src/main/thirdparty/**/*.java')

This would be pretty easy to implement. If you left off the "named()", then you'd get the "Multiple steps with name blah" error.

Subformats

A broader solution would be to create the concept of a subformat, which allows you to create a step which is actually a bunch of substeps, e.g.

googleJavaFormat()
licenseHeader('foo')
subformat 'thirdparty', {
  onlyIfPathEndsWith 'src/main/java/thirdparty/**/*.java'
  licenseHeader('lgpl')
}

or

googleJavaFormat()
licenseHeader('lgpl')
subformat 'thirdparty', {
  skipIfPathEndsWith 'src/main/java/thirdparty/**/*.java'
  licenseHeader('foo')
}

or

googleJavaFormat()
subformat 'thirdparty', {
  onlyIfPathEndsWith 'src/main/java/thirdparty/**/*.java'
  licenseHeader('lgpl')
}
subformat 'ourCode', {
  onlyIfPathEndsWith 'src/main/java/com/acme/**/*.java'
  licenseHeader('foo')
}

The nice thing about subformats is that they could apply to any step, not just licenses. Subformat should be backed by a new CompositeStep which maintains the path filters and the list of steps. Implementing the DSL part would look something like this:

public <T extends FormatExtension> void format(String name, Class<T> clazz, Action<T> configure) {

@rnett
Copy link
Author

rnett commented Jun 1, 2021

We don't have things split by folders though, I want spotless to detect which license it's working with, and update that one (or let either pass check). Named license headers looks nice, but it would be more like:

licenseHeader{
  default("/* Copyright ...")
  accept("/* Copyright 2 ...")
  accept("/* Copyright 3 ...")
}

Also we're using the maven plugin but that should be mostly transferable.

@nedtwigg
Copy link
Member

nedtwigg commented Jun 1, 2021

Interesting! I guess both ideas could be extended to filter based on content rather than path:

licenseHeader('foo').named('licenseFoo').onlyIfContentHas('/* Foo')
licenseHeader('bar').named('licenseBar').onlyIfContentHas('/* Bar')
subformat 'ourCode', {
  onlyIfContentHas '/* Copyright'
  licenseHeader('foo')
}

Probably should also have onlyIfContentMatches(String regex) (see here for how we've handled regexes elsewhere).

@nedtwigg
Copy link
Member

nedtwigg commented Jun 1, 2021

The key thing is single-responsibility principle. I think LicenseHeaderStep should only handle one kind of license, nothing else, and then it can be composed with other pieces to create other effects.

We've had previous requests for license-per-path, and your usecase of license-per-content is also valid. I'm open to your DSL idea, but it needs to handle the per-path and per-content cases, and they shouldn't require LicenseHeaderStep to change. It also shouldn't make the default case of "I want one license for all my files" any harder.

@simonbasle
Copy link

I like the subformat idea with path AND content-based filters! Such a feature could also be used to avoid overwriting a license header from a third-party file that is integrated into the codebase, untouched.

@nedtwigg
Copy link
Member

nedtwigg commented Dec 1, 2021

Published in plugin-gradle/6.0.1. Currently unsupported in maven, a PR for that is welcome.

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

Successfully merging a pull request may close this issue.

3 participants