JLLeitschuh/ktlint-gradle

`filter` config seems to be ignored

igorwojda opened this issue ยท 23 comments

A bit of context
Due to some problems with Android SafeArgs Gradle plugin I had to add additional source sets into feature modules (Gradle sub-projects)

    sourceSets {
getByName("test").java.srcDir("${project.rootDir}/app/build/generated/source/navigation-args/debug")
    }

Ktlint Issue
Now I am experiencing an issue where filter configuration seems to be ignored and checks ...Args classes that are located inside the generated folder (I am not sure if this is related to the above change, but 100% reproducible).

I have added to root build.gradle.kts file:

filter {
   exclude("**/generated/**")
}

However ktlint gradle still seens to check classes in generated folder

/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetails/AlbumDetailFragmentArgs.kt:26:30

Since this class is generated by SafeArgs Gradle plugin I have no control on code format and naturally, I want it not to be checked by ktlint.

Also, note that after adding the same source set to different modules this file is checked 3 times.

Reproduce
Here is the branch to test it
https://github.com/igorwojda/android-showcase/tree/ktlint-filter-not-working

  1. Open this project in Android Studio
  2. Build project, so AlbumDetailFragmentArgs is generated (Build -> Make project)
  3. Run ktlint ./gradlew ktlintcheck

Result

> Task :feature_favourite:ktlintTestSourceSetCheck FAILED
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetails/AlbumDetailFragmentArgs.kt:26:30: Unexpected spacing before ":" (colon-spacing)eSetCheck
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetails/AlbumDetailFragmentArgs.kt:35:29: Unexpected spacing before ":" (colon-spacing)
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetails/AlbumDetailFragmentArgs.kt:44:24: Unexpected spacing before ":" (colon-spacing)
"checkstyle" report written to /Users/igorwojda/StudioProjects/android-showcase/feature_favourite/build/reports/ktlint/ktlintTestSourceSetCheck.xml

> Task :feature_profile:ktlintTestSourceSetCheck FAILED
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetails/AlbumDetailFragmentArgs.kt:26:30: Unexpected spacing before ":" (colon-spacing)
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetails/AlbumDetailFragmentArgs.kt:35:29: Unexpected spacing before ":" (colon-spacing)
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetails/AlbumDetailFragmentArgs.kt:44:24: Unexpected spacing before ":" (colon-spacing)
"checkstyle" report written to /Users/igorwojda/StudioProjects/android-showcase/feature_profile/build/reports/ktlint/ktlintTestSourceSetCheck.xml

> Task :feature_album:ktlintTestSourceSetCheck FAILED
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetails/AlbumDetailFragmentArgs.kt:26:30: Unexpected spacing before ":" (colon-spacing)eSetCheck
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetails/AlbumDetailFragmentArgs.kt:35:29: Unexpected spacing before ":" (colon-spacing)
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetails/AlbumDetailFragmentArgs.kt:44:24: Unexpected spacing before ":" (colon-spacing)
"checkstyle" report written to /Users/igorwojda/StudioProjects/android-showcase/feature_album/build/reports/ktlint/ktlintTestSourceSetCheck.xml

Expected result
All checks related to AlbumDetailFragmentArgs.kt would be ignored (due to filter rule). Also is this class would not be ignored I would expect no duplicated checks.

Interesting, I don't experience any issues with ktlint-gradle in the project that uses SafeArgs plugin.

But I will check your repo and will try to figure out what is the problem ๐Ÿ‘

This is line that causing this issue: https://github.com/igorwojda/android-showcase/blob/3468173b90c206574953da1ff505938b2d1df169/feature_profile/build.gradle.kts#L52

Commenting it out makes ktlint green again.

Though plugin should support such case as well, I will try to fix it.

Thank you for such detailed report ๐Ÿ‘

As a temporary workaround you may wrap adding this source into:

