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

NamedArguments: Ignore when argument values are the same as the parameter name #4613

Merged
merged 2 commits into from Mar 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -129,6 +129,7 @@ complexity:
NamedArguments:
active: false
threshold: 3
ignoreArgumentsMatchingNames: false
NestedBlockDepth:
active: true
threshold: 4
Expand Down
Expand Up @@ -44,6 +44,9 @@ class NamedArguments(config: Config = Config.empty) : Rule(config) {
@Configuration("number of parameters that triggers this inspection")
private val threshold: Int by config(defaultValue = 3)

@Configuration("ignores when argument values are the same as the parameter names")
private val ignoreArgumentsMatchingNames: Boolean by config(defaultValue = false)

override fun visitCallExpression(expression: KtCallExpression) {
if (bindingContext == BindingContext.EMPTY) return
val valueArguments = expression.valueArguments
Expand All @@ -56,12 +59,21 @@ class NamedArguments(config: Config = Config.empty) : Rule(config) {

@Suppress("ReturnCount")
private fun KtCallExpression.canNameArguments(): Boolean {
val unnamedArguments = valueArguments.filterNot { it.isNamed() || it is KtLambdaArgument }
if (unnamedArguments.isEmpty()) return false
val resolvedCall = getResolvedCall(bindingContext) ?: return false
if (!resolvedCall.candidateDescriptor.hasStableParameterNames()) return false
return unnamedArguments.all {
resolvedCall.getParameterForArgument(it)?.varargElementType == null || it.getSpreadElement() != null

val unnamedArguments = valueArguments.mapNotNull { argument ->
if (argument.isNamed() || argument is KtLambdaArgument) return@mapNotNull null
val parameter = resolvedCall.getParameterForArgument(argument) ?: return@mapNotNull null
if (ignoreArgumentsMatchingNames &&
parameter.name.asString() == argument.getArgumentExpression()?.text
) return@mapNotNull null
argument to parameter
}
if (unnamedArguments.isEmpty()) return false

return unnamedArguments.all { (argument, parameter) ->
argument.getSpreadElement() != null || parameter.varargElementType == null
}
}
}
Expand Up @@ -200,5 +200,53 @@ class NamedArgumentsSpec(val env: KotlinCoreEnvironment) {
assertThat(findings).hasSize(1)
}
}

@Nested
inner class IgnoreArgumentsMatchingNames {
@Nested
inner class `ignoreArgumentsMatchingNames is true` {
val subject =
NamedArguments(TestConfig(mapOf("threshold" to 2, "ignoreArgumentsMatchingNames" to true)))

@Test
fun `all arguments are the same as the parameter names`() {
val code = """
fun foo(a: Int, b: Int, c: Int) {}
fun bar(a: Int, b: Int, c: Int) {
foo(a, b, c)
}
"""
val findings = subject.compileAndLintWithContext(env, code)
assertThat(findings).hasSize(0)
}

@Test
fun `some arguments are not the same as the parameter names`() {
val code = """
fun foo(a: Int, b: Int, c: Int) {}
fun bar(a: Int, b: Int, c: Int) {
foo(a, c, b)
}
"""
val findings = subject.compileAndLintWithContext(env, code)
assertThat(findings).hasSize(1)
}
}

@Nested
inner class `ignoreArgumentsMatchingNames is false` {
@Test
fun `all arguments are the same as parameter names`() {
val code = """
fun foo(a: Int, b: Int, c: Int) {}
fun bar(a: Int, b: Int, c: Int) {
foo(a, b, c)
}
"""
val findings = subject.compileAndLintWithContext(env, code)
assertThat(findings).hasSize(1)
}
}
}
}
}