From 58575daddb8c5d0a3754dfbd82ea8642f123ff52 Mon Sep 17 00:00:00 2001 From: Artur Bosch Date: Sun, 24 May 2020 11:03:26 +0200 Subject: [PATCH] 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 | 2 +- buildSrc/src/main/kotlin/apps.gradle.kts | 10 ----- buildSrc/src/main/kotlin/commons.gradle.kts | 4 ++ buildSrc/src/main/kotlin/packaging.gradle.kts | 7 +++ detekt-api/build.gradle.kts | 2 +- .../gitlab/arturbosch/detekt/cli/CliArgs.kt | 8 +--- .../arturbosch/detekt/cli/JCommander.kt | 35 ++++++++------- .../io/gitlab/arturbosch/detekt/cli/Main.kt | 20 +-------- .../arturbosch/detekt/cli/CliArgsSpec.kt | 42 +++++++++++++++--- .../arturbosch/detekt/cli/out/ReportsSpec.kt | 2 +- detekt-generator/build.gradle.kts | 44 +++++++------------ .../generator/{Runner.kt => Generator.kt} | 27 ++++++------ .../detekt/generator/GeneratorArgs.kt | 33 ++++++++------ .../arturbosch/detekt/generator/Main.kt | 30 ++++++------- detekt-rules/build.gradle.kts | 2 - 15 files changed, 136 insertions(+), 132 deletions(-) delete mode 100644 buildSrc/src/main/kotlin/apps.gradle.kts rename detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/{Runner.kt => Generator.kt} (52%) diff --git a/build.gradle.kts b/build.gradle.kts index 91e2f93b2..4c9320327 100644 --- a/build.gradle.kts +++ b/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") } diff --git a/buildSrc/src/main/kotlin/apps.gradle.kts b/buildSrc/src/main/kotlin/apps.gradle.kts deleted file mode 100644 index 4d3988ba1..000000000 --- a/buildSrc/src/main/kotlin/apps.gradle.kts +++ /dev/null @@ -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") - } -} diff --git a/buildSrc/src/main/kotlin/commons.gradle.kts b/buildSrc/src/main/kotlin/commons.gradle.kts index 0b211be63..95c140e71 100644 --- a/buildSrc/src/main/kotlin/commons.gradle.kts +++ b/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") diff --git a/buildSrc/src/main/kotlin/packaging.gradle.kts b/buildSrc/src/main/kotlin/packaging.gradle.kts index 31431abe5..3ba65b5b3 100644 --- a/buildSrc/src/main/kotlin/packaging.gradle.kts +++ b/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 { diff --git a/detekt-api/build.gradle.kts b/detekt-api/build.gradle.kts index 8a3bc3f91..e196ef0b4 100644 --- a/detekt-api/build.gradle.kts +++ b/detekt-api/build.gradle.kts @@ -17,7 +17,7 @@ tasks.withType().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 diff --git a/detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/CliArgs.kt b/detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/CliArgs.kt index 75384a54d..ce8778698 100644 --- a/detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/CliArgs.kt +++ b/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"], diff --git a/detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/JCommander.kt b/detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/JCommander.kt index 1f8dda646..6f5299081 100644 --- a/detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/JCommander.kt +++ b/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 parseArguments( +fun parseArguments( args: Array, 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 parseArguments( } val violations = mutableListOf() - 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() diff --git a/detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/Main.kt b/detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/Main.kt index ba6c2c5e2..85900d4f7 100644 --- a/detekt-cli/src/main/kotlin/io/gitlab/arturbosch/detekt/cli/Main.kt +++ b/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( - 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) diff --git a/detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/CliArgsSpec.kt b/detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/CliArgsSpec.kt index c4ccd4067..31ea834fa 100644 --- a/detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/CliArgsSpec.kt +++ b/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(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( + 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( + 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(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) = 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")) } } } }) diff --git a/detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/out/ReportsSpec.kt b/detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/out/ReportsSpec.kt index 10adf77b1..18ea7c3d4 100644 --- a/detekt-cli/src/test/kotlin/io/gitlab/arturbosch/detekt/cli/out/ReportsSpec.kt +++ b/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(args, NullPrintStream(), NullPrintStream()) + val cli = parseArguments(args, NullPrintStream(), NullPrintStream()) val reports = cli.reportPaths diff --git a/detekt-generator/build.gradle.kts b/detekt-generator/build.gradle.kts index b67554e21..d8c4e5563 100644 --- a/detekt-generator/build.gradle.kts +++ b/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")) -} diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/Runner.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/Generator.kt similarity index 52% rename from detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/Runner.kt rename to detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/Generator.kt index b73dbfbe4..8f30d3aac 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/Runner.kt +++ b/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 = + 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) } diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/GeneratorArgs.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/GeneratorArgs.kt index c75be8316..edc4bc47b 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/GeneratorArgs.kt +++ b/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 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!" }) } diff --git a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/Main.kt b/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/Main.kt index 5ad9b55b3..d4b72ee11 100644 --- a/detekt-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/Main.kt +++ b/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) { - val arguments = parseArguments( - 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() } diff --git a/detekt-rules/build.gradle.kts b/detekt-rules/build.gradle.kts index e1e7ff671..a493cb06b 100644 --- a/detekt-rules/build.gradle.kts +++ b/detekt-rules/build.gradle.kts @@ -1,5 +1,3 @@ -tasks.build { finalizedBy(":detekt-generator:generateDocumentation") } - dependencies { implementation(project(":detekt-api")) implementation(project(":detekt-metrics"))