if (!gradle.startParameter.taskNames.contains("ktlintCheck)) {
  // add additional source set
}

Thanks for the confirmation.

So above proposed fix don't really help me, because I had to add this source set to make SafeArgs plugin work with tests. As I understand correctly above fix will disable ktlint in all my modules which is basically the same as using filter("*") or disabling the plugin.

Bug (probably SageArgs?)
I am still want to investigate it more before reporting it to Google, but there is a problem with my module dependency setup (without this extra dependency getByName("test")... Unit tests run in IDE but fail to run while running ./gradlew testDebuUnitTest task (missing AlbumDetailFragmentArgs class).
https://issuetracker.google.com/issues/139242292

So above proposed fix don't really help me, because I had to add this source set to make SafeArgs plugin work with tests. As I understand correctly above fix will disable ktlint in all my modules which is basically the same as using filter("*") or disabling the plugin.

This fix disables adding safe args generated classes only when gradle command line has ktlintCheck task in it, so running tests, theoretically, should work.

Oh...I didn't get the initial intention. This works perfectly and I must say this is a really clever solution. thx @Tapchicoma !

I am running into a very similar issue just with the wire plugin. I had to work around the fact that it is not yet distributed as an official gradle plugin as described here: square/wire#1029 (comment)

For some reason the workaround

sourceSets {
    if (!gradle.startParameter.taskNames.contains("ktlint")) {
        getByName("main").java.srcDir("${buildDir.absolutePath}/generated/src/main/java")
    }
}

is not working for me.

But adding

ktlint {
    enableExperimentalRules.set(true)
    filter {
        // exclude("**/generated/**")
        exclude { projectDir.toURI().relativize(it.file.toURI()).path.contains("/generated/") }
    }
}

as mentioned here: #222 (comment) does work for me. Not quite sure why the one works and the other one not to be honest.

@Globegitter I think that in 1st example your task should be ktlintCheck, not just ktlint. I am also referencing path generated folder starting from project root. Check my config:
https://github.com/igorwojda/android-showcase/blob/96838be527d8a4e8b3fe217ad04566f41b953853/feature_album/build.gradle.kts#L56

@Tapchicoma I think I have encountered a variation of this problem. I just want to post is here, so you can take a look while fixing this issue:

When running ./gradlew check also fails for me with the same problem (ignored filters), but this time excluding source above fix does not work (even if wide the scope to all ktlint tasks if (!gradle.startParameter.taskNames.contains("ktlint"))).

> Task :feature_favourite:ktlintTestSourceSetCheck FAILED
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetail/AlbumDetailFragmentArgs.kt:26:30: Unexpected spacing before ":" (colon-spacing)
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetail/AlbumDetailFragmentArgs.kt:35:29: Unexpected spacing before ":" (colon-spacing)
/Users/igorwojda/StudioProjects/android-showcase/app/build/generated/source/navigation-args/debug/com/igorwojda/showcase/feature/album/presentation/albumdetail/AlbumDetailFragmentArgs.kt:44:24: Unexpected spacing before ":" (colon-spacing)
"checkstyle" report written to /Users/igorwojda/StudioProjects/android-showcase/feature_favourite/build/reports/ktlint/ktlintTestSourceSetCheck.xml
...

Above workaround works only when ktlintCheck task explicetly defined in command line invocation.

I also encountered this issue while using SQLDelight. The relative path check mentioned above resolved the issue for now.

exclude { projectDir.toURI().relativize(it.file.toURI()).path.contains("/sqldelight/") }

@igorwojda your issue is that added files are located outside module project root dir:

This is because regex-based matching is done on the relative path, i.e. the path following the root directory of each tree.

Proper solution would be replace regex with:

exclude { element -> element.file.path.contains("generated/") }

More details you could find in this issue: gradle/gradle#3417

@Globegitter @plannigan I don't know details of you issues, but could you check if solution above is working for your cases as well.

I would add this solution to project README.

saket commented

@Tapchicoma I can confirm that that works.

Is this now resolved, can it be closed?

saket commented

I think so, yes

Thanks everyone! ๐Ÿ‘‹

configure<KtlintExtension> {
    filter {
        exclude { it.file.path.contains("$buildDir/generated/") }
    }
}
ktlint {
    filter {
        exclude { it.file.path.contains("$buildDir/generated/") }
    }
}

If someone else ends up here and is wondering how to do this without the deprecated buildDir property, this is what worked for me.

ktlint {
    filter {
        exclude { it.file.path.contains("${layout.buildDirectory.get()}/generated/") }
        include("**/kotlin/**")
    }
}
jogerj commented

@plusmobileapps I had to adapt the exclusion rule so it works regardless of OS (something to do with different path separators, doesn't work on Windows)

ktlint {
    filter {
        exclude { it.file.path.contains(layout.buildDirectory.dir("generated").get().toString()) }
    }
}

Still doesn't work in my case :/
Only that #522 (comment) solution seems to work.

For anyone else who finds this thread looking for a solution, the Ktlint FAQ has a recommendation: https://pinterest.github.io/ktlint/dev-snapshot/faq/#how-do-i-disable-ktlint-for-generated-code

In short, add the following to your .editorconfig, where the path is a relative path:

[some/path/to/generated/code/**/*]
ktlint = disabled

This may not work for everyone's situation, and I think it is unfortunate that I can't just use the Gradle filter, but it was the one thing that worked reliably for me, so I wanted to share in hopes it is useful to others.