Add ignoreAnnotated option to LongParameterList #2570
Conversation
Resolves detekt#2563 This adds a new `ignoreAnnotated` option to `LongParameterList`, that allows for ignoring annotations within function/class/file scope.
Codecov Report
@@ Coverage Diff @@
## master #2570 +/- ##
============================================
+ Coverage 25.11% 25.13% +0.02%
Complexity 392 392
============================================
Files 378 377 -1
Lines 7391 7388 -3
Branches 1221 1220 -1
============================================
+ Hits 1856 1857 +1
+ Misses 5404 5400 -4
Partials 131 131
Continue to review full report at Codecov.
|
| } | ||
| } | ||
|
|
||
| private fun KtFile.isIgnored(): Boolean { |
There was a problem hiding this comment.
These three nodes have KtAnnotated as a common super type.
Please refactor to something like:
private fun KtAnnotated.isIgnored(): Boolean =
annotationEntries.any { ignoreAnnotated.contains(it.typeReference?.text) }
detekt-rules/build.gradle.kts
Outdated
| @@ -4,4 +4,7 @@ dependencies { | |||
| implementation(project(":detekt-api")) | |||
|
|
|||
| testImplementation(project(":detekt-test")) | |||
|
|
|||
| // For @javax.inject.Inject usage in LongParameterListSpec.kt | |||
| testImplementation("javax.inject:javax.inject:1") | |||
There was a problem hiding this comment.
This dependency is actually not needed for the parser as we do not resolve any types. Please make sure the tests run without it.
There was a problem hiding this comment.
The CI checks fail without this, see the failure on the commit before I added it
There was a problem hiding this comment.
If we use compileAndLint we need it in the classpath. The other option is to declare that annotation in every code snipet.
There was a problem hiding this comment.
Can we use just a custom annotation or any already present in the jdk? Deprecated for example.
The test case does not necessary need javax.inject :)
| @@ -26,6 +29,7 @@ import org.jetbrains.kotlin.psi.KtSecondaryConstructor | |||
| * @configuration constructorThreshold - number of constructor parameters required to trigger the rule (default: `7`) | |||
| * @configuration ignoreDefaultParameters - ignore parameters that have a default value (default: `false`) | |||
| * @configuration ignoreDataClasses - ignore long constructor parameters list for data classes (default: `true`) | |||
| * @configuration ignoreAnnotated - ignore long parameters list for constructors or functions in the context of these comma-separated annotation class names (default: `''`) | |||
There was a problem hiding this comment.
@BraisGabin I'm not show whether we should use the yaml list syntax for this configuration option?
There was a problem hiding this comment.
I was matching what some others that used fully qualified names do, and this allows for some wildcard matching. Unclear if using the actual yml list format would support this
There was a problem hiding this comment.
I would go with string for now. For consistency. I'll change all at once. And, to give a bit of context, we are talking about #2498
detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongParameterList.kt
Outdated
Show resolved
Hide resolved
...-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/LongParameterListSpec.kt
Show resolved
Hide resolved
Co-Authored-By: M Schalk <30376729+schalkms@users.noreply.github.com>
Resolves #2563
This adds a new
ignoreAnnotatedoption toLongParameterList, that allows for ignoring annotations within function/class/file scope.