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

Introduce --max-issues flag for cli - #2267 #3391

Merged
merged 5 commits into from
Jan 24, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,13 @@ class CliArgs {
)
var allRules: Boolean = false

@Parameter(
names = ["--max-issues"],
description = "Return exit code 0 only when found issues count does not exceed specified issues count."
)
// nullable for 1.x.x to prefer maxIssues from config file
var maxIssues: Int? = null

@Parameter(
names = ["--auto-correct", "-ac"],
description = "Allow rules to auto correct code if they support it. " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@ internal fun CliArgs.createSpec(output: Appendable, error: Appendable): Processi
autoCorrect = args.autoCorrect
@Suppress("DEPRECATION")
activateAllRules = args.failFast || args.allRules
maxIssuePolicy = RulesSpec.MaxIssuePolicy.NonSpecified // not yet supported; prefer to read from config
maxIssuePolicy = when (val count = args.maxIssues) {
null -> RulesSpec.MaxIssuePolicy.NonSpecified // prefer to read from config
0 -> RulesSpec.MaxIssuePolicy.NoneAllowed
in -1 downTo Int.MIN_VALUE -> RulesSpec.MaxIssuePolicy.AllowAny
else -> RulesSpec.MaxIssuePolicy.AllowAmount(count)
chao2zhang marked this conversation as resolved.
Show resolved Hide resolved
}
excludeCorrectable = false // not yet supported; loaded from config
runPolicy = args.toRunPolicy()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@ import io.gitlab.arturbosch.detekt.cli.runners.Runner
fun CliArgs.toSpec() = createSpec(NullPrintStream(), NullPrintStream())

fun createRunner(cliArgs: CliArgs): Runner = Runner(cliArgs, NullPrintStream(), NullPrintStream())

fun executeDetekt(vararg args: String) {
val cli = parseArguments(args)
createRunner(cli).execute()
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ package io.gitlab.arturbosch.detekt.cli.runners

import io.github.detekt.test.utils.StringPrintStream
import io.github.detekt.test.utils.createTempFileForTest
import io.github.detekt.test.utils.resource
import io.github.detekt.test.utils.resourceAsPath
import io.github.detekt.tooling.api.InvalidConfig
import io.github.detekt.tooling.api.MaxIssuesReached
import io.gitlab.arturbosch.detekt.cli.createRunner
import io.gitlab.arturbosch.detekt.cli.executeDetekt
import io.gitlab.arturbosch.detekt.cli.parseArguments
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatCode
Expand All @@ -15,7 +14,6 @@ import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths

class RunnerSpec : Spek({

Expand All @@ -25,33 +23,33 @@ class RunnerSpec : Spek({

it("should report one issue when maxIssues=2") {
val tmpReport = createTempFileForTest("RunnerSpec", ".txt")
val cliArgs = parseArguments(arrayOf("--input", inputPath.toString(),

executeDetekt(
"--input", inputPath.toString(),
"--report", "txt:$tmpReport",
"--config-resource", "/configs/max-issues-2.yml"
))

createRunner(cliArgs).execute()
)

assertThat(Files.readAllLines(tmpReport)).hasSize(1)
}

it("should throw on maxIssues=0") {
val cliArgs = parseArguments(arrayOf("--input", inputPath.toString(),
"--config-resource", "/configs/max-issues-0.yml"
))

assertThatThrownBy { createRunner(cliArgs).execute() }
.isExactlyInstanceOf(MaxIssuesReached::class.java)
assertThatThrownBy {
executeDetekt(
"--input", inputPath.toString(),
"--config-resource", "/configs/max-issues-0.yml"
)
}.isExactlyInstanceOf(MaxIssuesReached::class.java)
}

it("should never throw on maxIssues=-1") {
val tmpReport = createTempFileForTest("RunnerSpec", ".txt")
val cliArgs = parseArguments(arrayOf("--input", inputPath.toString(),

executeDetekt(
"--input", inputPath.toString(),
"--report", "txt:$tmpReport",
"--config-resource", "/configs/max-issues--1.yml"
))

createRunner(cliArgs).execute()
)

assertThat(Files.readAllLines(tmpReport)).hasSize(1)
}
Expand All @@ -60,13 +58,13 @@ class RunnerSpec : Spek({

it("should not throw on maxIssues=0 due to baseline") {
val tmpReport = createTempFileForTest("RunnerSpec", ".txt")
val cliArgs = parseArguments(arrayOf("--input", inputPath.toString(),

executeDetekt(
"--input", inputPath.toString(),
"--report", "txt:$tmpReport",
"--config-resource", "/configs/max-issues-0.yml",
"--baseline", resourceAsPath("configs/baseline-with-two-excludes.xml").toString()
))

createRunner(cliArgs).execute()
)

assertThat(Files.readAllLines(tmpReport)).isEmpty()
}
Expand All @@ -77,14 +75,14 @@ class RunnerSpec : Spek({

it("should not throw on maxIssues=0") {
val tmpReport = createTempFileForTest("RunnerSpec", ".txt")
val cliArgs = parseArguments(arrayOf("--input", inputPath.toString(),
"--baseline", Paths.get(resource("configs/baseline-empty.xml")).toString(),

executeDetekt(
"--input", inputPath.toString(),
"--baseline", resourceAsPath("configs/baseline-empty.xml").toString(),
"--create-baseline",
"--report", "txt:$tmpReport",
"--config-resource", "/configs/max-issues-0.yml"
))

createRunner(cliArgs).execute()
)

assertThat(tmpReport).hasContent("")
}
Expand Down Expand Up @@ -117,13 +115,15 @@ class RunnerSpec : Spek({
context("execute with strict config") {

beforeEachTest {
val args = parseArguments(arrayOf("--input", inputPath.toString(),
"--config-resource", "/configs/max-issues-0.yml"))

try {
Runner(args, outPrintStream, errPrintStream).execute()
} catch (ignored: MaxIssuesReached) {
}
val args = parseArguments(
arrayOf(
"--input", inputPath.toString(),
"--config-resource", "/configs/max-issues-0.yml"
)
)

assertThatThrownBy { Runner(args, outPrintStream, errPrintStream).execute() }
.isExactlyInstanceOf(MaxIssuesReached::class.java)
}

it("writes output to output printer") {
Expand All @@ -141,73 +141,101 @@ class RunnerSpec : Spek({
val path: Path = resourceAsPath("/cases/CleanPoko.kt")

it("should throw on invalid config property when validation=true") {
val cliArgs = parseArguments(arrayOf("--input", path.toString(),
"--config-resource", "/configs/invalid-config.yml"
))

assertThatThrownBy { createRunner(cliArgs).execute() }
.isExactlyInstanceOf(InvalidConfig::class.java)
assertThatThrownBy {
executeDetekt(
"--input", path.toString(),
"--config-resource", "/configs/invalid-config.yml"
)
}.isExactlyInstanceOf(InvalidConfig::class.java)
.hasMessageContaining("property")
}

it("should throw on invalid config properties when validation=true") {
val cliArgs = parseArguments(arrayOf("--input", path.toString(),
"--config-resource", "/configs/invalid-configs.yml"
))

assertThatThrownBy { createRunner(cliArgs).execute() }
.isExactlyInstanceOf(InvalidConfig::class.java)
assertThatThrownBy {
executeDetekt(
"--input", path.toString(),
"--config-resource", "/configs/invalid-configs.yml"
)
}.isExactlyInstanceOf(InvalidConfig::class.java)
.hasMessageContaining("properties")
}

it("should not throw on invalid config property when validation=false") {
val cliArgs = parseArguments(arrayOf("--input", path.toString(),
"--config-resource", "/configs/invalid-config_no-validation.yml"
))

assertThatCode { createRunner(cliArgs).execute() }.doesNotThrowAnyException()
assertThatCode {
executeDetekt(
"--input", path.toString(),
"--config-resource", "/configs/invalid-config_no-validation.yml"
)
}.doesNotThrowAnyException()
}

it("should not throw on deprecation warnings") {
val cliArgs = parseArguments(arrayOf("--input", path.toString(),
"--config-resource", "/configs/deprecated-property.yml"
))

assertThatCode { createRunner(cliArgs).execute() }.doesNotThrowAnyException()
assertThatCode {
executeDetekt(
"--input", path.toString(),
"--config-resource", "/configs/deprecated-property.yml"
)
}.doesNotThrowAnyException()
}
}

describe("executes the runner for a single rule") {

it("should load and run custom rule") {
val tmp = createTempFileForTest("SingleRuleRunnerSpec", ".txt")
val args = parseArguments(arrayOf("--input", inputPath.toString(),
"--report", "txt:$tmp",
"--run-rule", "test:test"
))

runCatching { createRunner(args).execute() }

assertThatThrownBy {
executeDetekt(
"--input", inputPath.toString(),
"--report", "txt:$tmp",
"--run-rule", "test:test"
)
}.isExactlyInstanceOf(MaxIssuesReached::class.java)
assertThat(Files.readAllLines(tmp)).hasSize(1)
}

it("should throw on non existing rule") {
val args = parseArguments(arrayOf("--run-rule", "test:non_existing"))
assertThatThrownBy { createRunner(args).execute() }
assertThatThrownBy { executeDetekt("--run-rule", "test:non_existing") }
.isExactlyInstanceOf(IllegalArgumentException::class.java)
}

it("should throw on non existing rule set") {
val args = parseArguments(arrayOf("--run-rule", "non_existing:test"))
assertThatThrownBy { createRunner(args).execute() }
assertThatThrownBy { executeDetekt("--run-rule", "non_existing:test") }
.isExactlyInstanceOf(IllegalArgumentException::class.java)
}

it("should throw on non existing run-rule") {
val args = parseArguments(arrayOf("--run-rule", ""))
assertThatThrownBy { createRunner(args).execute() }
assertThatThrownBy { executeDetekt("--run-rule", "") }
.isExactlyInstanceOf(IllegalArgumentException::class.java)
.withFailMessage("Unexpected empty 'runRule' argument.")
.hasMessage("Pattern 'RuleSetId:RuleId' expected.")
}
}

describe("runner with maxIssuePolicy") {

it("does fail via cli flag") {
assertThatThrownBy { executeDetekt("--input", inputPath.toString(), "--max-issues", "0") }
.isExactlyInstanceOf(MaxIssuesReached::class.java)
.hasMessage("Build failed with 1 weighted issues.")
}

it("does fail via cli flag even if config>maxIssues is specified") {
assertThatThrownBy {
executeDetekt(
"--input", inputPath.toString(),
"--max-issues", "0",
"--config-resource", "configs/max-issues--1.yml" // allow any
)
}.isExactlyInstanceOf(MaxIssuesReached::class.java)
.hasMessage("Build failed with 1 weighted issues.")
}

it("does not fail when cli flag is negative") {
executeDetekt("--input", inputPath.toString(), "--max-issues", "-1")
}

it("does not fail when cli flag is positive") {
executeDetekt("--input", inputPath.toString(), "--max-issues", "2")
}
}
})
4 changes: 4 additions & 0 deletions docs/pages/gettingstarted/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ Usage: detekt [options]
Default: false
--excludes, -ex
Globing patterns describing paths to exclude from the analysis.
--max-issues
DEPRECATED: please use '--build-upon-default-config' together with '--all-rules'.
chao2zhang marked this conversation as resolved.
Show resolved Hide resolved
Return exit code 0 only when found issues count does not exceed specified issues count.
config>maxIssues will be ignored if this option is provided.
--fail-fast
Same as 'build-upon-default-config' but explicitly running all available
rules. With this setting only exit code 0 is returned when the analysis
Expand Down