Rename Blacklist and Whitelist to be self explanatory (#2778)

* Rename Blacklist and Whitelist to be self explanatory

SuppressedFalsePositive and TemporarySuppressedIssue describe better what each list is meant for and are more culturally appropriate.

* Add backward compatibility for deprecated baseline attributes

* Use better names for tags that replace blacklist and whitelist

* Define constants for Blacklist and Whitelist

This was done to make it more clear why they still exist in code

* Fix documentation after replacing whitelist terminology

* Move documentation to WildcardRule so generator does not complain

Co-authored-by: Artur Bosch <arturbosch@gmx.de>
This commit is contained in:
Remco Mokveld
2020-06-11 12:49:34 +02:00
committed by GitHub
parent d207bd3217
commit 1e712757bb
21 changed files with 101 additions and 65 deletions

View File

@@ -23,7 +23,7 @@ It operates on the abstract syntax tree provided by the Kotlin compiler.
- Highly configurable rule sets
- Suppression of findings with Kotlin's `@Suppress` and Java's `@SuppressWarnings` annotations
- Specification of quality gates which will break your build
- Code Smell baseline and whitelisting for legacy projects
- Code Smell baseline and suppression for legacy projects
- [Gradle plugin](#with-gradle) for code analysis via Gradle builds
- Gradle tasks to use local `IntelliJ` distribution for formatting and inspecting Kotlin code
- [SonarQube integration](https://github.com/detekt/sonar-kotlin)
@@ -208,6 +208,7 @@ If you contributed to detekt but your name is not in the list, please feel free
- [Natig Babayev](https://github.com/natiginfo) - Readme improvements
- [David Phillips](https://github.com/daphil19) - New rule: MandatoryBracesLoops
- [Volkan Şahin](https://github.com/volsahin) - Documentation improvement
- [Remco Mokveld](https://github.com/remcomokveld) - Rename Blacklist/Whitelist to more meaningful names
### Mentions

View File

@@ -1,7 +1,7 @@
<?xml version="1.0" ?>
<SmellBaseline>
<Blacklist></Blacklist>
<Whitelist>
<ManuallySuppressedIssues/>
<CurrentIssues>
<ID>LargeClass:UnusedPrivateMemberSpec.kt$UnusedPrivateMemberSpec : Spek</ID>
</Whitelist>
</CurrentIssues>
</SmellBaseline>

View File

@@ -61,7 +61,7 @@ class RunnerSpec : Spek({
context("with additional baseline file") {
it("should not throw on maxIssues=0 due to baseline blacklist") {
it("should not throw on maxIssues=0 due to baseline") {
val tmpReport = createTempFileForTest("RunnerSpec", ".txt")
val cliArgs = createCliArgs(
"--input", inputPath.toString(),

View File

@@ -1,5 +1,5 @@
<?xml version="1.0" ?>
<SmellBaseline>
<Blacklist />
<Whitelist />
<ManuallySuppressedIssues />
<CurrentIssues />
</SmellBaseline>

View File

@@ -1,7 +1,7 @@
<?xml version="1.0" ?>
<SmellBaseline>
<Blacklist />
<Whitelist>
<ManuallySuppressedIssues />
<CurrentIssues>
<ID>test:Poko.kt$Poko</ID>
</Whitelist>
</CurrentIssues>
</SmellBaseline>

View File

@@ -8,9 +8,13 @@ import java.nio.file.Path
internal typealias FindingsIdList = Set<String>
internal typealias FindingId = String
internal data class Baseline(val blacklist: FindingsIdList, val whitelist: FindingsIdList) {
internal data class Baseline(
val manuallySuppressedIssues: FindingsIdList,
val currentIssues: FindingsIdList
) {
fun contains(id: FindingId): Boolean = whitelist.contains(id) || blacklist.contains(id)
fun contains(id: FindingId): Boolean =
currentIssues.contains(id) || manuallySuppressedIssues.contains(id)
companion object {
@@ -26,8 +30,8 @@ const val DETEKT_BASELINE_PATH_KEY = "detekt.baseline.path.key"
const val DETEKT_BASELINE_CREATION_KEY = "detekt.baseline.creation.key"
internal const val SMELL_BASELINE = "SmellBaseline"
internal const val BLACKLIST = "Blacklist"
internal const val WHITELIST = "Whitelist"
internal const val MANUALLY_SUPPRESSED_ISSUES = "ManuallySuppressedIssues"
internal const val CURRENT_ISSUES = "CurrentIssues"
internal const val ID = "ID"
internal val Finding.baselineId: String

View File

@@ -15,7 +15,7 @@ class BaselineFacade {
fun createOrUpdate(baselineFile: Path, findings: List<Finding>) {
val ids = findings.map { it.baselineId }.toSortedSet()
val baseline = if (baselineExists(baselineFile)) {
Baseline.load(baselineFile).copy(whitelist = ids)
Baseline.load(baselineFile).copy(currentIssues = ids)
} else {
Baseline(emptySet(), ids)
}

View File

@@ -42,11 +42,11 @@ internal class BaselineFormat {
private fun XMLStreamWriter.save(baseline: Baseline) {
document {
tag(SMELL_BASELINE) {
tag(BLACKLIST) {
baseline.blacklist.forEach { tag(ID, it) }
tag(MANUALLY_SUPPRESSED_ISSUES) {
baseline.manuallySuppressedIssues.forEach { tag(ID, it) }
}
tag(WHITELIST) {
baseline.whitelist.forEach { tag(ID, it) }
tag(CURRENT_ISSUES) {
baseline.currentIssues.forEach { tag(ID, it) }
}
}
}

View File

@@ -1,5 +1,6 @@
package io.gitlab.arturbosch.detekt.core.baseline
import io.gitlab.arturbosch.detekt.core.NotApiButProbablyUsedByUsers
import org.xml.sax.Attributes
import org.xml.sax.helpers.DefaultHandler
@@ -7,18 +8,21 @@ internal class BaselineHandler : DefaultHandler() {
private var current: String? = null
private var content: String = ""
private val whiteIds = mutableSetOf<String>()
private val blackIds = mutableSetOf<String>()
private val currentIssues = mutableSetOf<String>()
private val manuallySuppressedIssues = mutableSetOf<String>()
internal fun createBaseline() = Baseline(blackIds, whiteIds)
internal fun createBaseline() = Baseline(manuallySuppressedIssues, currentIssues)
override fun startElement(uri: String, localName: String, qName: String, attributes: Attributes) {
when (qName) {
BLACKLIST -> {
current = BLACKLIST
// Blacklist and Whitelist were previous XML tags. They have been replaced by more appropriate names
// For the removal of these to not be a breaking change these have been implemented to be synonyms
// of [MANUALLY_SUPPRESSED_ISSUES] and [CURRENT_ISSUES].
MANUALLY_SUPPRESSED_ISSUES, BLACKLIST -> {
current = MANUALLY_SUPPRESSED_ISSUES
}
WHITELIST -> {
current = WHITELIST
CURRENT_ISSUES, WHITELIST -> {
current = CURRENT_ISSUES
}
ID -> content = ""
}
@@ -29,16 +33,24 @@ internal class BaselineHandler : DefaultHandler() {
ID -> {
check(content.isNotBlank()) { "The content of the ID element must not be empty" }
when (current) {
BLACKLIST -> blackIds.add(content)
WHITELIST -> whiteIds.add(content)
MANUALLY_SUPPRESSED_ISSUES -> manuallySuppressedIssues.add(content)
CURRENT_ISSUES -> currentIssues.add(content)
}
content = ""
}
BLACKLIST, WHITELIST -> current == null
MANUALLY_SUPPRESSED_ISSUES, CURRENT_ISSUES -> current == null
}
}
override fun characters(ch: CharArray, start: Int, length: Int) {
if (current != null) content += String(ch, start, length)
}
companion object {
@NotApiButProbablyUsedByUsers
private const val BLACKLIST = "Blacklist"
@NotApiButProbablyUsedByUsers
private const val WHITELIST = "Whitelist"
}
}

View File

@@ -17,13 +17,13 @@ class BaselineFormatSpec : Spek({
it("loads the baseline file") {
val path = resourceAsPath("/baseline_feature/valid-baseline.xml")
val (blacklist, whitelist) = BaselineFormat().read(path)
val (manuallySuppressedIssues, currentIssues) = BaselineFormat().read(path)
assertThat(blacklist).hasSize(2)
assertThat(blacklist).anySatisfy { it.startsWith("LongParameterList") }
assertThat(blacklist).anySatisfy { it.startsWith("LongMethod") }
assertThat(whitelist).hasSize(1)
assertThat(whitelist).anySatisfy { it.startsWith("FeatureEnvy") }
assertThat(manuallySuppressedIssues).hasSize(2)
assertThat(manuallySuppressedIssues).anySatisfy { it.startsWith("LongParameterList") }
assertThat(manuallySuppressedIssues).anySatisfy { it.startsWith("LongMethod") }
assertThat(currentIssues).hasSize(1)
assertThat(currentIssues).anySatisfy { it.startsWith("FeatureEnvy") }
}
it("throws on an invalid baseline file extension") {
@@ -33,11 +33,22 @@ class BaselineFormatSpec : Spek({
}
it("throws on an invalid baseline ID declaration") {
val path = resourceAsPath("/baseline_feature/missing-whitelist-baseline.xml")
val path = resourceAsPath("/baseline_feature/missing-temporary-suppressed-baseline.xml")
assertThatIllegalStateException()
.isThrownBy { BaselineFormat().read(path) }
.withMessage("The content of the ID element must not be empty")
}
it("supports deprecated baseline values") {
val path = resourceAsPath("/baseline_feature/deprecated-baseline.xml")
val (manuallySuppressedIssues, currentIssues) = BaselineFormat().read(path)
assertThat(manuallySuppressedIssues).hasSize(2)
assertThat(manuallySuppressedIssues).anySatisfy { it.startsWith("LongParameterList") }
assertThat(manuallySuppressedIssues).anySatisfy { it.startsWith("LongMethod") }
assertThat(currentIssues).hasSize(1)
assertThat(currentIssues).anySatisfy { it.startsWith("FeatureEnvy") }
}
}
context("writes a baseline file") {

View File

@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8" ?>
<SmellBaseline>
<Blacklist>
<ID>LongParameterList:Signature</ID>
<ID>LongMethod:Signature</ID>
</Blacklist>
<Whitelist>
<ID>FeatureEnvy:Signature</ID>
</Whitelist>
</SmellBaseline>

View File

@@ -1,5 +1,5 @@
<?xml version="1.0" ?>
<SmellBaseline>
<Blacklist />
<Whitelist />
<ManuallySuppressedIssues />
<CurrentIssues />
</SmellBaseline>

View File

@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8" ?>
<SmellBaseline>
<Blacklist>
<ManuallySuppressedIssues>
<ID></ID>
</Blacklist>
</ManuallySuppressedIssues>
</SmellBaseline>

View File

@@ -1,10 +1,10 @@
<?xml version="1.0" encoding="UTF-8" ?>
<SmellBaseline>
<Blacklist>
<ManuallySuppressedIssues>
<ID>LongParameterList:Signature</ID>
<ID>LongMethod:Signature</ID>
</Blacklist>
<Whitelist>
</ManuallySuppressedIssues>
<CurrentIssues>
<ID>FeatureEnvy:Signature</ID>
</Whitelist>
</CurrentIssues>
</SmellBaseline>

View File

@@ -13,7 +13,7 @@ import org.jetbrains.kotlin.psi.KtClass
import org.jetbrains.kotlin.psi.KtNamedFunction
/**
* This rule reports functions inside data classes which have not been whitelisted as a conversion function.
* This rule reports functions inside data classes which have not been marked as a conversion function.
*
* Data classes should mainly be used to store data. This rule assumes that they should not contain any extra functions
* aside functions that help with converting objects from/to one another.

View File

@@ -16,9 +16,8 @@ import org.jetbrains.kotlin.psi.KtImportDirective
*
* Library updates can introduce naming clashes with your own classes which might result in compilation errors.
*
* **NOTE:** This rule is effectively overridden by the `NoWildcardImports` formatting rule (a wrapped ktlint rule).
* That rule will fail the check regardless of the whitelist configured here.
* Therefore if whitelist is needed `NoWildcardImports` rule should be disabled.
* **NOTE**: This rule has a twin implementation NoWildcardImports in the formatting rule set (a wrapped KtLint rule).
* When suppressing an issue of WildcardImport in the baseline file, make sure to suppress the corresponding NoWildcardImports issue.
*
* <noncompliant>
* import io.gitlab.arturbosch.detekt.*
@@ -39,7 +38,7 @@ import org.jetbrains.kotlin.psi.KtImportDirective
* }
* </compliant>
*
* @configuration excludeImports - Define a whitelist of package names that should be allowed to be imported
* @configuration excludeImports - Define a list of package names that should be allowed to be imported
* with wildcard imports. (default: `['java.util.*', 'kotlinx.android.synthetic.*']`)
*
* @active since v1.0.0

View File

@@ -19,7 +19,7 @@ programming flaws like unused or too complex constructs. Think of it as *pmd* or
- Highly configurable rule sets
- Suppression of findings with Kotlin's `@Suppress` and Java's `@SuppressWarnings` annotations
- Specification of quality gates which will break your build
- Code Smell baseline and whitelisting for legacy projects
- Code Smell baseline for legacy projects
- [Gradle plugin](#with-gradle) for code analysis via Gradle builds
- Gradle tasks to use local `IntelliJ` distribution for formatting and inspecting Kotlin code
- [SonarQube integration](https://github.com/arturbosch/sonar-kotlin)

View File

@@ -7,10 +7,10 @@ summary:
---
With the cli option `--baseline` or the detekt-gradle-plugin closure-property `baseline` you can specify a file which is used to generate a `baseline.xml`.
It is a file where code smells are white- or blacklisted.
It is a file where ignored code smells are defined.
The intention of a whitelist is that only new code smells are printed on further analysis.
The blacklist can be used to write down false positive detections (instead of suppressing them and pollute your code base).
The intention of `CurrentIssues` is that only new code smells are printed on further analysis.
The `ManuallySuppressedIssues` can be used to write down false positive detections (instead of suppressing them and pollute your code base).
The `ID` node has the following structure: `<RuleID>:<Codesmell_Signature>`.
When adding a custom issue to the xml file, make sure the `RuleID` should be self-explaining.
@@ -19,14 +19,14 @@ the `--report txt:path/to/report` cli flag.
```xml
<SmellBaseline>
<Blacklist>
<ManuallySuppressedIssues>
<ID>CatchRuntimeException:Junk.kt$e: RuntimeException</ID>
</Blacklist>
<Whitelist>
</ManuallySuppressedIssues>
<CurrentIssues>
<ID>NestedBlockDepth:Indentation.kt$Indentation$override fun procedure(node: ASTNode)</ID>
<ID>TooManyFunctions:LargeClass.kt$io.gitlab.arturbosch.detekt.rules.complexity.LargeClass.kt</ID>
<ID>ComplexMethod:DetektExtension.kt$DetektExtension$fun convertToArguments(): MutableList&lt;String&gt;</ID>
</Whitelist>
</CurrentIssues>
</SmellBaseline>
```

View File

@@ -1704,8 +1704,8 @@ See all issues at: [M7](https://gitlab.com/arturbosch/detekt/milestones/7)
##### New features
- allow to fail builds on code smell thresholds (configurable), see 'Configure build failure thresholds' in README
- blacklist code smells which are false positives
- generate a baseline whitelist of code smells if your project is already old and has many smells, so only new
- suppress code smells which are false positives
- generate a baseline of code smells if your project is already old and has many smells, so only new
smells are shown in analysis (see 'Code Smell baseline and ignore list' in README)
##### Improvements

View File

@@ -43,7 +43,7 @@ if (i > 0 && i < 5) {
### DataClassContainsFunctions
This rule reports functions inside data classes which have not been whitelisted as a conversion function.
This rule reports functions inside data classes which have not been marked as a conversion function.
Data classes should mainly be used to store data. This rule assumes that they should not contain any extra functions
aside functions that help with converting objects from/to one another.
@@ -1651,9 +1651,8 @@ which classes are imported and helps prevent naming conflicts.
Library updates can introduce naming clashes with your own classes which might result in compilation errors.
**NOTE:** This rule is effectively overridden by the `NoWildcardImports` formatting rule (a wrapped ktlint rule).
That rule will fail the check regardless of the whitelist configured here.
Therefore if whitelist is needed `NoWildcardImports` rule should be disabled.
**NOTE**: This rule has a twin implementation NoWildcardImports in the formatting rule set (a wrapped KtLint rule).
When suppressing an issue of WildcardImport in the baseline file, make sure to suppress the corresponding NoWildcardImports issue.
**Severity**: Style
@@ -1663,7 +1662,7 @@ Therefore if whitelist is needed `NoWildcardImports` rule should be disabled.
* ``excludeImports`` (default: ``['java.util.*', 'kotlinx.android.synthetic.*']``)
Define a whitelist of package names that should be allowed to be imported
Define a list of package names that should be allowed to be imported
with wildcard imports.
#### Noncompliant Code:

View File

@@ -14,7 +14,7 @@ detekt requires Gradle 5.0 or higher.
The detekt Gradle plugin will generate multiple tasks
- `detekt` - Runs a detekt analysis and complexity report on your source files. Configure the analysis inside the
`detekt` closure. By default the standard rule set without any white- or blacklist is executed on sources files located
`detekt` closure. By default the standard rule set without any ignore list is executed on sources files located
in `src/main/java` and `src/main/kotlin`. Reports are automatically generated in xml, html and txt format and can be
found in `build/reports/detekt/detekt.[xml|html|txt]` respectively. Please note that the `detekt` task is automatically
run when executing `gradle check`.