Faster documentation generation (#2722)

* Get rid of cli and core dependency for generator

* Remove generic argument parsing made for sharing but introduced complexity

* Do not use expensive shadow plugin on internal generator module

* Apply shadow plugin for cli in packaging

* Generate documentation after compilation of rules

* Trigger generateDocumentation when formatting rules are changed

* Further reduce complexity of parsing args

* Start generateDocumentation directly from gradle

* Test additional error paths of CliArgs

build.gradle.kts

@@ -1,10 +1,10 @@
 plugins {
     commons
-    apps
     packaging
     releasing
     detekt
     id("org.jetbrains.dokka") apply false
+    id("com.github.johnrengelman.shadow") apply false
     id("com.github.ben-manes.versions")
     id("org.sonarqube")
 }

buildSrc/src/main/kotlin/apps.gradle.kts

@@ -1,10 +0,0 @@
-plugins {
-    id("com.github.johnrengelman.shadow") apply false
-}
-
-configure(listOf(project(":detekt-cli"), project(":detekt-generator"))) {
-    apply {
-        plugin("application")
-        plugin("com.github.johnrengelman.shadow")
-    }
-}

buildSrc/src/main/kotlin/commons.gradle.kts

@@ -75,6 +75,10 @@ subprojects {
     }
 }
 
+configure(listOf(project(":detekt-rules"), project(":detekt-formatting"))) {
+    tasks.build { finalizedBy(":detekt-generator:generateDocumentation") }
+}
+
 jacoco.toolVersion = Versions.JACOCO
 
 val examplesOrTestUtils = setOf("detekt-test", "detekt-test-utils", "detekt-sample-extensions")

buildSrc/src/main/kotlin/packaging.gradle.kts

@@ -11,6 +11,13 @@ plugins {
     id("com.jfrog.bintray") apply false
 }
 
