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

Run detekt's Gradle tasks with Gradle's Worker API #4128

Merged
merged 6 commits into from Feb 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions detekt-gradle-plugin/build.gradle.kts
Expand Up @@ -69,6 +69,7 @@ dependencies {
compileOnly(libs.kotlin.gradle)
compileOnly(libs.kotlin.gradlePluginApi)
implementation(libs.sarif4k)
compileOnly("io.gitlab.arturbosch.detekt:detekt-cli:1.22.0")

testKitRuntimeOnly(libs.kotlin.gradle)
testKitJava11RuntimeOnly(libs.android.gradle.maxSupported)
Expand Down
Expand Up @@ -16,6 +16,7 @@ import io.gitlab.arturbosch.detekt.invoke.CustomReportArgument
import io.gitlab.arturbosch.detekt.invoke.DebugArgument
import io.gitlab.arturbosch.detekt.invoke.DefaultReportArgument
import io.gitlab.arturbosch.detekt.invoke.DetektInvoker
import io.gitlab.arturbosch.detekt.invoke.DetektWorkAction
import io.gitlab.arturbosch.detekt.invoke.DisableDefaultRuleSetArgument
import io.gitlab.arturbosch.detekt.invoke.InputArgument
import io.gitlab.arturbosch.detekt.invoke.JdkHomeArgument
Expand All @@ -32,6 +33,7 @@ import org.gradle.api.file.RegularFileProperty
import org.gradle.api.model.ObjectFactory
import org.gradle.api.provider.Property
import org.gradle.api.provider.Provider
import org.gradle.api.provider.ProviderFactory
import org.gradle.api.reporting.ReportingExtension
import org.gradle.api.tasks.CacheableTask
import org.gradle.api.tasks.Classpath
Expand All @@ -53,12 +55,15 @@ import org.gradle.api.tasks.TaskAction
import org.gradle.api.tasks.VerificationTask
import org.gradle.api.tasks.options.Option
import org.gradle.language.base.plugins.LifecycleBasePlugin
import org.gradle.workers.WorkerExecutor
import java.io.File
import javax.inject.Inject

@CacheableTask
abstract class Detekt @Inject constructor(
private val objects: ObjectFactory
private val objects: ObjectFactory,
private val workerExecutor: WorkerExecutor,
private val providers: ProviderFactory,
) : SourceTask(), VerificationTask {

@get:Classpath
Expand Down Expand Up @@ -249,12 +254,28 @@ abstract class Detekt @Inject constructor(

@TaskAction
fun check() {
DetektInvoker.create(task = this, isDryRun = isDryRun.orNull.toBoolean()).invokeCli(
arguments = arguments,
ignoreFailures = ignoreFailures,
classpath = detektClasspath.plus(pluginClasspath),
taskName = name
)
if (providers.gradleProperty(USE_WORKER_API).getOrElse("false") == "true") {
logger.info("Executing $name using Worker API")
val workQueue = workerExecutor.processIsolation { workerSpec ->
workerSpec.classpath.from(detektClasspath)
workerSpec.classpath.from(pluginClasspath)
}

workQueue.submit(DetektWorkAction::class.java) { workParameters ->
workParameters.arguments.set(arguments)
workParameters.ignoreFailures.set(ignoreFailures)
workParameters.dryRun.set(isDryRun.orNull.toBoolean())
workParameters.taskName.set(name)
}
} else {
logger.info("Executing $name using DetektInvoker")
DetektInvoker.create(isDryRun = isDryRun.orNull.toBoolean()).invokeCli(
arguments = arguments,
ignoreFailures = ignoreFailures,
classpath = detektClasspath.plus(pluginClasspath),
taskName = name
)
}
}

private fun convertCustomReportsToArguments(): List<CustomReportArgument> = reports.custom.map {
Expand Down
Expand Up @@ -11,6 +11,7 @@ import io.gitlab.arturbosch.detekt.invoke.ConfigArgument
import io.gitlab.arturbosch.detekt.invoke.CreateBaselineArgument
import io.gitlab.arturbosch.detekt.invoke.DebugArgument
import io.gitlab.arturbosch.detekt.invoke.DetektInvoker
import io.gitlab.arturbosch.detekt.invoke.DetektWorkAction
import io.gitlab.arturbosch.detekt.invoke.DisableDefaultRuleSetArgument
import io.gitlab.arturbosch.detekt.invoke.InputArgument
import io.gitlab.arturbosch.detekt.invoke.JvmTargetArgument
Expand All @@ -20,6 +21,7 @@ import org.gradle.api.file.DirectoryProperty
import org.gradle.api.file.FileTree
import org.gradle.api.file.RegularFileProperty
import org.gradle.api.provider.Property
import org.gradle.api.provider.ProviderFactory
import org.gradle.api.tasks.CacheableTask
import org.gradle.api.tasks.Classpath
import org.gradle.api.tasks.Console
Expand All @@ -36,9 +38,14 @@ import org.gradle.api.tasks.SkipWhenEmpty
import org.gradle.api.tasks.SourceTask
import org.gradle.api.tasks.TaskAction
import org.gradle.language.base.plugins.LifecycleBasePlugin
import org.gradle.workers.WorkerExecutor
import javax.inject.Inject

@CacheableTask
abstract class DetektCreateBaselineTask : SourceTask() {
abstract class DetektCreateBaselineTask @Inject constructor(
private val workerExecutor: WorkerExecutor,
private val providers: ProviderFactory,
) : SourceTask() {

init {
description = "Creates a detekt baseline on the given --baseline path."
Expand Down Expand Up @@ -139,11 +146,26 @@ abstract class DetektCreateBaselineTask : SourceTask() {

@TaskAction
fun baseline() {
DetektInvoker.create(task = this).invokeCli(
arguments = arguments,
ignoreFailures = ignoreFailures.getOrElse(false),
classpath = detektClasspath.plus(pluginClasspath),
taskName = name
)
if (providers.gradleProperty(USE_WORKER_API).getOrElse("false") == "true") {
logger.info("Executing $name using Worker API")
val workQueue = workerExecutor.processIsolation { workerSpec ->
workerSpec.classpath.from(detektClasspath)
workerSpec.classpath.from(pluginClasspath)
}

workQueue.submit(DetektWorkAction::class.java) { workParameters ->
workParameters.arguments.set(arguments)
workParameters.ignoreFailures.set(ignoreFailures)
workParameters.taskName.set(name)
}
} else {
logger.info("Executing $name using DetektInvoker")
DetektInvoker.create().invokeCli(
arguments = arguments,
ignoreFailures = ignoreFailures.getOrElse(false),
classpath = detektClasspath.plus(pluginClasspath),
taskName = name
)
}
}
}
Expand Up @@ -5,11 +5,13 @@ import io.gitlab.arturbosch.detekt.DetektPlugin.Companion.CONFIG_FILE
import io.gitlab.arturbosch.detekt.invoke.CliArgument
import io.gitlab.arturbosch.detekt.invoke.ConfigArgument
import io.gitlab.arturbosch.detekt.invoke.DetektInvoker
import io.gitlab.arturbosch.detekt.invoke.DetektWorkAction
import io.gitlab.arturbosch.detekt.invoke.GenerateConfigArgument
import org.gradle.api.DefaultTask
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.RegularFileProperty
import org.gradle.api.model.ObjectFactory
import org.gradle.api.provider.ProviderFactory
import org.gradle.api.services.BuildService
import org.gradle.api.services.BuildServiceParameters
import org.gradle.api.tasks.CacheableTask
Expand All @@ -18,13 +20,16 @@ import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.OutputFile
import org.gradle.api.tasks.TaskAction
import org.gradle.language.base.plugins.LifecycleBasePlugin
import org.gradle.workers.WorkerExecutor
import java.io.File
import java.nio.file.Files
import javax.inject.Inject

@CacheableTask
abstract class DetektGenerateConfigTask @Inject constructor(
objects: ObjectFactory
objects: ObjectFactory,
private val workerExecutor: WorkerExecutor,
private val providers: ProviderFactory,
) : DefaultTask() {

init {
Expand Down Expand Up @@ -71,11 +76,25 @@ abstract class DetektGenerateConfigTask @Inject constructor(

Files.createDirectories(configFile.get().asFile.parentFile.toPath())

DetektInvoker.create(task = this).invokeCli(
arguments = arguments,
classpath = detektClasspath.plus(pluginClasspath),
taskName = name,
)
if (providers.gradleProperty(USE_WORKER_API).getOrElse("false") == "true") {
logger.info("Executing $name using Worker API")
val workQueue = workerExecutor.processIsolation { workerSpec ->
workerSpec.classpath.from(detektClasspath)
3flex marked this conversation as resolved.
Show resolved Hide resolved
workerSpec.classpath.from(pluginClasspath)
}

workQueue.submit(DetektWorkAction::class.java) { workParameters ->
workParameters.arguments.set(arguments)
workParameters.taskName.set(name)
}
} else {
logger.info("Executing $name using DetektInvoker")
DetektInvoker.create().invokeCli(
arguments = arguments,
classpath = detektClasspath.plus(pluginClasspath),
taskName = name,
)
}
}

@Suppress("UnnecessaryAbstractClass")
Expand Down
Expand Up @@ -136,3 +136,4 @@ class DetektPlugin : Plugin<Project> {

const val CONFIGURATION_DETEKT = "detekt"
const val CONFIGURATION_DETEKT_PLUGINS = "detektPlugins"
const val USE_WORKER_API = "detekt.use.worker.api"
Expand Up @@ -3,9 +3,12 @@ package io.gitlab.arturbosch.detekt.invoke
import io.gitlab.arturbosch.detekt.internal.ClassLoaderCache
import io.gitlab.arturbosch.detekt.internal.GlobalClassLoaderCache
import org.gradle.api.GradleException
import org.gradle.api.Task
import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.FileCollection
import org.gradle.api.logging.Logger
import org.gradle.api.provider.ListProperty
import org.gradle.api.provider.Property
import org.gradle.workers.WorkAction
import org.gradle.workers.WorkParameters
import java.io.PrintStream
import java.lang.reflect.InvocationTargetException

Expand All @@ -20,15 +23,57 @@ internal interface DetektInvoker {

companion object {

fun create(task: Task, isDryRun: Boolean = false): DetektInvoker =
fun create(isDryRun: Boolean = false): DetektInvoker =
if (isDryRun) {
DryRunInvoker(task.logger)
DryRunInvoker()
} else {
DefaultCliInvoker()
}
}
}

interface DetektWorkParameters : WorkParameters {
val arguments: ListProperty<String>
val ignoreFailures: Property<Boolean>
val dryRun: Property<Boolean>
val taskName: Property<String>
val classpath: ConfigurableFileCollection
}

abstract class DetektWorkAction : WorkAction<DetektWorkParameters> {
@Suppress("SwallowedException", "TooGenericExceptionCaught")
override fun execute() {
if (parameters.dryRun.getOrElse(false)) {
DryRunInvoker().invokeCli(
parameters.arguments.get(),
parameters.classpath,
parameters.taskName.get(),
parameters.ignoreFailures.getOrElse(false)
)
return
}

try {
@Suppress("DEPRECATION")
val runner = io.gitlab.arturbosch.detekt.cli.buildRunner(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why the fully qualified import here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't recall the original reason, but I've now suppressed the deprecation on buildRunner. If this is an import instead, then the suppression would have to be on the file level, as you can't suppress deprecations in the import list. See most recent commit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take a look at https://github.com/detekt/detekt/pull/2896/files#diff-63765919cc19f1899b99d724bfe9c613285bf9967a35df542daf4974cac7d991R13
on how to run DetektCli without relying on internal cli module.

Edit: Ok, maybe not as I mentioned in the description of linked pr: #2896 (comment)

We will need to stay on our custom classloader cache and call the cli module reflectively for the best performance.

parameters.arguments.get().toTypedArray(),
System.out,
System.err
)
runner.execute()
} catch (e: Exception) {
if (isBuildFailure(e.message) && parameters.ignoreFailures.get()) {
return
} else {
throw GradleException(e.message ?: "There was a problem running detekt.")
}
}
}

private fun isBuildFailure(msg: String?) =
msg != null && "Build failed with" in msg && "issues" in msg
}

internal class DefaultCliInvoker(
private val classLoaderCache: ClassLoaderCache = GlobalClassLoaderCache
) : DetektInvoker {
Expand Down Expand Up @@ -61,18 +106,18 @@ internal class DefaultCliInvoker(
private fun isAnalysisFailure(msg: String) = "Analysis failed with" in msg && "issues" in msg
}

private class DryRunInvoker(private val logger: Logger) : DetektInvoker {
private class DryRunInvoker : DetektInvoker {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger can't be passed to the Worker as everything in DetektWorkParameters must be serializable. Using println instead means the tests still pass as anything passed to stdout is logged at QUIET level by Gradle. There's no impact on users as the dry run invoker should only be used internally.


override fun invokeCli(
arguments: List<String>,
classpath: FileCollection,
taskName: String,
ignoreFailures: Boolean
) {
logger.info("Invoking detekt with dry-run.")
logger.info("Task: $taskName")
logger.info("Arguments: ${arguments.joinToString(" ")}")
logger.info("Classpath: ${classpath.files}")
logger.info("Ignore failures: $ignoreFailures")
println("Invoking detekt with dry-run.")
println("Task: $taskName")
println("Arguments: ${arguments.joinToString(" ")}")
println("Classpath: ${classpath.files}")
println("Ignore failures: $ignoreFailures")
}
}