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

Feature. Enable confiuring of ktlint reporter in gradle plugin #724

Merged
merged 15 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions diktat-gradle-plugin/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ dependencies {
exclude("com.pinterest.ktlint", "ktlint-ruleset-standard")
}
implementation("com.pinterest.ktlint:ktlint-reporter-plain:$ktlintVersion")
implementation("com.pinterest.ktlint:ktlint-reporter-json:$ktlintVersion")
implementation("com.pinterest.ktlint:ktlint-reporter-checkstyle:$ktlintVersion")
petertrr marked this conversation as resolved.
Show resolved Hide resolved
implementation("org.cqfn.diktat:diktat-rules:$diktatVersion")

testImplementation("org.junit.jupiter:junit-jupiter-api:$junitVersion")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.gradle.testkit.runner.TaskOutcome
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Test
import java.io.File

Expand Down Expand Up @@ -39,6 +40,32 @@ class DiktatGradlePluginFunctionalTest {
)
}

/*
* Should fix output here
petertrr marked this conversation as resolved.
Show resolved Hide resolved
*/
@Test
fun `should have json reporter files`() {
buildFile.appendText(
"""${System.lineSeparator()}
diktat {
inputs = files("src/**/*.kt")
reporterType = "json"
output = "test.txt"
}
""".trimIndent()
)
val result = runDiktat(testProjectDir, shouldSucceed = false)

val diktatCheckBuildResult = result.task(":$DIKTAT_CHECK_TASK")
requireNotNull(diktatCheckBuildResult)
Assertions.assertTrue(testProjectDir.root.walkTopDown().filter { it.name == "test.txt" }.firstOrNull() != null)
petertrr marked this conversation as resolved.
Show resolved Hide resolved
Assertions.assertEquals(TaskOutcome.FAILED, diktatCheckBuildResult.outcome)
val file = testProjectDir.root.walkTopDown().filter { it.name == "test.txt" }.first()
Assertions.assertTrue(
file.readLines().any { it.contains("[HEADER_MISSING_OR_WRONG_COPYRIGHT]") }
)
}

