Custom Spotless Task

[t-rad679]

Hello!

I'm trying to create a custom spotless task that only acts on files changed in git. I would also like the option to act on all files, so ideally I would have two separate tasks. For example, spotlessJavaCheck should check the pattern **/*.java, while spotlessChangedCheck should check only the files changed in git.

I've already written a function in the build script which identifies the changed files, and I would like to set the spotless target to these files.

Any advice?

[nedtwigg]

You can use -Pfiles=... to specify files manually at the command line, see #178 for details. We have plans to implement something like this directly in #511.

[t-rad679]

Thanks for the reply!

Is there a way to do this directly from a custom gradle task? The goal is for our engineers to be able to just run a task without having to do any additional configuration. Or is this what you're working on?

[t-rad679]

Also, I'm unable to get that to work...either with files or spotlessFiles mentioned on the readme.

[t-rad679]

More specifically, I run ./gradlew spotlessJavaCheck -Pfiles=Test.Java (and the same thing with -PspotlessFiles and it checks all files. I have triple checked that Test.java is located in the current working dir (which is the project root dir) and that it contains errors that should trigger a spotless error. I have also tried setting this property in gradle.properties and programmatically in the build script, to no avail.

[nedtwigg]

Ah, sorry, -PspotlessFiles is the right one, not -Pfiles. I'm not sure why it's not working for you (I assume the .Java vs .java in your paragraph above is a typo). The property gets ingested here:

spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java

Lines 282 to 289 in e870e78

	String filePatterns;
	if (project.hasProperty(FILES_PROPERTY) && project.property(FILES_PROPERTY) instanceof String) {
	filePatterns = (String) project.property(FILES_PROPERTY);
	} else {
	// needs to be non-null since it is an @Input property of the task
	filePatterns = "";
	}
	spotlessTask.setFilePatterns(filePatterns);

And then it takes effect here:

spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessTask.java

Lines 190 to 211 in e870e78

	Predicate<File> shouldInclude;
	if (this.filePatterns.isEmpty()) {
	shouldInclude = file -> true;
	} else {
	// a list of files has been passed in via project property
	final String[] includePatterns = this.filePatterns.split(",");
	final List<Pattern> compiledIncludePatterns = Arrays.stream(includePatterns)
	.map(Pattern::compile)
	.collect(Collectors.toList());
	shouldInclude = file -> compiledIncludePatterns
	.stream()
	.anyMatch(filePattern -> filePattern.matcher(file.getAbsolutePath())
	.matches());
	}
	// find the outOfDate files
	List<File> outOfDate = new ArrayList<>();
	inputs.outOfDate(inputDetails -> {
	File file = inputDetails.getFile();
	if (shouldInclude.test(file) && file.isFile() && !file.equals(getCacheFile())) {
	outOfDate.add(file);
	}
	});

And here is the gradle plugin test which makes sure that it still works:
https://github.com/diffplug/spotless/blob/e870e78999887595d8480c699c4ef71c9e974b32/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/SpecificFilesTest.java

So that's the first thing to figure out - why -PspotlessFiles isn't working for you. Once we've got that figured out, I'm happy to help figure out any remaining pieces.

[t-rad679]

Excellent. I'll dig around, thank you!

And yes, the capital J was a typo. I double checked after you mentioned it.

[t-rad679]

Ok, so here's the progress I've made:

I played around with printing both the filePatterns variable and the spotlessFiles property itselfI. I have been using a separate gradle file to handle the spotless logic (i.e., in build.gradle.kts I have apply(from = "style.gradle.kts") and style.gradle.kts is where the spotless configuration lives). When I moved all that logic to the base config file, I was at least able to get spotlessFiles to populate as a property, but filePatterns is not populated even then.

Also note, it is pretty important to me to be able to configure the Spotless logic outside of the base build script. I want to share this logic between projects. I still can't make it work even by using the base script, but if at all possible I want to make it work with the dependency script.

[nedtwigg]

That's pretty strange. Can you provide a verbatim command-line that you're executing? For example, one of the commands used from the test linked above is:

./gradlew spotlessApply -PspotlessFiles=.*/src/main/java/test(1|3).java

I wonder if perhaps there is a missing ". For example

./gradlew spotlessApply "-PspotlessFiles=.*/src/main/java/test(1|3).java"

it is pretty important to me to be able to configure the Spotless logic outside of the base

Almost everywhere that I use Spotless, I configure it exclusively through apply from: blah, so that should be fine. One trick you can use in Gradle 6+ is the pluginManagement / plugins apply false trick, although that shouldn't be related to problems in the -PspotlessFiles=blah stuff.

[t-rad679]

Okay, I am now able to get spotlessFiles to populate from my dependency build script. Still no filePatterns though.

[nedtwigg]

One important thing is that filePatterns is set in an afterEvaluate block. Looking at it now, I don't see a good reason for it, but it shouldn't be a problem either (unless you're also getting warnings like using method Project#afterEvaluate(Action) when the project is already evaluated has been deprecated, in which case there's the problem!)

Depending on when you are doing println() on filePatterns, that might explain why you're not seeing it. The only thing that matters is the filePatterns value during task execution - where in the execution path are you looking at the filePatterns value? Make sure it's a doFirst {} block on a task.

[t-rad679]

I mess around with it here and there to see what happens, but the main command I'm using is:

./gradlew spotlessCheck -PspotlessFiles=Test.java

I'm running this from the root directory of the project, and have confirmed that Test.java is located there.

And you were correct about why filePatterns was not showing up in my print statements...I added them to a doFirst block and now it shows up as expected.

[nedtwigg]

Okay, so we know that -PspotlessFiles=Test.java is setting filePatterns, as expected. In the tests, filePatterns is also behaving as expected. But you are still observing that it checks all files, rather than only the files you want it to check? The logic linked above is pretty simple, are you sure the patterns you're supplying and the output you're getting are not the expected result? If we have that figured out, you can use this to create a second task.

[t-rad679]

What it's doing now is not checking any of the files. I'm playing around with it to see if it's that particular file, but I don't think so. I tried Test.java both with the indent I expect and with another commonly expected indent, then I tried putting in a file that the full, normal run of Spotless returned errors from. I've also tried using both absolute and relative paths.

[nedtwigg]

I would try this:

• pick a file, let's say dir/SomeClass.java
• mutilate its formatting
• ./gradlew spotlessApply
• confirm it is fixed
• mutilate its formatting
• ./gradlew -PspotlessFiles=
• confirm it is not fixed
• ./gradlew -PspotlessFiles=SomeClass.java
• confirm it is fixed

[t-rad679]

Sorry for the delay again. I will take a look at your most recent suggestions by Monday.

In particular, your last comment gave me the idea that maybe there's something going on with spotlessCheck that is not an issue in spotlessApply. Do you know of any reason there would be a difference between the two? I have been testing it with spotlessCheck because I don't want to deal with reverting the hundreds of changed files every time I try it and it doesn't work correctly, but if -PspotlessFiles only works for spotlessApply, that is fine for my use case.

[nedtwigg]

Do you know of any reason there would be a difference between the two?

I don't, I think that my instructions above would work as just as well with spotlessCheck.

I don't want to deal with reverting the hundreds of changed files every time I try

Start with a toy example! Delete every file except "Test.java", git makes that easy and safe to do. "A complex system that works is invariably found to have evolved from a simple system that worked."

[t-rad679]

Ok. I deleted all source files, just leaving a short Test.java in the project root directory. Unfortunately, I still can't get spotlessFiles to work with either spotlessCheck or spotlessApply. Here's some CLI output where spotlessCheck is run first without spotlessFiles and then with:

Tome-of-Ancient-Secrets:heimdall trevorradcliffe$ ./gradlew spotlessCheck
> Task :spotlessJava FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':spotlessJava'.
> The following files had format violations:
      Test.java
          @@ -1,9 +1,9 @@
           public·class·Test·{
          -··int·wrongIndent·=·0;
          -
          +····int·wrongIndent·=·0;

           ····//·Too·many·newlines
          -····String·tooLong·=·"thislineiswaytoolongandshouldbebrokenbyalinteroratleastflaggedbysaidlinterifitdoesntwanttobeopinionated";
          -}
          +····String·tooLong·=
          +············"thislineiswaytoolongandshouldbebrokenbyalinteroratleastflaggedbysaidlinterifitdoesntwanttobeopinionated";
          +}
  Run 'gradlew spotlessApply' to fix these violations.

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 763ms
1 actionable task: 1 executed
Tome-of-Ancient-Secrets:heimdall trevorradcliffe$ ./gradlew spotlessCheck -PspotlessFiles=Test.java

