Alternate sourceSets are not detected

[rubengees]

Plugin version: 6.2.0
Ktlint version: Bundled
Project type: Android
Android Gradle plugin version: 3.4.0-alpha01 (happened in earlier 3.3.x ones too)

Config:

apply plugin: "org.jlleitschuh.gradle.ktlint"

ktlint {
    android = true
}

I have configured an alternative sourceSet for my kotlin sources:

sourceSets {
    main {
        java.srcDirs += "src/main/kotlin"
    }
}

Once I do this, the plugin does not seem to be able to detect kotlin files in that directory. Kotlin files in src/main/java are detected.

Is this some misconfiguration on my end? I can create and share a sample project if needed.
Thanks for looking into this!

[Tapchicoma]

Interesting, I suppose you've applied kotlin-android plugin?

[rubengees]

Sure, my sample project and also the real project in which I first found this work properly apart from this and the plugin also worked before the Android Gradle Plugin had this change with the lazy configuration (e.g. plugin versions < 6 work).

[Tapchicoma]

Indeed, I was able to reproduce on android sample project. Will try to fix it.

[Tapchicoma]

Also I will assume that plugin version 5.1.0 should work in this case.

[Tapchicoma]

The problem here that user defined source sets are only available in afterEvaluate {} block. Usages of this has been removed in #122 (6.x.x versions). So it will be not easy to fix.

Will try to figure out how Gradle Java plugin approaches this issue.

[Tapchicoma]

@rubengees fix will be available in 6.2.1 version.

[rubengees]

Cool, thanks!

[tasomaniac]

I'm still having this problem in my integration tests. Not really in a real project. ktlintDebugCheck task gives me NO-SOURCE

Generated project is something like below. Notice that I don't really have anything inside main folder. Instead, I'm adding a srcDir manually where I have the sample files with errors.

buildscript {
    repositories {
        google()
        jcenter()
    }
    dependencies {
        classpath 'com.android.tools.build:gradle:3.1.4'
        classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.2.20'
    }
}
plugins {
    id 'org.jlleitschuh.gradle.ktlint' version '6.1.0'   
    id 'com.novoda.static-analysis'
}
repositories {
    google()
    jcenter()
}
apply plugin: 'com.android.library'
apply plugin: 'kotlin-android'

android {
    compileSdkVersion 27

    defaultConfig {
        minSdkVersion 16
        targetSdkVersion 27
        versionCode 1
        versionName '1.0'
    }
    lintOptions {
        disable 'OldTargetApi'
    }
    sourceSets {
        main {
            manifest.srcFile '/Users/sdane/dev/taso/gradle-static-analysis-plugin/plugin/src/test/fixtures/AndroidManifest.xml'
            java {
                srcDirs += '/Users/sdane/dev/taso/gradle-static-analysis-plugin/plugin/src/test/fixtures/sources/ktlint/with-error'
            }
        }
    }
    
}

staticAnalysis {
    penalty {
        maxWarnings = 0
        maxErrors = 0
    }
    
            ktlint {
            reporters = [
                org.jlleitschuh.gradle.ktlint.reporter.ReporterType.PLAIN, 
                org.jlleitschuh.gradle.ktlint.reporter.ReporterType.CHECKSTYLE
            ]
            
            includeVariants { it.name == "debug" }
            }
            
}

[tasomaniac]

As far as I see in the line from referenced PR https://github.com/JLLeitschuh/ktlint-gradle/pull/154/files#diff-513ff7df76b3a229c796d8d72281e589R123