+project(":detekt-cli") {
+    apply {
+        plugin("application")
+        plugin("com.github.johnrengelman.shadow")
+    }
+}
+
 subprojects {
 
     apply {

detekt-api/build.gradle.kts

@@ -17,7 +17,7 @@ tasks.withType<DokkaTask>().configureEach {
     outputFormat = "jekyll"
     outputDirectory = "$rootDir/docs/pages/kdoc"
     configuration {
-        // suppresses undocumented classes but not dokka warnings https://github.com/Kotlin/dokka/issues/90
+        moduleName = project.name
         reportUndocumented = false
         @Suppress("MagicNumber")
         jdkVersion = 8

detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/CliArgs.kt

@@ -5,11 +5,7 @@ import org.jetbrains.kotlin.config.JvmTarget
 import org.jetbrains.kotlin.config.LanguageVersion
 import java.nio.file.Path
 
-interface Args {
-    var help: Boolean
-}
-
-class CliArgs : Args {
+class CliArgs {
 
     @Parameter(
         names = ["--input", "-i"],
@@ -129,7 +125,7 @@ class CliArgs : Args {
         names = ["--help", "-h"],
         help = true, description = "Shows the usage."
     )
-    override var help: Boolean = false
+    var help: Boolean = false
 
     @Parameter(
         names = ["--run-rule"],

detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/JCommander.kt

@@ -2,23 +2,19 @@ package io.gitlab.arturbosch.detekt.cli
 
 import com.beust.jcommander.JCommander
 import com.beust.jcommander.ParameterException
+import io.gitlab.arturbosch.detekt.core.exists
+import io.gitlab.arturbosch.detekt.core.isFile
 import java.io.PrintStream
 
 @Suppress("detekt.SpreadOperator", "detekt.ThrowsCount")
-inline fun <reified T : Args> parseArguments(
+fun parseArguments(
     args: Array<out String>,
     outPrinter: PrintStream,
-    errorPrinter: PrintStream,
-    validateCli: T.(MessageCollector) -> Unit = {}
-): T {
-    val cli = T::class.java.declaredConstructors
-        .firstOrNull()
-        ?.newInstance() as? T
-        ?: error("Could not create Args object for class ${T::class.java}")
+    errorPrinter: PrintStream
+): CliArgs {
+    val cli = CliArgs()
 
-    val jCommander = JCommander()
-
-    jCommander.addObject(cli)
+    val jCommander = JCommander(cli)
     jCommander.programName = "detekt"
 
     try {
@@ -35,11 +31,20 @@ inline fun <reified T : Args> parseArguments(
     }
 
     val violations = mutableListOf<String>()
-    validateCli(cli, object : MessageCollector {
-        override fun plusAssign(msg: String) {
-            violations += msg
+    val baseline = cli.baseline
+
+    if (cli.createBaseline && baseline == null) {
+        violations += "Creating a baseline.xml requires the --baseline parameter to specify a path."
+    }
+
+    if (!cli.createBaseline && baseline != null) {
+        if (!baseline.exists()) {
+            violations += "The file specified by --baseline should exist '$baseline'."
+        } else if (!baseline.isFile()) {
+            violations += "The path specified by --baseline should be a file '$baseline'."
         }
-    })
+    }
+
     if (violations.isNotEmpty()) {
         violations.forEach(errorPrinter::println)
         errorPrinter.println()

detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/Main.kt

@@ -9,8 +9,6 @@ import io.gitlab.arturbosch.detekt.cli.runners.Executable
 import io.gitlab.arturbosch.detekt.cli.runners.Runner
 import io.gitlab.arturbosch.detekt.cli.runners.SingleRuleRunner
 import io.gitlab.arturbosch.detekt.cli.runners.VersionPrinter
-import io.gitlab.arturbosch.detekt.core.exists
-import io.gitlab.arturbosch.detekt.core.isFile
 import java.io.PrintStream
 import kotlin.system.exitProcess
 
@@ -41,23 +39,7 @@ fun buildRunner(
     outputPrinter: PrintStream,
     errorPrinter: PrintStream
 ): Executable {
-    val arguments = parseArguments<CliArgs>(
-        args,
-        outputPrinter,
-        errorPrinter
-    ) { messages ->
-        val baseline = baseline
-        if (createBaseline && baseline == null) {
-            messages += "Creating a baseline.xml requires the --baseline parameter to specify a path."
-        }
-        if (!createBaseline && baseline != null) {
-            if (!baseline.exists()) {
-                messages += "The file specified by --baseline should exist '$baseline'."
-            } else if (!baseline.isFile()) {
-                messages += "The path specified by --baseline should be a file '$baseline'."
-            }
-        }
-    }
+    val arguments = parseArguments(args, outputPrinter, errorPrinter)
     return when {
         arguments.showVersion -> VersionPrinter(outputPrinter)
         arguments.generateConfig -> ConfigExporter(arguments)

detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/CliArgsSpec.kt

@@ -17,13 +17,13 @@ internal class CliArgsSpec : Spek({
     describe("Parsing the input path") {
 
         it("the current working directory is used if parameter is not set") {
-            val cli = parseArguments<CliArgs>(emptyArray(), NullPrintStream(), NullPrintStream())
+            val cli = parseArguments(emptyArray(), NullPrintStream(), NullPrintStream())
             assertThat(cli.inputPaths).hasSize(1)
             assertThat(cli.inputPaths.first()).isEqualTo(Paths.get(System.getProperty("user.dir")))
         }
 
         it("a single value is converted to a path") {
-            val cli = parseArguments<CliArgs>(
+            val cli = parseArguments(
                 arrayOf("--input", "$projectPath"),
                 NullPrintStream(),
                 NullPrintStream())
@@ -34,7 +34,7 @@ internal class CliArgsSpec : Spek({
         it("multiple input paths can be separated by comma") {
             val mainPath = projectPath.resolve("src/main").toAbsolutePath()
             val testPath = projectPath.resolve("src/test").toAbsolutePath()
-            val cli = parseArguments<CliArgs>(
+            val cli = parseArguments(
                 arrayOf("--input", "$mainPath,$testPath"),
                 NullPrintStream(),
                 NullPrintStream())
@@ -47,13 +47,43 @@ internal class CliArgsSpec : Spek({
             val params = arrayOf("--input", "$pathToNonExistentDirectory")
 
             assertThatExceptionOfType(ParameterException::class.java)
-                .isThrownBy { parseArguments<CliArgs>(params, NullPrintStream(), NullPrintStream()).inputPaths }
+                .isThrownBy { parseArguments(params, NullPrintStream(), NullPrintStream()).inputPaths }
                 .withMessageContaining("does not exist")
         }
+    }
 
-        it("reports an error when using --create-baseline without a --baseline file") {
+    describe("Valid combination of options") {
+
+        fun fixture(args: Array<String>) = parseArguments(args, NullPrintStream(), NullPrintStream())
+
+        describe("Baseline feature") {
+
+            it("reports an error when using --create-baseline without a --baseline file") {
+                assertThatExceptionOfType(HandledArgumentViolation::class.java)
+                    .isThrownBy { fixture(arrayOf("--create-baseline")) }
+            }
+
+            it("reports an error when using --baseline file does not exist") {
+                val pathToNonExistentDirectory = projectPath.resolve("nonExistent").toString()
+                assertThatExceptionOfType(HandledArgumentViolation::class.java)
+                    .isThrownBy { fixture(arrayOf("--baseline", pathToNonExistentDirectory)) }
+            }
+
+            it("reports an error when using --baseline file which is not a file") {
+                val directory = Paths.get(resource("/cases")).toString()
+                assertThatExceptionOfType(HandledArgumentViolation::class.java)
+                    .isThrownBy { fixture(arrayOf("--baseline", directory)) }
+            }
+        }
+
+        it("throws HelpRequest on --help") {
+            assertThatExceptionOfType(HelpRequest::class.java)
+                .isThrownBy { fixture(arrayOf("--help")) }
+        }
+
+        it("throws HandledArgumentViolation on wrong options") {
             assertThatExceptionOfType(HandledArgumentViolation::class.java)
-                .isThrownBy { buildRunner(arrayOf("--create-baseline"), NullPrintStream(), NullPrintStream()) }
+                .isThrownBy { fixture(arrayOf("--unknown-to-us-all")) }
         }
     }
 })

detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/out/ReportsSpec.kt

@@ -28,7 +28,7 @@ internal class ReportsSpec : Spek({
                 "--report", "$reportUnderTest:/tmp/path3",
                 "--report", "html:D:_Gradle\\xxx\\xxx\\build\\reports\\detekt\\detekt.html"
             )
-            val cli = parseArguments<CliArgs>(args, NullPrintStream(), NullPrintStream())
+            val cli = parseArguments(args, NullPrintStream(), NullPrintStream())
 
             val reports = cli.reportPaths
 

detekt-generator/build.gradle.kts

@@ -1,42 +1,40 @@
 import java.io.ByteArrayOutputStream
 
-apply {
-    plugin("application")
-    plugin("com.github.johnrengelman.shadow")
+dependencies {
+    implementation(project(":detekt-parser"))
+    implementation(project(":detekt-api"))
+    implementation(project(":detekt-rules"))
+    implementation(project(":detekt-formatting"))
+    implementation("com.beust:jcommander:${Versions.JCOMMANDER}")
+
+    testImplementation(project(":detekt-test-utils"))
 }
 
-application {
-    mainClassName = "io.gitlab.arturbosch.detekt.generator.Main"
-}
-
-val jar by tasks.getting(Jar::class) {
-    manifest {
-        attributes.apply { put("Main-Class", "io.gitlab.arturbosch.detekt.generator.Main") }
-    }
-}
+val documentationDir = "${rootProject.rootDir}/docs/pages/documentation"
 
 val generateDocumentation by tasks.registering {
-    dependsOn(tasks.shadowJar, ":detekt-api:dokka")
+    dependsOn(tasks.build, ":detekt-api:dokka")
     description = "Generates detekt documentation and the default config.yml based on Rule KDoc"
     group = "documentation"
 
     inputs.files(
         fileTree("${rootProject.rootDir}/detekt-rules/src/main/kotlin"),
+        fileTree("${rootProject.rootDir}/detekt-formatting/src/main/kotlin"),
         file("${rootProject.rootDir}/detekt-generator/build/libs/detekt-generator-${Versions.DETEKT}-all.jar"))
     outputs.files(
-        fileTree("${rootProject.rootDir}/detekt-generator/documentation"),
+        fileTree(documentationDir),
         file("${rootProject.rootDir}/detekt-cli/src/main/resources/default-detekt-config.yml"))
 
     doLast {
         javaexec {
-            main = "-jar"
+            classpath(configurations.runtimeClasspath.get(), sourceSets.main.get().output)
+            main = "io.gitlab.arturbosch.detekt.generator.Main"
             args = listOf(
-                "${rootProject.rootDir}/detekt-generator/build/libs/detekt-generator-${Versions.DETEKT}-all.jar",
                 "--input",
                 "${rootProject.rootDir}/detekt-rules/src/main/kotlin" + "," +
                     "${rootProject.rootDir}/detekt-formatting/src/main/kotlin",
                 "--documentation",
-                "${rootProject.rootDir}/docs/pages/documentation",
+                documentationDir,
                 "--config",
                 "${rootProject.rootDir}/detekt-cli/src/main/resources")
         }
@@ -70,7 +68,7 @@ fun assertDocumentationUpToDate() {
     val configDiff = ByteArrayOutputStream()
     exec {
         commandLine = listOf(
-            "git", "diff", "${rootProject.rootDir}/docs/pages/documentation", "${rootProject.rootDir}/docs/pages/kdoc"
+            "git", "diff", documentationDir, "${rootProject.rootDir}/docs/pages/kdoc"
         )
         standardOutput = configDiff
     }
@@ -80,13 +78,3 @@ fun assertDocumentationUpToDate() {
             "Please build detekt locally to update it and commit the changed files.")
     }
 }
-
-dependencies {
-    implementation(project(":detekt-cli"))
-    implementation(project(":detekt-core"))
-    implementation(project(":detekt-rules"))
-    implementation(project(":detekt-formatting"))
-    implementation("com.beust:jcommander:${Versions.JCOMMANDER}")
-
-    testImplementation(project(":detekt-test-utils"))
-}

detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/Generator.kt

@@ -1,34 +1,35 @@
 package io.gitlab.arturbosch.detekt.generator
 
-import io.gitlab.arturbosch.detekt.core.KtTreeCompiler
-import io.gitlab.arturbosch.detekt.core.ProcessingSettings
+import io.github.detekt.parser.KtCompiler
 import io.gitlab.arturbosch.detekt.generator.collection.DetektCollector
 import io.gitlab.arturbosch.detekt.generator.printer.DetektPrinter
+import org.jetbrains.kotlin.psi.KtFile
 import java.io.PrintStream
+import java.nio.file.Files
 import java.nio.file.Path
+import java.util.stream.Collectors
 import kotlin.system.measureTimeMillis
 
-class Runner(
+class Generator(
     private val arguments: GeneratorArgs,
-    private val outPrinter: PrintStream,
-    private val errPrinter: PrintStream
+    private val outPrinter: PrintStream = System.out
 ) {
     private val collector = DetektCollector()
     private val printer = DetektPrinter(arguments)
 
-    private fun createCompiler(path: Path) = KtTreeCompiler.instance(ProcessingSettings(
-        listOf(path),
-        outPrinter = outPrinter,
-        errPrinter = errPrinter))
+    private fun parseAll(parser: KtCompiler, root: Path): Collection<KtFile> =
+        Files.walk(root)
+            .filter { it.fileName.toString().endsWith(".kt") }
+            .map { parser.compile(root, it) }
+            .collect(Collectors.toList())
 
     fun execute() {
+        val parser = KtCompiler()
         val time = measureTimeMillis {
             val ktFiles = arguments.inputPath
-                .flatMap { createCompiler(it).compile(it) }
+                .flatMap { parseAll(parser, it) }
 
-            ktFiles.forEach { file ->
-                collector.visit(file)
-            }
+            ktFiles.forEach(collector::visit)
 
             printer.print(collector.items)
         }

detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/GeneratorArgs.kt

@@ -1,12 +1,11 @@
 package io.gitlab.arturbosch.detekt.generator
 
 import com.beust.jcommander.Parameter
-import io.gitlab.arturbosch.detekt.cli.Args
-import io.gitlab.arturbosch.detekt.cli.ExistingPathConverter
-import io.gitlab.arturbosch.detekt.cli.MultipleExistingPathConverter
+import java.nio.file.Files
 import java.nio.file.Path
+import java.nio.file.Paths
 
-class GeneratorArgs : Args {
+class GeneratorArgs {
 
     @Parameter(names = ["--input", "-i"],
         required = true,
@@ -15,26 +14,32 @@ class GeneratorArgs : Args {
 
     @Parameter(names = ["--documentation", "-d"],
         required = true,
-        converter = ExistingPathConverter::class, description = "Output path for generated documentation.")
-    private var documentation: Path? = null
+        description = "Output path for generated documentation.")
+    private var documentation: String? = null
 
     @Parameter(names = ["--config", "-c"],
         required = true,
-        converter = ExistingPathConverter::class, description = "Output path for generated detekt config.")
-    private var config: Path? = null
+        description = "Output path for generated detekt config.")
+    private var config: String? = null
 
     @Parameter(names = ["--help", "-h"],
         help = true, description = "Shows the usage.")
-    override var help: Boolean = false
+    var help: Boolean = false
 
     val inputPath: List<Path> by lazy {
-        MultipleExistingPathConverter().convert(
-            checkNotNull(input) { "Input parameter was not initialized by jcommander!" }
-        )
+        checkNotNull(input) { "Input parameter was not initialized by jcommander!" }
+            .splitToSequence(",", ";")
+            .map(String::trim)
+            .filter { it.isNotEmpty() }
+            .map { first -> Paths.get(first) }
+            .onEach { require(Files.exists(it)) { "Input path must exist!" } }
+            .toList()
     }
     val documentationPath: Path
-        get() = checkNotNull(documentation) { "Documentation output path was not initialized by jcommander!" }
+        get() = Paths.get(checkNotNull(documentation) {
+            "Documentation output path was not initialized by jcommander!"
+        })
 
     val configPath: Path
-        get() = checkNotNull(config) { "Configuration output path was not initialized by jcommander!" }
+        get() = Paths.get(checkNotNull(config) { "Configuration output path was not initialized by jcommander!" })
 }

detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/Main.kt

@@ -2,25 +2,23 @@
 
 package io.gitlab.arturbosch.detekt.generator
 
-import io.gitlab.arturbosch.detekt.cli.parseArguments
-import io.gitlab.arturbosch.detekt.core.isFile
+import com.beust.jcommander.JCommander
 import java.nio.file.Files
+import kotlin.system.exitProcess
 
+@Suppress("detekt.SpreadOperator")
 fun main(args: Array<String>) {
-    val arguments = parseArguments<GeneratorArgs>(
-        args,
-        System.out,
-        System.err
-    ) { messages ->
-        if (Files.exists(documentationPath) && documentationPath.isFile()) {
-            messages += "Documentation path must be a directory."
-        }
+    val options = GeneratorArgs()
+    val parser = JCommander(options)
+    parser.parse(*args)
 
-        if (Files.exists(configPath) && configPath.isFile()) {
-            messages += "Config path must be a directory."
-        }
-        // input paths are validated by MultipleExistingPathConverter
+    if (options.help) {
+        parser.usage()
+        exitProcess(0)
     }
-    val executable = Runner(arguments, System.out, System.err)
-    executable.execute()
+
+    require(Files.isDirectory(options.documentationPath)) { "Documentation path must be a directory." }
+    require(Files.isDirectory(options.configPath)) { "Config path must be a directory." }
+
+    Generator(options).execute()
 }

detekt-rules/build.gradle.kts

@@ -1,5 +1,3 @@
-tasks.build { finalizedBy(":detekt-generator:generateDocumentation") }
-
 dependencies {
     implementation(project(":detekt-api"))
     implementation(project(":detekt-metrics"))