BUILD SUCCESSFUL in 713ms
1 actionable task: 1 executed

For reference, here are the contents of Test.java:

public class Test {
  int wrongIndent = 0;


    // Too many newlines
    String tooLong = "thislineiswaytoolongandshouldbebrokenbyalinteroratleastflaggedbysaidlinterifitdoesntwanttobeopinionated";
}

When I run spotlessApply, all issues in the file are fixed. When I run spotlessApply -PspotlessFiles=Test.java, nothing changes.

Configs

Since it seems pretty likely this is playing a role here, I'm gonna go ahead and include my configuration files.

style.gradle.kts (Created in a separate style repo, but temporarily copied to the dependent repo while I figure out cross-repo dependencies.):

//TODO: Remove this file and depend on styleguide repo after cross-repo sharing is ready
import com.diffplug.gradle.spotless.SpotlessExtension
import com.diffplug.gradle.spotless.SpotlessPlugin
import com.diffplug.gradle.spotless.SpotlessTask

// Must use legacy plugin syntax for importing external files to build.gradle
buildscript {
    repositories {
        maven(url = "https://plugins.gradle.org/m2/")
    }
    dependencies {
        classpath("com.diffplug.spotless:spotless-plugin-gradle:3.27.1")
    }
}

apply(plugin = "checkstyle")
apply(plugin =  "java")
apply<SpotlessPlugin>()

