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 built in support for ktlint linting of gradle script kotlin files #115

Closed
JLLeitschuh opened this issue May 24, 2017 · 13 comments
Closed

Comments

@JLLeitschuh
Copy link
Member

Currently, the only ktlinting support is for production files.
It would be awesome if you could also configure it to lint any .gradle.kts files for Gradle Script Kotlin

@nedtwigg
Copy link
Member

This is the code that sets up the default kotlin sources.

It needs tweaking for Gradle Script Kotlin, Android, and probably for kotlin-js as well.

Setting the target manually is pretty easy, but we'd love a PR which improves the defaults.

@JLLeitschuh
Copy link
Member Author

Would you like a new task added or the current one expanded?

Options:

  1. Add more configuration to the kotlin block.
  2. Create a new extension KotlinBuildExtension that is specific to formatting build.gradle.kts files.

Personally, I think option 2 makes more sense as a user may have different configuration for the kotlin block on a per-subproject basis while your formatting config for build.gradle.kts should be common across all of your files.

@nedtwigg
Copy link
Member

I think 2 makes the most sense. Code usually has a copyright notice at the top, and build files usually don't.

@JLLeitschuh
Copy link
Member Author

Is there a pre-existing extension that already targets formatting groovy build.gradle files?

@nedtwigg
Copy link
Member

Kinda. There is a groovy extension written by @fvgh, but it's not specific to build.gradle files. Might make sense to use the same approach for groovy / gradleGroovy and kotlin / gradleKotlin. Any thoughts @fvgh?

@fvgh
Copy link
Member

fvgh commented Jul 31, 2017

Sorry for the late response. Till now I was always able to work around the issue since the projects were using sub-projects and I have defined the formatting on all gradle files in the root project, using the target definition. I always wanted to make a proposal to extend the registration of all FormatExtension in SpotlessExtension by a custom name, so that you can specify multiple Groovy extensions with different targets.
But gradleGroovy as a fixed extension name with a default file pattern is also an option.

@nedtwigg
Copy link
Member

For now, it looks like we'll have kotlinGradle which automatically applies to *.gradle.kts, and is otherwise identical in behavior to kotlin. How do you feel about the same for groovy, groovyGradle which applies to *.gradle, and is otherwise identical to groovy?

@fvgh
Copy link
Member

fvgh commented Aug 1, 2017

It is an option.
Drawback is the introduction of another interface on spotless level.
The benefit is that nobody has to specify file extensions, like *.gradle.kts and *.gradle, which can be assumed as not modifiable. So the configuration is cleaner.

It worries me that the current solution forces the user to take the restrictions imposed by parseTarget in combination with the fixed extension. By allowing multiple configurations of language-specific formats, the user can do what ever he wants.

Just to get a common understanding, I was more thinking about something like:

groovy {
  /* The nominal config */
}

groovy, 'format-gradle' {
 target '*.gradle'
 /* What ever is necessary */
}

I think the *.gradle / parseTarget approach would currently omit any gradle files in custom directories which I include with apply from, so here the user has to specify a custom target, right?

Another thing I was once wondering about is how to deal with certain Java files which have a different licence but are in the same project. But I know that this is a rare scenario. Just needed it when I was patching code.

@nedtwigg
Copy link
Member

nedtwigg commented Aug 1, 2017

Here is one approach to this problem.

What limitations in target are you worried about? The user can pass any FileCollection, so I can't think of any limitations, really.

Re: Java files within a project with a different license,

java, 'externalFiles' {
    target 'src/main/java/org/someexternernalpackage/**/*.java'
    ...
}

seems like a good idea to me. But I think *.gradle and *.gradle.kts are common-enough usecases to earn their own DSL. There are also subtle differences, e.g. license headers rely on a package statement, which gradle scripts don't have, so it doesn't make sense to support license headers for build scripts.

@nedtwigg
Copy link
Member

nedtwigg commented Aug 1, 2017

But that's a matter for a new issue :)

@fvgh
Copy link
Member

fvgh commented Aug 1, 2017

That would be a new issue, but quickly done.
Just thought whether in that case the groovyGradle and kotlinGradle becomes superfluous and many ways to achieve the same goal are just confusing.
But I see that you prefer convention over configuration. I'll provide you with an update 😄

I don't think that we should explicitly exclude the license header. It is valid (and I used it for minor things) to define a plugin and/or task within a gradle file. This is when one might also have a licence. The license header is already smart enough to deal with situations where it does not find the delimiter by throwing an exception. So it avoids unintended user configuration.
Agreed?

@nedtwigg
Copy link
Member

nedtwigg commented Aug 1, 2017

Every FormatExtension has licenseHeader(String header, String delimiter).

JavaExtension and KotlinExtension also have licenseHeader(String header) where the delimiter has the default value package , which works for code, but doesn't work for build.gradle.kts. So that's all I meant by remove licenseHeader, just the package specialization. 😃

@fvgh
Copy link
Member

fvgh commented Aug 1, 2017

Ok, I see, my mistake. Agreed.

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

No branches or pull requests

3 participants