@Test
fun `should execute diktatCheck with explicit inputs`() {
buildFile.appendText(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.cqfn.diktat.plugin.gradle

import com.pinterest.ktlint.core.Reporter
import org.gradle.api.file.FileCollection
import org.gradle.api.tasks.Internal
import java.io.File

/**
Expand All @@ -18,6 +19,17 @@ open class DiktatExtension {
*/
var debug = false

/**
* Type of the reporter to use
*/
var reporterType: String = "plain"

/**
* Type of output
* Default: System.out
*/
var output: String = ""

/**
* Path to diktat yml config file. Can be either absolute or relative to project's root directory.
* Default value: `diktat-analysis.yml` in rootDir.
Expand All @@ -29,12 +41,6 @@ open class DiktatExtension {
*/
lateinit var excludes: FileCollection

/**
* Ktlint's [Reporter] which will be used during run.
* Private until I find a way to configure it.
*/
internal lateinit var reporter: Reporter

/**
* Paths that will be scanned for .kt(s) files
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class DiktatGradlePlugin : Plugin<Project> {
}
diktatConfigFile = project.rootProject.file("diktat-analysis.yml")
excludes = project.files()
reporter = PlainReporter(System.out)
}

// only gradle 7+ (or maybe 6.8) will embed kotlin 1.4+, kx.serialization is incompatible with kotlin 1.3, so until then we have to use JavaExec wrapper
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package org.cqfn.diktat.plugin.gradle

import com.pinterest.ktlint.reporter.checkstyle.CheckStyleReporter
import com.pinterest.ktlint.reporter.json.JsonReporter
import com.pinterest.ktlint.reporter.plain.PlainReporter
import org.cqfn.diktat.plugin.gradle.DiktatGradlePlugin.Companion.DIKTAT_CHECK_TASK
import org.cqfn.diktat.plugin.gradle.DiktatGradlePlugin.Companion.DIKTAT_FIX_TASK
import org.cqfn.diktat.ruleset.rules.DIKTAT_CONF_PROPERTY
Expand All @@ -17,6 +20,7 @@ import org.gradle.api.tasks.VerificationTask
import org.gradle.util.GradleVersion

import java.io.File
import java.io.PrintStream
import javax.inject.Inject

/**
Expand Down Expand Up @@ -50,6 +54,9 @@ open class DiktatJavaExecTaskBase @Inject constructor(
} else {
main = "com.pinterest.ktlint.Main"
}
if (diktatExtension.reporterType == "html") {
Copy link
Member

Choose a reason for hiding this comment

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

Is it needed? If yes, why onlu for html?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Others are already included in ktlint (plain, json, checkstyle)

Copy link
Member

Choose a reason for hiding this comment

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

Oh, ok, that's strange. Leave a comment in code to explain this, pls.

diktatConfiguration.dependencies.add(project.dependencies.create("com.pinterest.ktlint:ktlint-reporter-html:$KTLINT_VERSION"))
}
classpath = diktatConfiguration
project.logger.debug("Setting diktatCheck classpath to ${diktatConfiguration.dependencies.toSet()}")
if (diktatExtension.debug) {
Expand Down Expand Up @@ -83,6 +90,8 @@ open class DiktatJavaExecTaskBase @Inject constructor(
diktatExtension.excludes.files.forEach {
addPattern(it, negate = true)
}

add(createReporterFlag(diktatExtension))
}
logger.debug("Setting JavaExec args to $args")
}
Expand All @@ -107,6 +116,24 @@ open class DiktatJavaExecTaskBase @Inject constructor(
@Suppress("FUNCTION_BOOLEAN_PREFIX")
override fun getIgnoreFailures(): Boolean = ignoreFailuresProp.getOrElse(false)

private fun createReporterFlag(diktatExtension: DiktatExtension): String {
val flag: StringBuilder = StringBuilder()

// Plain, checkstyle and json reporter are provided out of the box in ktlint
when(diktatExtension.reporterType) {
"json" -> flag.append("--reporter=json")
"html" -> flag.append("--reporter=html")
"checkstyle" -> flag.append("--reporter=checkstyle")
else -> flag.append("--reporter=plain")
Copy link
Member

Choose a reason for hiding this comment

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

I think they already have baseline reporter (or was it added in 0.40.0?) Still, let's log a warning if unknown reporter is specified and then fallback to plain.

Copy link
Member

Choose a reason for hiding this comment

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

Also, one could imagine a use case, when end user adds a custom reporter as a dependency in our plugin's configuration and then specifies it's name (need to see, how it's loaded in ktlint). So it might be more versatile to allow arbitrary names here and then let ktlint validate them.

Copy link
Collaborator Author

@aktsay6 aktsay6 Jan 26, 2021

Choose a reason for hiding this comment

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

We need to specify it(--reporter=plain) here if a user will provide us an output here

}

if (diktatExtension.output.isNotEmpty()) {
flag.append(",output=${diktatExtension.output}")
petertrr marked this conversation as resolved.
Show resolved Hide resolved
}

return flag.toString()
}

@Suppress("MagicNumber")
private fun isMainClassPropertySupported(gradleVersionString: String) =
GradleVersion.version(gradleVersionString) >= GradleVersion.version("6.4")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,10 @@ class DiktatGradlePluginTest {
Assertions.assertIterableEquals(project.fileTree("src").files, diktatExtension.inputs.files)
Assertions.assertTrue(diktatExtension.inputs.files.isNotEmpty())
}

@Test
fun `check that the right reporter dependency added`() {
val diktatExtension = project.extensions.getByName("diktat") as DiktatExtension
Assertions.assertTrue(diktatExtension.reporterType == "plain")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class DiktatJavaExecTaskTest {
@Test
fun `check command line for various inputs`() {
assertCommandLineEquals(
listOf(null, combinePathParts("src", "**", "*.kt"))
listOf(null, combinePathParts("src", "**", "*.kt"), "--reporter=plain")
) {
inputs = project.files("src/**/*.kt")
}
Expand All @@ -32,7 +32,7 @@ class DiktatJavaExecTaskTest {
@Test
fun `check command line in debug mode`() {
assertCommandLineEquals(
listOf(null, "--debug", combinePathParts("src", "**", "*.kt"))
listOf(null, "--debug", combinePathParts("src", "**", "*.kt"), "--reporter=plain")
) {
inputs = project.files("src/**/*.kt")
debug = true
Expand All @@ -43,7 +43,7 @@ class DiktatJavaExecTaskTest {
fun `check command line with excludes`() {
assertCommandLineEquals(
listOf(null, combinePathParts("src", "**", "*.kt"),
"!${combinePathParts("src", "main", "kotlin", "generated")}"
"!${combinePathParts("src", "main", "kotlin", "generated")}", "--reporter=plain"
)
) {
inputs = project.files("src/**/*.kt")
Expand Down Expand Up @@ -76,6 +76,41 @@ class DiktatJavaExecTaskTest {
Assertions.assertEquals(File(project.projectDir.parentFile, "diktat-analysis.yml").absolutePath, task.systemProperties[DIKTAT_CONF_PROPERTY])
}

@Test
fun `check command line has reporter type and output`() {
assertCommandLineEquals(
listOf(null, "--reporter=json,output=some.txt")
) {
inputs = project.files()
diktatConfigFile = project.file("../diktat-analysis.yml")
reporterType = "json"
output = "some.txt"
}
}

@Test
fun `check command line has reporter type without output`() {
assertCommandLineEquals(
listOf(null, "--reporter=json")
) {
inputs = project.files()
diktatConfigFile = project.file("../diktat-analysis.yml")
reporterType = "json"
}
}

@Test
fun `check that project has html dependency`() {
val task = project.registerDiktatTask {
inputs = project.files()
diktatConfigFile = project.file("../diktat-analysis.yml")
reporterType = "html"
}

Assertions.assertTrue(project.configurations.getByName("diktat").dependencies.any { it.name == "ktlint-reporter-html" })
Assertions.assertEquals(File(project.projectDir.parentFile, "diktat-analysis.yml").absolutePath, task.systemProperties[DIKTAT_CONF_PROPERTY])
}

@Test
fun `check system property with multiproject build with default config`() {
setupMultiProject()
Expand Down