From f6a55670a0823a0fc408e81eb1a2618dc704bdfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brais=20Gab=C3=ADn?= Date: Thu, 4 Aug 2022 19:29:57 +0200 Subject: [PATCH] Implement requiresTypeResoultion Rule --- config/detekt/baseline.xml | 15 +++ .../gitlab/arturbosch/detekt/api/MultiRule.kt | 1 + detekt-rules-ruleauthors/build.gradle.kts | 4 + .../detekt/authors/RequiresTypeResolution.kt | 81 ++++++++++++++++ .../detekt/authors/RuleAuthorsProvider.kt | 8 +- .../src/main/resources/config/config.yml | 2 + .../authors/RequiresTypeResolutionSpec.kt | 92 +++++++++++++++++++ 7 files changed, 201 insertions(+), 2 deletions(-) create mode 100644 detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RequiresTypeResolution.kt create mode 100644 detekt-rules-ruleauthors/src/test/kotlin/io/gitlab/arturbosch/detekt/authors/RequiresTypeResolutionSpec.kt diff --git a/config/detekt/baseline.xml b/config/detekt/baseline.xml index 4d1a6d7d972..aec87732271 100644 --- a/config/detekt/baseline.xml +++ b/config/detekt/baseline.xml @@ -2,5 +2,20 @@ + RequiresTypeResolution:FunctionOnlyReturningConstant.kt$FunctionOnlyReturningConstant : Rule + RequiresTypeResolution:ImplicitDefaultLocale.kt$ImplicitDefaultLocale : Rule + RequiresTypeResolution:InstanceOfCheckForException.kt$InstanceOfCheckForException : Rule + RequiresTypeResolution:LateinitUsage.kt$LateinitUsage : Rule + RequiresTypeResolution:LongParameterList.kt$LongParameterList : Rule + RequiresTypeResolution:MapGetWithNotNullAssertionOperator.kt$MapGetWithNotNullAssertionOperator : Rule + RequiresTypeResolution:MemberNameEqualsClassName.kt$MemberNameEqualsClassName : Rule + RequiresTypeResolution:OptionalUnit.kt$OptionalUnit : Rule + RequiresTypeResolution:SpreadOperator.kt$SpreadOperator : Rule + RequiresTypeResolution:UnnecessaryAbstractClass.kt$UnnecessaryAbstractClass : Rule + RequiresTypeResolution:UnusedImports.kt$UnusedImports : Rule + RequiresTypeResolution:UnusedPrivateMember.kt$UnusedPrivateMember : Rule + RequiresTypeResolution:UseCheckOrError.kt$UseCheckOrError : Rule + RequiresTypeResolution:UseDataClass.kt$UseDataClass : Rule + RequiresTypeResolution:UseRequire.kt$UseRequire : Rule diff --git a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/MultiRule.kt b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/MultiRule.kt index a9034a82b7a..e20d8b99f32 100644 --- a/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/MultiRule.kt +++ b/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/MultiRule.kt @@ -7,6 +7,7 @@ import org.jetbrains.kotlin.psi.KtFile * Can be used to combine different rules which do similar work like * scanning the source code line by line to increase performance. */ +@Suppress("RequiresTypeResolution") abstract class MultiRule : BaseRule() { abstract val rules: List diff --git a/detekt-rules-ruleauthors/build.gradle.kts b/detekt-rules-ruleauthors/build.gradle.kts index e0469ce857e..5450965954d 100644 --- a/detekt-rules-ruleauthors/build.gradle.kts +++ b/detekt-rules-ruleauthors/build.gradle.kts @@ -7,3 +7,7 @@ dependencies { testImplementation(projects.detektTest) testImplementation(libs.assertj) } + +tasks.withType { + kotlinOptions.freeCompilerArgs = listOf("-Xcontext-receivers") +} diff --git a/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RequiresTypeResolution.kt b/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RequiresTypeResolution.kt new file mode 100644 index 00000000000..dafba19a67c --- /dev/null +++ b/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RequiresTypeResolution.kt @@ -0,0 +1,81 @@ +@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.KtElement +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.types.typeUtil.supertypes +import kotlin.reflect.KClass + +/** + * If a rule uses `bindingContext` should be annotated with `@RequiresTypeResolution`. And if it doesn't it shouldn't + * be annotated with it. + */ +@ActiveByDefault("1.22.0") +@RequiresTypeResolution +class RequiresTypeResolution(config: Config = Config.empty) : Rule(config) { + override val issue = Issue( + javaClass.simpleName, + Severity.Defect, + "`@RequiresTypeResolution` should be used if and only if `bindingContext` is used.", + Debt.FIVE_MINS + ) + + override fun visitClass(klass: KtClass) { + super.visitClass(klass) + + if (klass.extendsFrom(BaseRule::class)) { + val usesBindingContext = usesBindingContext(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`" + ) + ) + } + } + } +} + +private fun usesBindingContext(element: KtElement): Boolean { + return element.containingKtFile.text.contains("bindingContext", ignoreCase = false) +} + +context(BaseRule) private inline fun KtClass.extendsFrom(kClass: KClass): Boolean { + return bindingContext[BindingContext.CLASS, this] + ?.defaultType + ?.supertypes() + .orEmpty() + .any { it.fqNameOrNull()?.toString() == checkNotNull(kClass.qualifiedName) } +} + +context(BaseRule) private inline fun KtClass.isAnnotatedWith(kClass: KClass): Boolean { + return annotationEntries + .asSequence() + .mapNotNull { it.typeReference } + .mapNotNull { bindingContext[BindingContext.TYPE, it] } + .any { it.fqNameOrNull()?.toString() == checkNotNull(kClass.qualifiedName) } +} diff --git a/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RuleAuthorsProvider.kt b/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RuleAuthorsProvider.kt index b0ae711cfc5..3b24de36bee 100644 --- a/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RuleAuthorsProvider.kt +++ b/detekt-rules-ruleauthors/src/main/kotlin/io/gitlab/arturbosch/detekt/authors/RuleAuthorsProvider.kt @@ -13,6 +13,10 @@ class RuleAuthorsProvider : RuleSetProvider { override val ruleSetId: String = "detekt" - @Suppress("UseEmptyCounterpart") - override fun instance(config: Config) = RuleSet(ruleSetId, listOf()) + override fun instance(config: Config) = RuleSet( + ruleSetId, + listOf( + RequiresTypeResolution(config), + ) + ) } diff --git a/detekt-rules-ruleauthors/src/main/resources/config/config.yml b/detekt-rules-ruleauthors/src/main/resources/config/config.yml index 87a20dcc624..248c155493e 100644 --- a/detekt-rules-ruleauthors/src/main/resources/config/config.yml +++ b/detekt-rules-ruleauthors/src/main/resources/config/config.yml @@ -1,2 +1,4 @@ detekt: active: true + RequiresTypeResolution: + active: true diff --git a/detekt-rules-ruleauthors/src/test/kotlin/io/gitlab/arturbosch/detekt/authors/RequiresTypeResolutionSpec.kt b/detekt-rules-ruleauthors/src/test/kotlin/io/gitlab/arturbosch/detekt/authors/RequiresTypeResolutionSpec.kt new file mode 100644 index 00000000000..7310fb3bd31 --- /dev/null +++ b/detekt-rules-ruleauthors/src/test/kotlin/io/gitlab/arturbosch/detekt/authors/RequiresTypeResolutionSpec.kt @@ -0,0 +1,92 @@ +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 RequiresTypeResolutionSpec(private val env: KotlinCoreEnvironment) { + + private val rule = RequiresTypeResolution() + + @Test + fun `should not report classes that doesn't extend from BaseRule`() { + val code = """ + class A { + val issue: Int = error("bindingContext") + } + """ + 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 B(config: Config) : Rule(config) { + override val issue = error("I don't care") + + private fun asdf() { + bindingContext + } + } + """ + 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 C(config: Config) : Rule(config) { + override val issue = error("I don't care") + } + """ + 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 D(config: Config) : Rule(config) { + override val issue = error("I don't care") + + private fun asdf() { + bindingContext + } + } + """ + val findings = rule.compileAndLintWithContext(env, code) + assertThat(findings).isEmpty() + } + + @Test + fun `should not report Rules that doesn'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 E(config: Config) : Rule(config) { + override val issue = error("I don't care") + } + """ + val findings = rule.compileAndLintWithContext(env, code) + assertThat(findings).hasSize(1) + } +}