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.

Sign up for GitHub

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

SONARKT-423 Migrate SingletonPatternCheck to kotlin-analysis-api #521

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

SONARKT-423 Migrate SingletonPatternCheck to kotlin-analysis-api #521

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 14 additions & 20 deletions sonar-kotlin-checks/src/main/java/org/sonarsource/kotlin/checks/SingletonPatternCheck.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
*/
package org.sonarsource.kotlin.checks

import org.jetbrains.kotlin.descriptors.ConstructorDescriptor
import org.jetbrains.kotlin.analysis.api.resolution.successfulFunctionCallOrNull
import org.jetbrains.kotlin.analysis.api.resolution.symbol
import org.jetbrains.kotlin.analysis.api.symbols.KaConstructorSymbol
import org.jetbrains.kotlin.psi.KtBlockExpression
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtClass
Expand All @@ -30,23 +32,19 @@ import org.jetbrains.kotlin.psi.KtProperty
import org.jetbrains.kotlin.psi.KtPropertyDelegate
import org.jetbrains.kotlin.psi.allConstructors
import org.jetbrains.kotlin.psi.psiUtil.isPrivate
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.calls.util.getCall
import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
import org.sonar.check.Rule
import org.sonarsource.kotlin.api.checks.AbstractCheck
import org.sonarsource.kotlin.api.checks.ConstructorMatcher
import org.sonarsource.kotlin.api.checks.FunMatcher
import org.sonarsource.kotlin.api.frontend.KotlinFileContext
import org.sonarsource.kotlin.api.visiting.KtTreeVisitor
import org.sonarsource.kotlin.api.visiting.withKaSession

private val lazyInitializationMatcher = FunMatcher(
name = "lazy",
definingSupertype = "kotlin"
)

@org.sonarsource.kotlin.api.frontend.K1only
@Rule(key = "S6515")
class SingletonPatternCheck : AbstractCheck() {

Expand All @@ -57,13 +55,13 @@ class SingletonPatternCheck : AbstractCheck() {
if (singletonClassCandidates.isEmpty()) return

val singleConstructorCallExtractor =
SingleConstructorCallExtractor(singletonClassCandidates, kotlinFileContext.bindingContext)
SingleConstructorCallExtractor(singletonClassCandidates)
singleConstructorCallExtractor.visitTree(file)
val singleConstructorCallByClass = singleConstructorCallExtractor.singleConstructorCallByClass
if (singleConstructorCallByClass.isEmpty()) return

singleConstructorCallByClass.values.stream().filter {
isInitializingCall(it) || isLazyInitializingCall(it, kotlinFileContext.bindingContext)
isInitializingCall(it) || isLazyInitializingCall(it)
}.forEach {
kotlinFileContext.reportIssue(it, "Singleton pattern should use object declarations or expressions.")
}
Expand All @@ -85,27 +83,23 @@ private class SingletonClassCandidateExtractor : KtTreeVisitor() {

private fun isSingletonClassCandidate(klass: KtClass): Boolean {
val constructors = klass.allConstructors
return klass.isPrivate() || (constructors.isNotEmpty() && constructors.all { it.isPrivate() } )
return klass.isPrivate() || (constructors.isNotEmpty() && constructors.all { it.isPrivate() })
}


private class SingleConstructorCallExtractor(
private val singletonClassCandidates: MutableSet<String>,
private val bindingContext: BindingContext,
) : KtTreeVisitor() {

val singleConstructorCallByClass: MutableMap<String, KtCallExpression> = mutableMapOf()

private var constructorMatcher = ConstructorMatcher { withTypeNames(*singletonClassCandidates.toTypedArray()) }

override fun visitCallExpression(expression: KtCallExpression) {
if (singletonClassCandidates.isEmpty() || !constructorMatcher.matches(expression, bindingContext)) return

val constructorDescriptor =
bindingContext[BindingContext.RESOLVED_CALL, expression.getCall(bindingContext)]?.resultingDescriptor as? ConstructorDescriptor
?: return
override fun visitCallExpression(expression: KtCallExpression): Unit = withKaSession {
if (singletonClassCandidates.isEmpty() || !constructorMatcher.matches(expression)) return

val fqName = constructorDescriptor.constructedClass.fqNameSafe.asString()
val kaConstructorSymbol = expression.resolveToCall()?.successfulFunctionCallOrNull()
?.partiallyAppliedSymbol?.symbol as? KaConstructorSymbol ?: return
val fqName = kaConstructorSymbol.containingClassId?.asFqNameString() ?: return
singleConstructorCallByClass.put(fqName, expression)?.let {
singleConstructorCallByClass.remove(fqName)
singletonClassCandidates.remove(fqName)
Expand All @@ -119,7 +113,7 @@ private fun isInitializingCall(callExpression: KtCallExpression): Boolean {
return isStaticProperty(property)
}

private fun isLazyInitializingCall(callExpression: KtCallExpression, bindingContext: BindingContext): Boolean {
private fun isLazyInitializingCall(callExpression: KtCallExpression): Boolean {
val bodyExpression = callExpression.parent as? KtBlockExpression ?: return false
if (bodyExpression.statements.last() !== callExpression) return false
val call = (((bodyExpression.parent as? KtFunctionLiteral)
Expand All @@ -128,7 +122,7 @@ private fun isLazyInitializingCall(callExpression: KtCallExpression, bindingCont
?.parent as? KtCallExpression ?: return false

val property = (call.parent as? KtPropertyDelegate)?.parent as? KtProperty ?: return false
return isStaticProperty(property) && lazyInitializationMatcher.matches(call.getResolvedCall(bindingContext))
return isStaticProperty(property) && lazyInitializationMatcher.matches(call)
}

private fun isStaticProperty(property: KtProperty): Boolean =
Expand Down