configure<CheckstyleExtension> {
    toolVersion = "8.28"
    configFile = rootProject.file("$projectDir/gradle/nextraq_checkstyle.xml")
}

configure<SpotlessExtension> {
    java {
        eclipse().configFile("$projectDir/gradle/eclipse-java-nextraq-style.xml")
        endWithNewline()
    }
}

// Checkstyle and Spotless felt the need to add themselves to the java plugin's build process without asking. How rude.
if (gradle.startParameter.taskNames
        .intersect(
                listOf("check",
                        *tasks.filter { it.dependsOn.contains("check") }.map { it.name }.toTypedArray()))
                .isNotEmpty()) {
    tasks.withType(Checkstyle::class) {
        enabled = false
    }
    tasks.withType(SpotlessTask::class) {
        enabled = false
    }
}

lint.gradle.kts (In the dependent repo, where code is being linted):

import com.diffplug.gradle.spotless.SpotlessExtension

buildscript {
    repositories {
        maven(url = "https://plugins.gradle.org/m2/")
    }
    dependencies {
        classpath("com.diffplug.spotless:spotless-plugin-gradle:3.27.1")
    }
}

apply(from = "$projectDir/gradle/style.gradle.kts")

configure<SpotlessExtension> {
    java {
        target(
            fileTree(
                    mapOf(
                            "dir" to ".",
                            "include" to listOf("**/*.java")
                    )
            )
        )
    }
}

And, in build.gradle.kts I do:

apply(from = "$projectDir/gradle/lint.gradle.kts")

I can't thank you enough for all your help here.

[nedtwigg]

There's a lot that can be commented out of this to make the test case simpler, especially the if (gradle.startParameter.taskNames .... Another possibility is that the regex compile is getting screwed-up by ., maybe try Test\.java.

[t-rad679]

No dice on Test\.java.

What alternative to that if statement that disables spotless and checkstyle tasks automatically included in build processes where we don't want them? Spotless especially should only ever be run manually.

[nedtwigg]