You use ext.sourceSets {, what about using ext.sourceSets.all { instead.

[JLLeitschuh]

@tasomaniac Those two suggestions are, functionally, equivalent.

[tasomaniac]

sourceSets function with an Action argument says configure in the docs. So it is used to configure sourceSets. Not sure if it returns a live collection.

all returns a live collection where you will also get future instances in the collection.

[tasomaniac]

Anyways, it seems like not only the additional source sets but the default ones are also ignored when it is used with Novoda plugin. I will investigate this.

novoda/gradle-static-analysis-plugin#146

[JLLeitschuh]

@tasomaniac This is the code as it exists in the current release:

ktlint-gradle/plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/KtlintPlugin.kt

Lines 158 to 172 in f32441b

	target.extensions.configure(BaseExtension::class.java) { ext ->
	ext.sourceSets { sourceSet ->
	sourceSet.all { androidSourceSet ->
	// Passing Callable, so returned FileCollection, will lazy evaluate it
	// only when task will need it.
	// Solves the problem of having additional source dirs in
	// current AndroidSourceSet, that are not available on eager
	// evaluation.
	createTasks(
	androidSourceSet.name,
	target.files(Callable { androidSourceSet.java.srcDirs })
	)
	}
	}
	}

Is this issue not fixed for you? Do you need it re-opened?

[tasomaniac]

It works if I configure it directly in my build.gradle file. It does not if I let Novoda static analysis plugin to configure KtLint.

That's why I think it is ok to keep this issue closed. I will do some debugging and will get back to you on this.

[tasomaniac]

After long debug session, I finally found the problem. 🎉

It is not just about Novoda plugin, so you can actually re-open this one.

It is a common practice to create tasks for each variant in Android. If you have flavor1Debug and flavor2Debug they would share the debug source set for example. I assumed that ktlint would configure the tasks also based on the Android variant. And you would have main always added as a source set.

The above link you posted created the task by only looking at the sourceSet kinds on the extension, rather than build variants. That is why ktlintDebugCheck only checks for debug folder and skips the main folder. And this was my problem.

Long story short, running ktlintDebugCheck does not include the main sourceSet.

[tasomaniac]

Here, as far as I see, you were using Android variants already. This is the last working version for me.

https://github.com/JLLeitschuh/ktlint-gradle/blob/v5.1.0/plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/KtlintPlugin.kt#L101

[JLLeitschuh]

@tasomaniac Can you provide a sample script that demonstrates your problem using the latest version of ktlint-gradle?
Ideally something minimal that doesn't involve the novoda plugin.

[tasomaniac]

Working on fix now. PR is coming soon.

Here how I reproduce it

• create a new Android project
• have some kotlin file with errors
• run ktlintDebugCheck task

[tasomaniac]

One way to reproduce inside the repo:

• Make an error in the file AlternativeSample.kt inside samples/android-app
• Run ktlintKellerbierDebugCheck
  • On master -> the error is not detected
  • checkout branch in Use variants dynamically to put sourceSets #167 -> error should be detected

[Tapchicoma]

Hmm, I intentionally removed usages of Android variant manager:

• it is not available for instant app plugin (extension has package-private visibility)
• To have same approach with plain kotlin projects: check all SourceSet that kotlin plugin has.

Usually user doesn't need to call specific task, but rather call ktlintCheck(or ktlintFormat) task, as it will check only updated sources and use for non-changed sources "UP-TO-DATE" or "FROM CACHE" approaches.

It is a common practice to create tasks for each variant in Android. If you have flavor1Debug and flavor2Debug they would share the debug source set for example. I assumed that ktlint would configure the tasks also based on the Android variant. And you would have main always added as a source set.

Possible solution from ktlint-gradle plugin side is to create special meta tasks for android variants (excluding instant app plugin) that will trigger variant source sets tasks.

[tasomaniac]

This is a huge behavior change though :( I would suggest to put it into changelog.

Usually user doesn't need to call specific task

I don't think it is safe/good to assume this ☝️. On Android people usually call the check tasks only for a certain variant to speed up things. It is really a common practice. Some projects have many flavors.

The big difference between plain Kotlin projects vs Android is that Kotlin only has main as a sourceSet by default whereas Android has many combination of sourceSets and most of them are empty. Meta task idea is good but then naming would be hard, wouldn't it?

As an example: flavor1Debug already has ktlintFlavor1DebugCheck task which does not check the sourceSets for flavor1Debug but checks the flavor1Debug which is usually empty in most projects.

[Tapchicoma]

Possible way:

• add Sources to names of current tasks, so it will become ktlintFlavor1DebugSourcesCheck
• use ktlintFlavor1DebugCheck as variant meta task name

Or:

• use Variant for this meta tasks name, so they will be something like ktlintFlavor1DebugVariantCheck

[tasomaniac]

Both sounds good to me. 👍

[Tapchicoma]

Opened new issue #170, as this particular problem doesn't related to original bug problem.

[tasomaniac]

Make sense.