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

Implement rule to check the correct usage of @RequiresTypeResolution #5182

Merged
merged 10 commits into from Oct 16, 2022
5 changes: 5 additions & 0 deletions config/detekt/detekt.yml
Expand Up @@ -277,3 +277,8 @@ libraries:
active: true
excludes: ['**/*.kt']
includes: ['**/detekt-api/src/main/**/api/*.kt']

ruleauthors:
ViolatesTypeResolutionRequirements:
active: true
excludes: ['**/test/**', '**/*Test.kt', '**/*Spec.kt']
Expand Up @@ -8,6 +8,7 @@ import org.jetbrains.kotlin.psi.KtFile
* scanning the source code line by line to increase performance.
*/
@Deprecated("multi rules are much more difficult to maintain and support will be removed in the future")
@Suppress("ViolatesTypeResolutionRequirements")
abstract class MultiRule : BaseRule() {

abstract val rules: List<Rule>
Expand Down
Expand Up @@ -28,6 +28,7 @@ import org.jetbrains.kotlin.psi.KtSecondaryConstructor
/**
* Reports functions and constructors which have more parameters than a certain threshold.
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.0.0")
class LongParameterList(config: Config = Config.empty) : Rule(config) {
override val issue = Issue(
Expand Down
Expand Up @@ -42,6 +42,7 @@ import org.jetbrains.kotlin.resolve.calls.util.getType
* str.toLowerCase(Locale.US)
* </compliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.16.0")
class ImplicitDefaultLocale(config: Config = Config.empty) : Rule(config) {

Expand Down
Expand Up @@ -28,6 +28,7 @@ import org.jetbrains.kotlin.psi.psiUtil.containingClass
* }
* </noncompliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
class LateinitUsage(config: Config = Config.empty) : Rule(config) {

override val issue = Issue(
Expand Down
Expand Up @@ -43,6 +43,7 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
* map.getOrElse("key", { "" })
* </compliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.21.0")
class MapGetWithNotNullAssertionOperator(config: Config) : Rule(config) {

Expand Down
Expand Up @@ -44,6 +44,7 @@ import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf
* }
* </compliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.21.0")
class InstanceOfCheckForException(config: Config = Config.empty) : Rule(config) {

Expand Down
Expand Up @@ -52,6 +52,7 @@ import org.jetbrains.kotlin.resolve.BindingContext
* }
* </compliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.2.0")
class MemberNameEqualsClassName(config: Config = Config.empty) : Rule(config) {

Expand Down
Expand Up @@ -50,6 +50,7 @@ import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
* }
* </compliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.0.0")
class SpreadOperator(config: Config = Config.empty) : Rule(config) {

Expand Down
4 changes: 4 additions & 0 deletions detekt-rules-ruleauthors/build.gradle.kts
Expand Up @@ -7,3 +7,7 @@ dependencies {
testImplementation(projects.detektTest)
testImplementation(libs.assertj)
}

tasks.withType<org.jetbrains.kotlin.gradle.tasks.KotlinCompile> {
kotlinOptions.freeCompilerArgs = listOf("-Xcontext-receivers")
}
Expand Up @@ -13,6 +13,10 @@ class RuleAuthorsProvider : RuleSetProvider {

override val ruleSetId: String = "ruleauthors"

@Suppress("UseEmptyCounterpart")
override fun instance(config: Config) = RuleSet(ruleSetId, listOf())
override fun instance(config: Config) = RuleSet(
ruleSetId,
listOf(
ViolatesTypeResolutionRequirements(config),
)
)
}
@@ -0,0 +1,96 @@
@file:Suppress("ForbiddenComment")

package io.gitlab.arturbosch.detekt.authors

import io.gitlab.arturbosch.detekt.api.BaseRule
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.ActiveByDefault
import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
import io.gitlab.arturbosch.detekt.rules.fqNameOrNull
import org.jetbrains.kotlin.psi.KtClass
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtNameReferenceExpression
import org.jetbrains.kotlin.psi.KtReferenceExpression
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull
import org.jetbrains.kotlin.resolve.descriptorUtil.getAllSuperclassesWithoutAny
import kotlin.reflect.KClass

/**
* If a rule uses the property [BaseRule.bindingContext] should be annotated with `@RequiresTypeResolution`.
* And if the rule doesn't use that property it shouldn't be annotated with it.
*/
@ActiveByDefault("1.22.0")
@RequiresTypeResolution
class ViolatesTypeResolutionRequirements(config: Config = Config.empty) : Rule(config) {
override val issue = Issue(
javaClass.simpleName,
Severity.Defect,
"`@RequiresTypeResolution` should be used if and only if the property `bindingContext` is used.",
Debt.FIVE_MINS
)

private val klasses: MutableList<KtClass> = mutableListOf()
private var usesBindingContext: Boolean = false

override fun visitKtFile(file: KtFile) {
super.visitKtFile(file)
klasses.forEach { klass ->
val isAnnotatedWithRequiresTypeResolution = klass.isAnnotatedWith(RequiresTypeResolution::class)
if (usesBindingContext && !isAnnotatedWithRequiresTypeResolution) {
report(
CodeSmell(
issue,
Entity.atName(klass),
"`${klass.name}` uses `bindingContext` but is not annotated with `@RequiresTypeResolution`"
)
)
} else if (!usesBindingContext && isAnnotatedWithRequiresTypeResolution) {
report(
CodeSmell(
issue,
Entity.atName(klass),
"`${klass.name}` is annotated with `@RequiresTypeResolution` but doesn't use `bindingContext`"
)
)
}
}
klasses.clear()
usesBindingContext = false
}

override fun visitClass(klass: KtClass) {
super.visitClass(klass)

if (klass.extendsFrom(BaseRule::class)) {
klasses.add(klass)
}
}

override fun visitReferenceExpression(expression: KtReferenceExpression) {
super.visitReferenceExpression(expression)
usesBindingContext = usesBindingContext ||
(expression is KtNameReferenceExpression && expression.text == "bindingContext")
}
}

context(BaseRule) private inline fun <reified T : Any> KtClass.extendsFrom(kClass: KClass<T>): Boolean {
return bindingContext[BindingContext.CLASS, this]
?.getAllSuperclassesWithoutAny()
.orEmpty()
.any { it.fqNameOrNull()?.toString() == checkNotNull(kClass.qualifiedName) }
}

context(BaseRule) private inline fun <reified T : Any> KtClass.isAnnotatedWith(kClass: KClass<T>): Boolean {
return annotationEntries
.asSequence()
.mapNotNull { it.typeReference }
.mapNotNull { bindingContext[BindingContext.TYPE, it] }
.any { it.fqNameOrNull()?.toString() == checkNotNull(kClass.qualifiedName) }
}
2 changes: 2 additions & 0 deletions detekt-rules-ruleauthors/src/main/resources/config/config.yml
@@ -1,2 +1,4 @@
ruleauthors:
active: true
ViolatesTypeResolutionRequirements:
active: true
@@ -0,0 +1,115 @@
package io.gitlab.arturbosch.detekt.authors

import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.junit.jupiter.api.Test

@KotlinCoreEnvironmentTest
internal class ViolatesTypeResolutionRequirementsSpec(private val env: KotlinCoreEnvironment) {

private val rule = ViolatesTypeResolutionRequirements()

@Test
fun `should not report classes that don't extend from BaseRule`() {
val code = """
class A {
val issue: Int = error("bindingContext")
}
""".trimIndent()
val findings = rule.compileAndLintWithContext(env, code)
assertThat(findings).isEmpty()
}

@Test
fun `should report Rules that use bindingContext and are not annotated`() {
val code = """
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Rule

class A(config: Config) : Rule(config) {
override val issue = error("I don't care")

private fun asdf() {
bindingContext
}
}
""".trimIndent()
val findings = rule.compileAndLintWithContext(env, code)
assertThat(findings).hasSize(1)
}

@Test
fun `should not report Rules that doesn't use bindingContext and are not annotated`() {
val code = """
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Rule

class A(config: Config) : Rule(config) {
override val issue = error("I don't care")
}
""".trimIndent()
val findings = rule.compileAndLintWithContext(env, code)
assertThat(findings).isEmpty()
}

@Test
fun `should not report Rules that use bindingContext and are annotated`() {
val code = """
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution

@RequiresTypeResolution
class A(config: Config) : Rule(config) {
override val issue = error("I don't care")

private fun asdf() {
bindingContext
}
}
""".trimIndent()
val findings = rule.compileAndLintWithContext(env, code)
assertThat(findings).isEmpty()
}

@Test
fun `should report Rules that don't use bindingContext and are annotated`() {
val code = """
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution

@RequiresTypeResolution
class A(config: Config) : Rule(config) {
override val issue = error("I don't care")
}
""".trimIndent()
val findings = rule.compileAndLintWithContext(env, code)
assertThat(findings).hasSize(1)
}

@Test
fun `should report Rules that use bindingContext outside class and are not annotated`() {
val code = """
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Rule

class A(config: Config) : Rule(config) {
override val issue = error("I don't care")

private fun asdf() {
extension()
}
}

inline fun Rule.extension(): Boolean {
bindingContext
return true
}
""".trimIndent()
val findings = rule.compileAndLintWithContext(env, code)
assertThat(findings).hasSize(1)
}
}
Expand Up @@ -35,6 +35,7 @@ import org.jetbrains.kotlin.psi.psiUtil.containingClass
* const val constantString = "1"
* </compliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.2.0")
class FunctionOnlyReturningConstant(config: Config = Config.empty) : Rule(config) {

Expand Down
Expand Up @@ -44,6 +44,7 @@ import org.jetbrains.kotlin.types.typeUtil.isInterface
* }
* </noncompliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.2.0")
class UnnecessaryAbstractClass(config: Config = Config.empty) : Rule(config) {

Expand Down
Expand Up @@ -28,6 +28,7 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.getImportableDescriptor
* Exempt from this rule are imports resulting from references to elements within KDoc and
* from destructuring declarations (componentN imports).
*/
@Suppress("ViolatesTypeResolutionRequirements")
class UnusedImports(config: Config) : Rule(config) {

override val issue = Issue(
Expand Down
Expand Up @@ -56,6 +56,7 @@ private const val ARRAY_GET_METHOD_NAME = "get"
* If these private elements are unused they should be removed. Otherwise, this dead code
* can lead to confusion and potential bugs.
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.16.0")
class UnusedPrivateMember(config: Config = Config.empty) : Rule(config) {

Expand Down
Expand Up @@ -37,6 +37,7 @@ import org.jetbrains.kotlin.psi.KtThrowExpression
* }
* </compliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.21.0")
class UseCheckOrError(config: Config = Config.empty) : Rule(config) {

Expand Down
Expand Up @@ -45,6 +45,7 @@ import org.jetbrains.kotlin.types.KotlinType
* class A(val b: B) : I by b
* </compliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
class UseDataClass(config: Config = Config.empty) : Rule(config) {

override val issue: Issue = Issue(
Expand Down
Expand Up @@ -29,6 +29,7 @@ import org.jetbrains.kotlin.psi.KtThrowExpression
* require(value >= 0) { "value is $value but should be at least 0" }
* </compliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
@ActiveByDefault(since = "1.21.0")
class UseRequire(config: Config = Config.empty) : Rule(config) {

Expand Down
Expand Up @@ -50,6 +50,7 @@ import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull
* override fun foo() = Unit
* </compliant>
*/
@Suppress("ViolatesTypeResolutionRequirements")
class OptionalUnit(config: Config = Config.empty) : Rule(config) {

override val issue = Issue(
Expand Down