What alternative to that if statement that disables spotless

https://github.com/diffplug/spotless/tree/master/plugin-gradle#disabling-warnings-and-error-messages

Regardless of that, my goal is just to figure out why a simple unit test is passing, but failing for your build. There are two possibilities - A) the unit test is actually broken and we just don't realize it (possible, but I don't see how). B) Your build is different in some surprising way (I see lots of ways it is different).

Removing complexity helps narrow things down. You can always add them back later.

[t-rad679]

Ah cool! Thanks for the tip on disabling Spotless in the Java plugin build process!

I tried commenting out that code and it still does not work as expected. I know there's a lot of weird stuff in my configs, but those were the only solutions to issues I was having. In particular, using Kotlin and imported build scripts seemed to cause issues that needed me to do weird stuff to get spotless included in the build. Any other ideas? I'll also keep playing around with it and let you know what I find.

[nedtwigg]

Nope, my only ideas are that the unit test is broken in a surprising way, or that your build is complicated in a surprising way. The path to figuring out which are small toy projects which eliminate confounding variables. Best of luck.

[t-rad679]

Ok. I've tried quite a variety of different configurations. I've tried removing a number of things from my configs, leaving only things that removing breaks Spotless. I've also tried messing around with the way that I import Spotless and the way that I import style.gradle.kts and lint.gradle.kts. The way that I am doing it is the only way that actually works. I also looked around GitHub for some ideas, but the vast majority either import Spotless diectly into build.gradle.kts or use an exact copy of a configuration from a sample application that is confusing and I can't seem to get working.

What specifically would you say is unusual about my configuration, and can you point me to the particular unit test you're referring to?

[t-rad679]

I've also been playing around with doing this directly in my styleguide project with a dummy build.gradle.kts and copied Test.java over there. It has the bare minimum configuration necessary.

[t-rad679]

I'm guessing this is the test? It doesn't look like it tests Kotlin configurations. Yeah, that "shouldn't" matter, but these things often do. Kotlin Gradle has been different from Groovy Gradle in many surprising ways, in my experience.

I'll try rewriting my style.gradle.kts in Groovy and see what happens, but, at least for this project, moving back to Groovy for the build.gradle.kts is not an option.

It also doesn't test an imported Spotless configuration like I'm using, which has some extra nuances due to deficiencies in Gradle (primarily the thing where legacy plugin syntax is required). There is one more issue where SpotlessPlugin and SpotlessExtension have to be imported and specified directly, rather than using the plugin ID syntax, though I can't remember the cause of that particular issue.

While most of these cases ideally wouldn't need to be tested, they do end up being relevant in practice. Even if it doesn't make sense to test them in the long run, they still could be causing the issue I'm seeing.

I'll let you know what happens with trying the Groovy configuration.

[nedtwigg]

I'm guessing this is the test?

Yup, that's the one.

What specifically would you say is unusual about my configuration

I don't have any specific recommendations, except that the unit test works, and the build doesn't. The only tool I can recommend is to make the build more like the unit test until it works, or (as you note) to make the unit test more the like build until it breaks.

I don't have a hunch that your build has a bug, nor that Spotless has a bug. I have no hunches whatsoever, beyond a working unit test and a general-purpose debugging methodology to find the gap between a working test-case and a broken use-case.

FWIW, the vast majority of Spotless users in the wild are using check to call spotlessCheck on all files, so that's the well-worn path. -PspotlessFiles was added because someone wanted it, they implemented it cleanly, and they added a unit test and docs (#321). If you want to fix any bugs or add any features to that functionality, we'd be happy to merge them, but it's not a feature that our core group of maintainers uses.

[t-rad679]

Ok that makes sense. Thank you for all the input you've given up to this point! I sincerely appreciate it; it has been quite helpful. I will keep looking into this and report back to let you and anyone else who is curious what I find.

For now I'll close the ticket. Feel free to reopen if you like.

[t-rad679]

See #528

[nedtwigg]

FYI, -PspotlessFiles has been deprecated and will be removed. Migration path available here: #602