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

Add UnnecessaryBracesAroundTrailingLambda rule #6029

Merged
merged 3 commits into from
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,8 @@ style:
active: true
UnnecessaryBackticks:
active: false
UnnecessaryBracesAroundTrailingLambda:
active: false
UnnecessaryFilter:
active: true
UnnecessaryInheritance:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
import io.gitlab.arturbosch.detekt.api.internal.DefaultRuleSetProvider
import io.gitlab.arturbosch.detekt.rules.style.movelambdaout.UnnecessaryBracesAroundTrailingLambda
import io.gitlab.arturbosch.detekt.rules.style.optional.MandatoryBracesLoops
import io.gitlab.arturbosch.detekt.rules.style.optional.OptionalUnit
import io.gitlab.arturbosch.detekt.rules.style.optional.PreferToOverPairSyntax
Expand Down Expand Up @@ -73,6 +74,7 @@ class StyleGuideProvider : DefaultRuleSetProvider {
RedundantVisibilityModifierRule(config),
UntilInsteadOfRangeTo(config),
UnnecessaryApply(config),
UnnecessaryBracesAroundTrailingLambda(config),
UnnecessaryFilter(config),
UnnecessaryLet(config),
MayBeConst(config),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package io.gitlab.arturbosch.detekt.rules.style.movelambdaout

import org.jetbrains.kotlin.builtins.isFunctionOrSuspendFunctionType
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtCallableReferenceExpression
import org.jetbrains.kotlin.psi.KtDelegatedSuperTypeEntry
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtLabeledExpression
import org.jetbrains.kotlin.psi.KtLambdaExpression
import org.jetbrains.kotlin.psi.KtNameReferenceExpression
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
import org.jetbrains.kotlin.psi.unpackFunctionLiteral
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.types.typeUtil.isTypeParameter

// source from https://github.com/JetBrains/intellij-community/blob/88e23175cefa446adb4aa64dda8096112a37e2f8/plugins/kotlin/core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt
@Suppress("ReturnCount", "CyclomaticComplexMethod")
private fun KtCallExpression.canMoveLambdaOutsideParentheses(bindingContext: BindingContext): Boolean {
if (isEligible().not()) return false
if (getStrictParentOfType<KtDelegatedSuperTypeEntry>() != null) return false
val lastLambdaExpression = getLastLambdaExpression() ?: kotlin.run {
return false
}
if (lastLambdaExpression.parentLabeledExpression()?.parentLabeledExpression() != null) return false

val callee = calleeExpression
if (callee is KtNameReferenceExpression) {
val lambdaArgumentCount =
valueArguments.mapNotNull { it.getArgumentExpression()?.unpackFunctionLiteral() }.count()
val referenceArgumentCount =
valueArguments.count { it.getArgumentExpression() is KtCallableReferenceExpression }
val targets = bindingContext[BindingContext.REFERENCE_TARGET, callee]?.let { listOf(it) }
?: bindingContext[BindingContext.AMBIGUOUS_REFERENCE_TARGET, callee]
?: emptyList()

Check warning on line 35 in detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/movelambdaout/MoveLambdaOutsideParenthesesInspection.kt

View check run for this annotation

Codecov / codecov/patch

detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/movelambdaout/MoveLambdaOutsideParenthesesInspection.kt#L35

Added line #L35 was not covered by tests
val candidates = targets.filterIsInstance<FunctionDescriptor>()
// if there are functions among candidates but none of them have last function parameter then not show
// the intention
@Suppress("ComplexCondition")
if (
candidates.isNotEmpty() && candidates.none { candidate ->
val params = candidate.valueParameters
val lastParamType = params.lastOrNull()?.type
(
lastParamType?.isFunctionOrSuspendFunctionType == true ||
lastParamType?.isTypeParameter() == true
) && params.count {
it.type.let { type -> type.isFunctionOrSuspendFunctionType || type.isTypeParameter() }
} == lambdaArgumentCount + referenceArgumentCount
}
) {
return false
}
}
return true
}

private fun KtCallExpression.isEligible(): Boolean {
return when {
valueArguments.lastOrNull()?.isNamed() == true -> false
valueArguments.count { it.getArgumentExpression()?.unpackFunctionLiteral() != null } > 1 -> false
else -> true
}
}

internal fun shouldReportUnnecessaryBracesAroundTrailingLambda(
bindingContext: BindingContext,
element: KtCallExpression,
) =
element.canMoveLambdaOutsideParentheses(bindingContext)

private fun KtCallExpression.getLastLambdaExpression(): KtLambdaExpression? {
if (lambdaArguments.isNotEmpty()) return null
return valueArguments.lastOrNull()?.getArgumentExpression()?.unpackFunctionLiteral()
}

private fun KtExpression.parentLabeledExpression(): KtLabeledExpression? {
return getStrictParentOfType<KtLabeledExpression>()?.takeIf { it.baseExpression == this }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package io.gitlab.arturbosch.detekt.rules.style.movelambdaout

import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtNameReferenceExpression

/**
* In Kotlin functions the last lambda parameter of a function is a function then a lambda expression passed as the
* corresponding argument can be placed outside the parentheses.
* see [Passing trailing lambdas](https://kotlinlang.org/docs/lambdas.html#passing-trailing-lambdas).
* Prefer the usage of trailing lambda.
* <noncompliant>
* fun test() {
* repeat(10, {
* println(it)
* })
* }
* </noncompliant>
*
* <compliant>
* fun test() {
* repeat(10) {
* println(it)
* }
* }
* </compliant>
*/
atulgpt marked this conversation as resolved.
Show resolved Hide resolved
@RequiresTypeResolution
class UnnecessaryBracesAroundTrailingLambda(config: Config = Config.empty) : Rule(config) {
override val issue: Issue = Issue(
javaClass.simpleName,
Severity.Style,
"Braces around trailing lambda is unnecessary.",
Debt.FIVE_MINS
)

override fun visitCallExpression(expression: KtCallExpression) {
super.visitCallExpression(expression)
if (shouldReportUnnecessaryBracesAroundTrailingLambda(bindingContext, expression)) {
report(
CodeSmell(
issue,
Entity.from(getIssueElement(expression)),
"Braces around trailing lambda can be removed."
)
)
}
}

private fun getIssueElement(expression: KtCallExpression): PsiElement {
return (expression.calleeExpression as? KtNameReferenceExpression) ?: expression
}
}