Implement ignoreAnnotated as a core feature (#4102)

* Add missing tests

* Add AnnotationSuppressor

* Use ignoreAnnotated instead of the custom ones from our rules

* Rename LongParameter.ignoreAnnotated to not clash with the general suppression
This commit is contained in:
Brais Gabín
2021-10-02 10:47:12 +02:00
committed by GitHub
parent ee0ae169f8
commit f12697d700
21 changed files with 410 additions and 181 deletions

View File

@@ -1,7 +1,6 @@
package io.gitlab.arturbosch.detekt.rules.complexity
import io.github.detekt.metrics.linesOfCode
import io.gitlab.arturbosch.detekt.api.AnnotationExcluder
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.Entity
@@ -39,15 +38,10 @@ class LongMethod(config: Config = Config.empty) : Rule(config) {
@Configuration("number of lines in a method to trigger the rule")
private val threshold: Int by config(defaultValue = 60)
@Configuration("ignore long methods in the context of these annotation class names")
private val ignoreAnnotated: List<String> by config(emptyList())
private val functionToLinesCache = HashMap<KtNamedFunction, Int>()
private val functionToBodyLinesCache = HashMap<KtNamedFunction, Int>()
private val nestedFunctionTracking = IdentityHashMap<KtNamedFunction, HashSet<KtNamedFunction>>()
private lateinit var annotationExcluder: AnnotationExcluder
override fun preVisit(root: KtFile) {
functionToLinesCache.clear()
functionToBodyLinesCache.clear()
@@ -77,8 +71,6 @@ class LongMethod(config: Config = Config.empty) : Rule(config) {
}
override fun visitNamedFunction(function: KtNamedFunction) {
if (annotationExcluder.shouldExclude(function.annotationEntries)) return
val parentMethods = function.getStrictParentOfType<KtNamedFunction>()
val bodyEntity = function.bodyBlockExpression ?: function.bodyExpression
val lines = (if (parentMethods != null) function else bodyEntity)?.linesOfCode() ?: 0
@@ -92,11 +84,6 @@ class LongMethod(config: Config = Config.empty) : Rule(config) {
?.let { functionToLinesCache[function] = lines - it }
}
override fun visitKtFile(file: KtFile) {
annotationExcluder = AnnotationExcluder(file, ignoreAnnotated)
super.visitKtFile(file)
}
private fun findAllNestedFunctions(startFunction: KtNamedFunction): Sequence<KtNamedFunction> = sequence {
var nestedFunctions = nestedFunctionTracking[startFunction]
while (!nestedFunctions.isNullOrEmpty()) {

View File

@@ -24,7 +24,6 @@ import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.KtParameterList
import org.jetbrains.kotlin.psi.KtPrimaryConstructor
import org.jetbrains.kotlin.psi.KtSecondaryConstructor
import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
/**
* Reports functions and constructors which have more parameters than a certain threshold.
@@ -61,27 +60,20 @@ class LongParameterList(config: Config = Config.empty) : Rule(config) {
private val ignoreDataClasses: Boolean by config(true)
@Configuration(
"ignore long parameters list for constructors, functions or their parameters in the " +
"context of these annotation class names; (e.g. ['Inject', 'Module', 'Suppress', 'Value']); " +
"the most common cases are for dependency injection where constructors are annotated with `@Inject` " +
"or parameters are annotated with `@Value` and should not be counted for the rule to trigger"
"ignore the annotated parameters for the count (e.g. `fun foo(@Value bar: Int)` would not be counted"
)
private val ignoreAnnotated: List<String> by config(emptyList<String>()) { list ->
private val ignoreAnnotatedParameter: List<String> by config(emptyList<String>()) { list ->
list.map { it.removePrefix("*").removeSuffix("*") }
}
private lateinit var annotationExcluder: AnnotationExcluder
override fun visitKtFile(file: KtFile) {
annotationExcluder = AnnotationExcluder(file, ignoreAnnotated)
annotationExcluder = AnnotationExcluder(file, ignoreAnnotatedParameter)
super.visitKtFile(file)
}
override fun visitNamedFunction(function: KtNamedFunction) {
val owner = function.containingClassOrObject
if (owner is KtClass && owner.isIgnored()) {
return
}
checkLongParameterList(function, functionThreshold, "function ${function.nameAsSafeName}")
}
@@ -105,10 +97,10 @@ class LongParameterList(config: Config = Config.empty) : Rule(config) {
checkLongParameterList(constructor, constructorThreshold, "constructor")
}
private fun KtClass.isDataClassOrIgnored() = isIgnored() || ignoreDataClasses && isData()
private fun KtClass.isDataClassOrIgnored() = ignoreDataClasses && isData()
private fun checkLongParameterList(function: KtFunction, threshold: Int, identifier: String) {
if (function.isOverride() || function.isIgnored() || function.containingKtFile.isIgnored()) return
if (function.isOverride()) return
val parameterList = function.valueParameterList ?: return
val parameterNumber = parameterList.parameterCount()

View File

@@ -1,6 +1,5 @@
package io.gitlab.arturbosch.detekt.rules.complexity
import io.gitlab.arturbosch.detekt.api.SourceLocation
import io.gitlab.arturbosch.detekt.api.ThresholdedCodeSmell
import io.gitlab.arturbosch.detekt.test.TestConfig
import io.gitlab.arturbosch.detekt.test.assertThat
@@ -151,45 +150,4 @@ class LongMethodSpec : Spek({
assertThat(findings[0] as ThresholdedCodeSmell).hasValue(5)
}
}
describe("annotated functions") {
val code = """
annotation class Composable
annotation class TestAnn
fun foo() {
println()
println()
}
@Composable
fun bar() {
println()
println()
}
@TestAnn
fun baz() {
println()
println()
}
"""
it("does not ignore annotated functions if ignoreAnnotated is empty") {
val config = TestConfig(mapOf("threshold" to 2))
assertThat(LongMethod(config).compileAndLint(code)).hasSourceLocations(
SourceLocation(4, 5),
SourceLocation(10, 5),
SourceLocation(16, 5)
)
}
it("ignores annotated functions if ignoreAnnotated includes the given annotation class") {
val config = TestConfig(mapOf("threshold" to 2, "ignoreAnnotated" to listOf("Composable")))
assertThat(LongMethod(config).compileAndLint(code)).hasSourceLocations(
SourceLocation(4, 5),
SourceLocation(16, 5)
)
}
}
})

View File

@@ -99,7 +99,7 @@ class LongParameterListSpec : Spek({
val config by memoized {
TestConfig(
mapOf(
"ignoreAnnotated" to listOf(
"ignoreAnnotatedParameter" to listOf(
"Generated",
"kotlin.Deprecated",
"kotlin.jvm.JvmName",
@@ -113,54 +113,6 @@ class LongParameterListSpec : Spek({
val rule by memoized { LongParameterList(config) }
it("does not report long parameter list for constructors if file is annotated with ignored annotation") {
val code = """
@file:kotlin.jvm.JvmName("test")
class Data(val a: Int)
"""
assertThat(rule.compileAndLint(code)).isEmpty()
}
it("does not report long parameter list for functions if file is annotated with ignored annotation") {
val code = """
@file:kotlin.jvm.JvmName("test")
class Data {
fun foo(a: Int) {}
}
"""
assertThat(rule.compileAndLint(code)).isEmpty()
}
it("does not report long parameter list for constructors if class is annotated with ignored annotation") {
val code = """
annotation class Generated
@Generated class Data(val a: Int)
"""
assertThat(rule.compileAndLint(code)).isEmpty()
}
it("does not report long parameter list for functions if class is annotated with ignored annotation") {
val code = """
annotation class Generated
@Generated class Data {
fun foo(a: Int) {}
}
"""
assertThat(rule.compileAndLint(code)).isEmpty()
}
it("does not report long parameter list for constructors if constructor is annotated with ignored annotation") {
val code = "class Data @kotlin.Deprecated(message = \"\") constructor(val a: Int)"
assertThat(rule.compileAndLint(code)).isEmpty()
}
it("does not report long parameter list for functions if function is annotated with ignored annotation") {
val code = """class Data {
@kotlin.Deprecated(message = "") fun foo(a: Int, b: Int, c: Int, d: Int, e: Int, f: Int, g: Int) {} }
"""
assertThat(rule.compileAndLint(code)).isEmpty()
}
it("reports long parameter list for constructors if constructor parameters are annotated with annotation that is not ignored") {
val code = """
@Target(AnnotationTarget.VALUE_PARAMETER)