jlengrand/detekt
1
0
Fork 0
mirror of https://github.com/jlengrand/detekt.git synced 2026-03-10 08:11:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

Introduce UseAnyOrNoneInsteadOfFind rule (#4247)

Browse Source 
* Introduce UseAnyOrNoneInsteadOfFind rule

* Also report for `lastOrNull`

* Add a test that just calls `find`

* Import `org.assertj.core.api.Assertions.assertThat`
This commit is contained in:
Toshiaki Kameyama
2021-11-10 17:03:54 +09:00
committed by GitHub
parent 8c1d992709
commit dffde1270a
7 changed files with 180 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
detekt-core/src/main/resources/default-detekt-config.yml
View File

@@ -764,6 +764,8 @@ style:
UnusedPrivateMember:
active: true
allowedNames: '(_|ignored|expected|serialVersionUID)'
UseAnyOrNoneInsteadOfFind:
active: false
UseArrayLiteralsInAnnotations:
active: false
UseCheckNotNull:

3
detekt-psi-utils/api/detekt-psi-utils.api
View File

@@ -76,10 +76,13 @@ public final class io/gitlab/arturbosch/detekt/rules/KtAnnotatedExtensionsKt {
public final class io/gitlab/arturbosch/detekt/rules/KtBinaryExpressionKt {
public static final fun isNonNullCheck (Lorg/jetbrains/kotlin/psi/KtBinaryExpression;)Z
public static final fun isNullCheck (Lorg/jetbrains/kotlin/psi/KtBinaryExpression;)Z
}
public final class io/gitlab/arturbosch/detekt/rules/KtCallExpressionKt {
public static final fun isCalling (Lorg/jetbrains/kotlin/psi/KtCallExpression;Ljava/util/List;Lorg/jetbrains/kotlin/resolve/BindingContext;)Z
public static final fun isCalling (Lorg/jetbrains/kotlin/psi/KtCallExpression;Lorg/jetbrains/kotlin/name/FqName;Lorg/jetbrains/kotlin/resolve/BindingContext;)Z
public static final fun isCalling (Lorg/jetbrains/kotlin/resolve/calls/model/ResolvedCall;Lorg/jetbrains/kotlin/name/FqName;)Z
public static final fun isCallingWithNonNullCheckArgument (Lorg/jetbrains/kotlin/psi/KtCallExpression;Lorg/jetbrains/kotlin/name/FqName;Lorg/jetbrains/kotlin/resolve/BindingContext;)Z
}

4
detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtBinaryExpression.kt
View File

@@ -6,3 +6,7 @@ import org.jetbrains.kotlin.psi.KtBinaryExpression
fun KtBinaryExpression.isNonNullCheck(): Boolean {
return operationToken == KtTokens.EXCLEQ && (left?.text == "null" || right?.text == "null")
}
fun KtBinaryExpression.isNullCheck(): Boolean {
return operationToken == KtTokens.EQEQ && (left?.text == "null" || right?.text == "null")
}

16
detekt-psi-utils/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/KtCallExpression.kt
View File

@@ -1,18 +1,32 @@
package io.gitlab.arturbosch.detekt.rules
import org.jetbrains.kotlin.descriptors.CallableDescriptor
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.psi.KtBinaryExpression
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
fun KtCallExpression.isCalling(fqName: FqName, bindingContext: BindingContext): Boolean {
return bindingContext != BindingContext.EMPTY &&
calleeExpression?.text == fqName.shortName().asString() &&
getResolvedCall(bindingContext)?.resultingDescriptor?.fqNameSafe == fqName
getResolvedCall(bindingContext)?.isCalling(fqName) == true
}
@Suppress("ReturnCount")
fun KtCallExpression.isCalling(fqNames: List<FqName>, bindingContext: BindingContext): Boolean {
if (bindingContext == BindingContext.EMPTY) return false
val calleeText = calleeExpression?.text ?: return false
val targetFqNames = fqNames.filter { it.shortName().asString() == calleeText }
if (targetFqNames.isEmpty()) return false
val resolvedCall = getResolvedCall(bindingContext) ?: return false
return targetFqNames.any { resolvedCall.isCalling(it) }
}
fun ResolvedCall<out CallableDescriptor>.isCalling(fqName: FqName): Boolean = resultingDescriptor.fqNameSafe == fqName
fun KtCallExpression.isCallingWithNonNullCheckArgument(
fqName: FqName,
bindingContext: BindingContext

3
detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/StyleGuideProvider.kt
View File

@@ -91,7 +91,8 @@ class StyleGuideProvider : DefaultRuleSetProvider {
UseIfEmptyOrIfBlank(config),
MultilineLambdaItParameter(config),
UseIsNullOrEmpty(config),
UseOrEmpty(config)
UseOrEmpty(config),
UseAnyOrNoneInsteadOfFind(config),
)
)
}

70
detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseAnyOrNoneInsteadOfFind.kt Normal file
View File

@@ -0,0 +1,70 @@
package io.gitlab.arturbosch.detekt.rules.style
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 io.gitlab.arturbosch.detekt.rules.isCalling
import io.gitlab.arturbosch.detekt.rules.isNonNullCheck
import io.gitlab.arturbosch.detekt.rules.isNullCheck
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.psi.KtBinaryExpression
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtQualifiedExpression
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
import org.jetbrains.kotlin.resolve.BindingContext
/**
* Turn on this rule to flag `find` calls for null check that can be replaced with a `any` or `none` call.
*
* <noncompliant>
* listOf(1, 2, 3).find { it == 4 } != null
* listOf(1, 2, 3).find { it == 4 } == null
* </noncompliant>
*
* <compliant>
* listOf(1, 2, 3).any { it == 4 }
* listOf(1, 2, 3).none { it == 4 }
* </compliant>
*/
@RequiresTypeResolution
class UseAnyOrNoneInsteadOfFind(config: Config = Config.empty) : Rule(config) {
override val issue: Issue = Issue(
"UseAnyOrNoneInsteadOfFind",
Severity.Style,
"Use 'any' or 'none' instead of 'find' and null check",
Debt.FIVE_MINS
)
@Suppress("ReturnCount")
override fun visitCallExpression(expression: KtCallExpression) {
super.visitCallExpression(expression)
if (bindingContext == BindingContext.EMPTY) return
val functionName = expression.calleeExpression?.text ?: return
val qualifiedOrThis = expression.getStrictParentOfType<KtQualifiedExpression>() ?: expression
val binary = qualifiedOrThis.getStrictParentOfType<KtBinaryExpression>()?.takeIf {
it.left == qualifiedOrThis || it.right == qualifiedOrThis
} ?: return
if (!expression.isCalling(functionFqNames, bindingContext)) return
val replacement = when {
binary.isNonNullCheck() -> "any"
binary.isNullCheck() -> "none"
else -> return
}
val message = "Use '$replacement' instead of '$functionName'"
report(CodeSmell(issue, Entity.from(expression), message))
}
companion object {
private val functionNames = listOf("find", "firstOrNull", "lastOrNull")
private val functionFqNames =
listOf("kotlin.collections", "kotlin.sequences", "kotlin.text").flatMap { packageName ->
functionNames.map { functionName -> FqName("$packageName.$functionName") }
}
}
}

84
detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UseAnyOrNoneInsteadOfFindSpec.kt Normal file
View File

@@ -0,0 +1,84 @@
package io.gitlab.arturbosch.detekt.rules.style
import io.gitlab.arturbosch.detekt.rules.setupKotlinEnvironment
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe
class UseAnyOrNoneInsteadOfFindSpec : Spek({
setupKotlinEnvironment()
val env: KotlinCoreEnvironment by memoized()
val subject by memoized { UseAnyOrNoneInsteadOfFind() }
describe("UseAnyOrNoneInsteadOfFind rule") {
it("Reports collections.find != null") {
val code = "val x = listOf(1, 2, 3).find { it == 4 } != null"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).hasSize(1)
assertThat(actual[0].message).isEqualTo("Use 'any' instead of 'find'")
}
it("Reports sequences.find != null") {
val code = "val x = sequenceOf(1, 2, 3).find { it == 4 } != null"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).hasSize(1)
}
it("Reports text.find != null") {
val code = "val x = \"123\".find { it == '4' } != null"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).hasSize(1)
}
it("Reports collections.firstOrNull != null") {
val code = "val x = arrayOf(1, 2, 3).firstOrNull { it == 4 } != null"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).hasSize(1)
assertThat(actual[0].message).isEqualTo("Use 'any' instead of 'firstOrNull'")
}
it("Reports sequences.firstOrNull != null") {
val code = "val x = sequenceOf(1, 2, 3).firstOrNull { it == 4 } != null"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).hasSize(1)
}
it("Reports text.firstOrNull != null") {
val code = "val x = \"123\".firstOrNull { it == '4' } != null"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).hasSize(1)
}
it("Reports collections.find == null") {
val code = "val x = setOf(1, 2, 3).find { it == 4 } == null"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).hasSize(1)
assertThat(actual[0].message).isEqualTo("Use 'none' instead of 'find'")
}
it("Reports null != collections.find") {
val code = "val x = null != listOf(1, 2, 3).find { it == 4 }"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).hasSize(1)
assertThat(actual[0].message).isEqualTo("Use 'any' instead of 'find'")
}
it("Reports collections.find != null in extension") {
val code = "fun List<Int>.test(): Boolean = find { it == 4 } != null"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).hasSize(1)
}
it("Reports collections.lastOrNull != null") {
val code = "val x = listOf(1, 2, 3).lastOrNull { it == 4 } != null"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).hasSize(1)
assertThat(actual[0].message).isEqualTo("Use 'any' instead of 'lastOrNull'")
}
it("Does not report collections.find") {
val code = "val x = listOf(1, 2, 3).find { it == 4 }"
val actual = subject.compileAndLintWithContext(env, code)
assertThat(actual).isEmpty()
}
}
})