-
-
Notifications
You must be signed in to change notification settings - Fork 755
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add PropertyUsedBeforeDeclaration rule (#6062)
- Loading branch information
1 parent
11ec4f0
commit a6aeb38
Showing
4 changed files
with
182 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
89 changes: 89 additions & 0 deletions
89
...e/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PropertyUsedBeforeDeclaration.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
package io.gitlab.arturbosch.detekt.rules.bugs | ||
|
||
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.descriptors.DeclarationDescriptor | ||
import org.jetbrains.kotlin.psi.KtClassOrObject | ||
import org.jetbrains.kotlin.psi.KtNameReferenceExpression | ||
import org.jetbrains.kotlin.psi.KtProperty | ||
import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType | ||
import org.jetbrains.kotlin.resolve.BindingContext | ||
import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall | ||
import org.jetbrains.kotlin.utils.addIfNotNull | ||
|
||
/** | ||
* Reports properties that are used before declaration. | ||
* | ||
* <noncompliant> | ||
* class C { | ||
* private val number | ||
* get() = if (isValid) 1 else 0 | ||
* | ||
* val list = listOf(number) | ||
* | ||
* private val isValid = true | ||
* } | ||
* | ||
* fun main() { | ||
* println(C().list) // [0] | ||
* } | ||
* </noncompliant> | ||
* | ||
* <compliant> | ||
* class C { | ||
* private val isValid = true | ||
* | ||
* private val number | ||
* get() = if (isValid) 1 else 0 | ||
* | ||
* val list = listOf(number) | ||
* } | ||
* | ||
* fun main() { | ||
* println(C().list) // [1] | ||
* } | ||
* </compliant> | ||
*/ | ||
@RequiresTypeResolution | ||
class PropertyUsedBeforeDeclaration(config: Config = Config.empty) : Rule(config) { | ||
override val issue = Issue( | ||
javaClass.simpleName, | ||
Severity.Defect, | ||
"Properties before declaration should not be used.", | ||
Debt.FIVE_MINS | ||
) | ||
|
||
override fun visitClassOrObject(classOrObject: KtClassOrObject) { | ||
super.visitClassOrObject(classOrObject) | ||
|
||
val classMembers = classOrObject.body?.children ?: return | ||
|
||
val allProperties = classMembers.filterIsInstance<KtProperty>().mapNotNull { | ||
val name = it.name ?: return@mapNotNull null | ||
val descriptor = bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, it] ?: return@mapNotNull null | ||
name to descriptor | ||
}.toMap() | ||
|
||
val declaredProperties = mutableSetOf<DeclarationDescriptor>() | ||
|
||
classMembers.forEach { member -> | ||
member.forEachDescendantOfType<KtNameReferenceExpression> { | ||
val property = allProperties[it.text] | ||
if (property != null && property !in declaredProperties && property == it.descriptor()) { | ||
report(CodeSmell(issue, Entity.from(it), "'${it.text}' is before declaration.")) | ||
} | ||
} | ||
if (member is KtProperty) { | ||
declaredProperties.addIfNotNull(allProperties[member.name]) | ||
} | ||
} | ||
} | ||
|
||
private fun KtNameReferenceExpression.descriptor() = getResolvedCall(bindingContext)?.resultingDescriptor | ||
} |
89 changes: 89 additions & 0 deletions
89
...c/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/PropertyUsedBeforeDeclarationSpec.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
package io.gitlab.arturbosch.detekt.rules.bugs | ||
|
||
import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest | ||
import io.gitlab.arturbosch.detekt.test.assertThat | ||
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext | ||
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment | ||
import org.junit.jupiter.api.Test | ||
|
||
@KotlinCoreEnvironmentTest | ||
class PropertyUsedBeforeDeclarationSpec(private val env: KotlinCoreEnvironment) { | ||
private val subject = PropertyUsedBeforeDeclaration() | ||
|
||
@Test | ||
fun `used before declaration in getter`() { | ||
val code = """ | ||
class C { | ||
private val number get() = if (isValid) 1 else 0 | ||
val list = listOf(number) | ||
private val isValid = true | ||
} | ||
fun main() { | ||
println(C().list) // [0] | ||
} | ||
""".trimIndent() | ||
val findings = subject.compileAndLintWithContext(env, code) | ||
assertThat(findings).hasSize(1) | ||
assertThat(findings).hasTextLocations(45 to 52) | ||
assertThat(findings.first()).hasMessage("'isValid' is before declaration.") | ||
} | ||
|
||
@Test | ||
fun `used before declaration in init`() { | ||
val code = """ | ||
class C { | ||
init { | ||
run { | ||
println(isValid) // false | ||
} | ||
} | ||
private val isValid = true | ||
} | ||
fun main() { | ||
C() | ||
} | ||
""".trimIndent() | ||
val findings = subject.compileAndLintWithContext(env, code) | ||
assertThat(findings).hasSize(1) | ||
} | ||
|
||
@Test | ||
fun `used before declaration in function`() { | ||
val code = """ | ||
class C { | ||
fun f() = isValid | ||
private val isValid = true | ||
} | ||
""".trimIndent() | ||
val findings = subject.compileAndLintWithContext(env, code) | ||
assertThat(findings).hasSize(1) | ||
} | ||
|
||
@Test | ||
fun `used after declaration in getter`() { | ||
val code = """ | ||
class C { | ||
private val isValid = true | ||
private val number get() = if (isValid) 1 else 0 | ||
val list = listOf(number) | ||
} | ||
""".trimIndent() | ||
val findings = subject.compileAndLintWithContext(env, code) | ||
assertThat(findings).isEmpty() | ||
} | ||
|
||
@Test | ||
fun `variable shadowing`() { | ||
val code = """ | ||
class C { | ||
fun f(): Boolean { | ||
val isValid = true | ||
return isValid | ||
} | ||
private val isValid = true | ||
} | ||
""".trimIndent() | ||
val findings = subject.compileAndLintWithContext(env, code) | ||
assertThat(findings).isEmpty() | ||
} | ||
} |