Skip to content

Commit

Permalink
NamedArguments: fix false positive with varargs (#3294)
Browse files Browse the repository at this point in the history
  • Loading branch information
t-kameyama committed Dec 18, 2020
1 parent 3409f39 commit 37f0a1d
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.ThresholdRule
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.calls.callUtil.getParameterForArgument
import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall

/**
Expand Down Expand Up @@ -43,16 +44,22 @@ class NamedArguments(
override fun visitCallExpression(expression: KtCallExpression) {
if (bindingContext == BindingContext.EMPTY) return
val valueArguments = expression.valueArguments
if (valueArguments.size > threshold &&
valueArguments.any { !it.isNamed() } &&
expression.getResolvedCall(bindingContext)?.candidateDescriptor?.hasStableParameterNames() == true
) {
if (valueArguments.size > threshold && expression.canNameArguments()) {
report(CodeSmell(issue, Entity.from(expression), issue.description))
} else {
super.visitCallExpression(expression)
}
}

private fun KtCallExpression.canNameArguments(): Boolean {
val resolvedCall = getResolvedCall(bindingContext) ?: return false
if (!resolvedCall.candidateDescriptor.hasStableParameterNames()) return false
val unnamedArguments = valueArguments.filter { !it.isNamed() }
return unnamedArguments.isNotEmpty() && unnamedArguments.all {
resolvedCall.getParameterForArgument(it)?.varargElementType == null || it.getSpreadElement() != null
}
}

companion object {
const val DEFAULT_FUNCTION_THRESHOLD = 3
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,38 @@ class NamedArgumentsSpec : Spek({

it("java method invocation should not be flagged") {
val code = """
import java.time.LocalDateTime
fun test() {
LocalDateTime.of(2020, 3, 13, 14, 0, 0)
}
import java.time.LocalDateTime
fun test() {
LocalDateTime.of(2020, 3, 13, 14, 0, 0)
}
"""
val findings = namedArguments.compileAndLintWithContext(env, code)
assertThat(findings).hasSize(0)
}

it("invocation with varargs should not be flagged") {
val code = """
fun foo(vararg i: Int) {}
fun bar(a: Int, b: Int, c: Int, vararg s: String) {}
fun test() {
foo(1, 2, 3, 4, 5)
bar(1, 2, 3, "a")
}
"""
val findings = namedArguments.compileAndLintWithContext(env, code)
assertThat(findings).hasSize(0)
}

it("invocation with spread operator should be flagged") {
val code = """
fun bar(a: Int, b: Int, c: Int, vararg s: String) {}
fun test() {
bar(1, 2, 3, *arrayOf("a"))
}
"""
val findings = namedArguments.compileAndLintWithContext(env, code)
assertThat(findings).hasSize(1)
}
}
})

0 comments on commit 37f0a1d

Please sign in to comment.