SONARKT-494 Migrate UnpredictableHashSaltCheck to kotlin-analysis-api

kotlin-checks-test-sources/src/main/kotlin/checks/EqualsOverridenWithHashCodeCheckSample.kt

@@ -2,7 +2,6 @@ package checks
 
 class EqualsOverridenWithHashCodeCheckSample {
 
-
     override fun equals(other: Any?): Boolean { // Noncompliant {{This class overrides "equals()" and should therefore also override "hashCode()".}}
         return super.equals(other)
     }
@@ -89,3 +88,9 @@ abstract class AmbiguousParent3 {
         return super.equals(other)
     }
 }
+
+private val testAnonymousClass = object : Any() {
+    override fun equals(other: Any?): Boolean { // Noncompliant
+        return super.equals(other)
+    }
+}

sonar-kotlin-api/src/main/java/org/sonarsource/kotlin/api/checks/ApiExtensions.kt

@@ -23,9 +23,11 @@ import com.intellij.psi.impl.source.tree.LeafPsiElement
 import com.intellij.psi.util.PsiTreeUtil
 import org.jetbrains.kotlin.analysis.api.KaIdeApi
 import org.jetbrains.kotlin.analysis.api.resolution.KaCall
+import org.jetbrains.kotlin.analysis.api.resolution.KaCallableMemberCall
 import org.jetbrains.kotlin.analysis.api.resolution.KaExplicitReceiverValue
 import org.jetbrains.kotlin.analysis.api.resolution.KaFunctionCall
 import org.jetbrains.kotlin.analysis.api.resolution.KaImplicitReceiverValue
+import org.jetbrains.kotlin.analysis.api.resolution.singleCallOrNull
 import org.jetbrains.kotlin.analysis.api.resolution.singleFunctionCallOrNull
 import org.jetbrains.kotlin.analysis.api.resolution.successfulCallOrNull
 import org.jetbrains.kotlin.analysis.api.resolution.successfulFunctionCallOrNull
@@ -689,6 +691,7 @@ fun ResolvedValueArgument.isNull(bindingContext: BindingContext) = (
 fun KtExpression.isPredictedNull() =
    predictRuntimeValueExpression().isNull()
 
+@Deprecated("use kotlin-analysis-api instead")
 fun KtExpression.getCalleeOrUnwrappedGetMethod(bindingContext: BindingContext) =
     (this as? KtDotQualifiedExpression)?.let { dotQualifiedExpression ->
         (dotQualifiedExpression.selectorExpression as? KtNameReferenceExpression)?.let { nameSelector ->
@@ -699,12 +702,24 @@ fun KtExpression.getCalleeOrUnwrappedGetMethod(bindingContext: BindingContext) =
 /**
  * Checks whether the expression is a call, matches the FunMatchers in [STRING_TO_BYTE_FUNS] and is called on a constant string value.
  */
+@Deprecated("use kotlin-analysis-api instead", ReplaceWith("this.isBytesInitializedFromString()"))
 fun KtExpression.isBytesInitializedFromString(bindingContext: BindingContext) =
     getCalleeOrUnwrappedGetMethod(bindingContext)?.let { callee ->
         STRING_TO_BYTE_FUNS.any { it.matches(callee) } &&
             (getCall(bindingContext)?.explicitReceiver as? ExpressionReceiver)?.expression?.predictRuntimeStringValue(bindingContext) != null
     } ?: false
 
+fun KtExpression.isBytesInitializedFromString(): Boolean = withKaSession {
+    val call = [email protected]()?.successfulCallOrNull<KaCallableMemberCall<*, *>>()
+        ?: return@withKaSession false
+    STRING_TO_BYTE_FUNS.any { it.matches(call) } &&
+            ((call.partiallyAppliedSymbol.extensionReceiver ?: call.partiallyAppliedSymbol.dispatchReceiver)
+                    as? KaExplicitReceiverValue)
+                ?.expression
+                ?.predictRuntimeStringValue() != null
+}
+
+@Deprecated("use kotlin-analysis-api instead")
 fun ResolvedCall<*>.simpleArgExpressionOrNull(index: Int) =
     this.valueArgumentsByIndex
         ?.getOrNull(index)
@@ -762,12 +777,22 @@ fun KtProperty.findUsages(
 /**
  * Checks whether the variable has been initialized with the help of a secure random function
  */
+@Deprecated("use kotlin-analysis-api instead", ReplaceWith("this.isInitializedPredictably(searchStartNode)"))
 fun KtExpression.isInitializedPredictably(searchStartNode: KtExpression, bindingContext: BindingContext): Boolean {
     return this !is KtNameReferenceExpression || this.findUsages(searchStartNode) {
         it.getParentOfType<KtCallExpression>(false).getResolvedCall(bindingContext) matches SECURE_RANDOM_FUNS
     }.isEmpty()
 }
 
+fun KtExpression.isInitializedPredictably(searchStartNode: KtExpression): Boolean {
+    return this !is KtNameReferenceExpression || this.findUsages(searchStartNode) {
+        withKaSession {
+            it.getParentOfType<KtCallExpression>(false)?.resolveToCall()
+                ?.successfulFunctionCallOrNull() matches SECURE_RANDOM_FUNS
+        }
+    }.isEmpty()
+}
+
 /**
  * Checks if an expression is a function local variable
  */

sonar-kotlin-api/src/main/java/org/sonarsource/kotlin/api/checks/FunMatcher.kt

@@ -20,12 +20,17 @@ import org.jetbrains.kotlin.analysis.api.KaExperimentalApi
 import org.jetbrains.kotlin.analysis.api.resolution.KaCallableMemberCall
 import org.jetbrains.kotlin.analysis.api.resolution.KaFunctionCall
 import org.jetbrains.kotlin.analysis.api.resolution.KaReceiverValue
-import org.jetbrains.kotlin.analysis.api.resolution.KaVariableAccessCall
+import org.jetbrains.kotlin.analysis.api.resolution.KaSimpleVariableAccess
+import org.jetbrains.kotlin.analysis.api.resolution.KaSimpleVariableAccessCall
 import org.jetbrains.kotlin.analysis.api.resolution.successfulFunctionCallOrNull
+import org.jetbrains.kotlin.analysis.api.resolution.symbol
 import org.jetbrains.kotlin.analysis.api.signatures.KaCallableSignature
+import org.jetbrains.kotlin.analysis.api.symbols.KaCallableSymbol
 import org.jetbrains.kotlin.analysis.api.symbols.KaConstructorSymbol
 import org.jetbrains.kotlin.analysis.api.symbols.KaFunctionSymbol
 import org.jetbrains.kotlin.analysis.api.symbols.KaNamedFunctionSymbol
+import org.jetbrains.kotlin.analysis.api.symbols.KaPropertySymbol
+import org.jetbrains.kotlin.analysis.api.symbols.KaSyntheticJavaPropertySymbol
 import org.jetbrains.kotlin.analysis.api.symbols.name
 import org.jetbrains.kotlin.analysis.api.types.KaClassType
 import org.jetbrains.kotlin.backend.common.descriptors.isSuspend
@@ -39,9 +44,9 @@ import org.jetbrains.kotlin.psi.KtCallExpression
 import org.jetbrains.kotlin.psi.KtNamedFunction
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.BindingContext.RESOLVED_CALL
-import org.jetbrains.kotlin.resolve.calls.util.getCall
 import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
 import org.jetbrains.kotlin.resolve.calls.tasks.isDynamic
+import org.jetbrains.kotlin.resolve.calls.util.getCall
 import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
 import org.jetbrains.kotlin.resolve.descriptorUtil.isExtension
 import org.jetbrains.kotlin.resolve.descriptorUtil.overriddenTreeUniqueAsSequence
@@ -91,7 +96,8 @@ class FunMatcherImpl(
 
     @OptIn(KaExperimentalApi::class)
     fun matches(node: KtNamedFunction): Boolean = withKaSession {
-        return matches(null, node.symbol.asSignature())
+        val symbol = node.symbol
+        return matches(null, symbol, symbol.asSignature())
     }
 
     @Deprecated("use kotlin-analysis-api instead")
@@ -102,17 +108,57 @@ class FunMatcherImpl(
     fun matches(resolvedCall: ResolvedCall<*>?) = preCheckArgumentCount(resolvedCall?.call) &&
         matches(resolvedCall?.resultingDescriptor)
 
-    fun matches(resolvedCall: KaCallableMemberCall<*, *>?) =
+    @OptIn(KaExperimentalApi::class)
+    fun matches(resolvedCall: KaCallableMemberCall<*, *>?): Boolean = withKaSession {
         when (resolvedCall) {
-            is KaFunctionCall -> matches(
-                resolvedCall.partiallyAppliedSymbol.dispatchReceiver,
-                resolvedCall.partiallyAppliedSymbol.signature
-            )
-            is KaVariableAccessCall ->
-                // TODO handle calls of getters and setters
-                false
-            else -> false
+            null -> false
+            is KaFunctionCall -> {
+                val symbol = resolvedCall.partiallyAppliedSymbol
+                matches(symbol.dispatchReceiver, symbol.signature.symbol, symbol.signature)
+            }
+            is KaSimpleVariableAccessCall -> {
+                val propertySymbol = (resolvedCall.symbol as? KaPropertySymbol) ?: return false
+                when (resolvedCall.simpleAccess) {
+                    is KaSimpleVariableAccess.Read -> {
+                        val symbolForNameCheck =
+                            /**
+                             * Note that this allows to use matcher with name `"getExample"`
+                             * for both function call `getExample(...)` and read by property name `example`.
+                             */
+                            if (propertySymbol is KaSyntheticJavaPropertySymbol)
+                                propertySymbol.javaGetterSymbol
+                            else
+                                propertySymbol
+                        val getterSignature = propertySymbol.getter?.asSignature() ?: return false
+                        checkName(symbolForNameCheck) &&
+                                checkCallParameters(getterSignature) &&
+                                checkReturnType(getterSignature) &&
+                                checkTypeOrSupertype(null, propertySymbol)
+                    }
+                    is KaSimpleVariableAccess.Write -> {
+                        val symbolForNameCheck =
+                            /**
+                             * Note that this allows to use matcher with name `"setExample"`
+                             * for both function call `setExample(...)` and write by property name `example`.
+                             */
+                            if (propertySymbol is KaSyntheticJavaPropertySymbol)
+                                propertySymbol.javaSetterSymbol ?: return false
+                            else
+                                propertySymbol
+                        val setterSignature = propertySymbol.setter?.asSignature() ?: return false
+                        checkName(symbolForNameCheck) &&
+                                checkCallParameters(setterSignature) &&
+                                checkReturnType(setterSignature) &&
+                                checkTypeOrSupertype(
+                                    null,
+                                    // TODO propertySymbol works only in K2 (see ScheduledThreadPoolExecutorZeroCheck):
+                                    symbolForNameCheck
+                                )
+                    }
+                }
+            }
         }
+    }
 
     private fun preCheckArgumentCount(call: Call?) = call == null || call.valueArguments.size <= maxArgumentCount
 
@@ -130,6 +176,7 @@ class FunMatcherImpl(
 
     private fun matches(
         dispatchReceiver: KaReceiverValue?,
+        symbol: KaCallableSymbol,
         callableSignature: KaCallableSignature<*>
     ): Boolean {
         if (isDynamic != null) TODO()
@@ -138,8 +185,8 @@ class FunMatcherImpl(
             checkIsSuspending(callableSignature) &&
             checkIsOperator(callableSignature) &&
             checkReturnType(callableSignature) &&
-            checkName(callableSignature) &&
-            checkTypeOrSupertype(dispatchReceiver, callableSignature) &&
+            checkName(symbol) &&
+            checkTypeOrSupertype(dispatchReceiver, symbol) &&
             checkCallParameters(callableSignature)
     }
 
@@ -150,17 +197,17 @@ class FunMatcherImpl(
 
     private fun checkTypeOrSupertype(
         dispatchReceiver: KaReceiverValue?,
-        callableSignature: KaCallableSignature<*>,
+        symbol: KaCallableSymbol,
     ): Boolean {
         if (qualifiersOrDefiningSupertypes.isEmpty()) return true
 
         // TODO try to use only dispatchReceiver?
         // java.lang.Math.random has kotlin/Unit receiver type in K2 and null in K1?
         val receiverFQN = (dispatchReceiver?.type as? KaClassType)?.classId?.asFqNameString();
         if (qualifiersOrDefiningSupertypes.contains(receiverFQN)) return true
-        if (qualifiersOrDefiningSupertypes.contains(getActualQualifier(callableSignature))) return true
+        if (qualifiersOrDefiningSupertypes.contains(getActualQualifier(symbol))) return true
 
-        if (!definingSupertypes.isNullOrEmpty() && checkSubType(callableSignature)) return true
+        if (!definingSupertypes.isNullOrEmpty() && checkSubType(symbol)) return true
         return false
     }
 
@@ -174,8 +221,7 @@ class FunMatcherImpl(
 
     // TODO when null?
     /** @return dot-separated package name for top-level functions, class name otherwise */
-    private fun getActualQualifier(callableSignature: KaCallableSignature<*>): String? {
-        val symbol = callableSignature.symbol
+    private fun getActualQualifier(symbol: KaCallableSymbol): String? {
         return if (symbol is KaConstructorSymbol) {
             symbol.containingClassId?.asFqNameString()
         } else {
@@ -201,9 +247,9 @@ class FunMatcherImpl(
             }
         }
 
-    private fun checkSubType(callableSignature: KaCallableSignature<*>): Boolean = withKaSession {
-        if (callableSignature.symbol is KaConstructorSymbol) return false
-        return callableSignature.symbol.allOverriddenSymbols.any {
+    private fun checkSubType(symbol: KaCallableSymbol): Boolean = withKaSession {
+        if (symbol is KaConstructorSymbol) return false
+        return symbol.allOverriddenSymbols.any {
             val className: String? = it.callableId?.asSingleFqName()?.parent()?.asString()
             definingSupertypes.contains(className)
         }
@@ -218,9 +264,9 @@ class FunMatcherImpl(
             (nameRegex == null && names.isEmpty()) || name in names || (nameRegex != null && nameRegex.matches(name))
         } else false
 
-    private fun checkName(callableSignature: KaCallableSignature<*>): Boolean {
-        if (matchConstructor) return callableSignature.symbol is KaConstructorSymbol
-        val name = callableSignature.symbol.name?.asString() ?: return false
+    private fun checkName(symbol: KaCallableSymbol): Boolean {
+        if (matchConstructor) return symbol is KaConstructorSymbol
+        val name = symbol.name?.asString() ?: return false
         return (nameRegex == null && names.isEmpty()) || name in names || (nameRegex != null && nameRegex.matches(name))
     }
 
@@ -265,6 +311,11 @@ class FunMatcherImpl(
             else null
         } ?: true
 
+    /**
+     * Note that [KaCallableSignature] carries use-site type information,
+     * so for `fun <T> example(): T` matcher with [returnType] `"kotlin.String"`
+     * matches calls of `example<String>()`.
+     */
     private fun checkReturnType(callableSignature: KaCallableSignature<*>) =
        returnType?.let {
            it == callableSignature.returnType.asFqNameString()

sonar-kotlin-checks/src/main/java/org/sonarsource/kotlin/checks/UnpredictableHashSaltCheck.kt

@@ -16,24 +16,24 @@
  */
 package org.sonarsource.kotlin.checks
 
+import org.jetbrains.kotlin.analysis.api.resolution.KaFunctionCall
+import org.jetbrains.kotlin.analysis.api.resolution.successfulFunctionCallOrNull
 import org.jetbrains.kotlin.psi.KtCallExpression
-import org.jetbrains.kotlin.resolve.BindingContext
-import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
-import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
 import org.sonar.check.Rule
 import org.sonarsource.kotlin.api.checks.BYTE_ARRAY_CONSTRUCTOR
 import org.sonarsource.kotlin.api.checks.BYTE_ARRAY_CONSTRUCTOR_SIZE_ARG_ONLY
 import org.sonarsource.kotlin.api.checks.CallAbstractCheck
 import org.sonarsource.kotlin.api.checks.ConstructorMatcher
 import org.sonarsource.kotlin.api.checks.FunMatcherImpl
+import org.sonarsource.kotlin.api.checks.getFirstArgumentExpression
 import org.sonarsource.kotlin.api.checks.isBytesInitializedFromString
 import org.sonarsource.kotlin.api.checks.isInitializedPredictably
 import org.sonarsource.kotlin.api.checks.matches
 import org.sonarsource.kotlin.api.checks.predictRuntimeIntValue
 import org.sonarsource.kotlin.api.checks.predictRuntimeValueExpression
-import org.sonarsource.kotlin.api.checks.simpleArgExpressionOrNull
 import org.sonarsource.kotlin.api.frontend.KotlinFileContext
 import org.sonarsource.kotlin.api.frontend.secondaryOf
+import org.sonarsource.kotlin.api.visiting.withKaSession
 
 private const val SPECS_PACKAGE = "javax.crypto.spec"
 private const val KEY_SPEC_FUN_NAME = "PBEKeySpec"
@@ -49,31 +49,28 @@ private val matcherSaltIndexMap = mapOf(
     ConstructorMatcher("$SPECS_PACKAGE.$PARAMETER_SPEC_FUN_NAME") to 0,
 )
 
-@org.sonarsource.kotlin.api.frontend.K1only
 @Rule(key = "S2053")
 class UnpredictableHashSaltCheck : CallAbstractCheck() {
     override val functionsToVisit = matcherSaltIndexMap.keys
 
     override fun visitFunctionCall(
         callExpression: KtCallExpression,
-        resolvedCall: ResolvedCall<*>,
+        resolvedCall: KaFunctionCall<*>,
         matchedFun: FunMatcherImpl,
         kotlinFileContext: KotlinFileContext
-    ) {
+    ) = withKaSession {
         val saltArgIndex = matcherSaltIndexMap[matchedFun]!!
 
         if (callExpression.valueArguments.size < 2) {
             kotlinFileContext.reportIssue(callExpression, MSG_ADD_SALT)
             return
         }
 
-        val bindingContext = kotlinFileContext.bindingContext
-        val saltArg = resolvedCall.valueArgumentsByIndex?.elementAtOrNull(saltArgIndex)
-            ?.arguments?.elementAtOrNull(0)?.getArgumentExpression() ?: return
+        val saltArg = resolvedCall.argumentMapping.keys.elementAtOrNull(saltArgIndex) ?: return
 
-        val predictedSaltValue = saltArg.predictRuntimeValueExpression(bindingContext)
+        val predictedSaltValue = saltArg.predictRuntimeValueExpression()
 
-        if (predictedSaltValue.isBytesInitializedFromString(bindingContext)) {
+        if (predictedSaltValue.isBytesInitializedFromString()) {
             kotlinFileContext.reportIssue(
                 saltArg,
                 MSG_MAKE_UNPREDICTABLE,
@@ -82,15 +79,15 @@ class UnpredictableHashSaltCheck : CallAbstractCheck() {
             return
         }
 
-        val saltInitializer = predictedSaltValue.getResolvedCall(bindingContext)
+        val saltInitializer = predictedSaltValue.resolveToCall()?.successfulFunctionCallOrNull()
             ?.takeIf { it matches BYTE_ARRAY_CONSTRUCTOR } ?: return
 
-        if (saltInitializer.byteArrayInitSizeTooSmall(bindingContext)) {
+        if (saltInitializer.byteArrayInitSizeTooSmall()) {
             kotlinFileContext.reportIssue(saltArg, MSG_MIN_LEN, listOf(kotlinFileContext.secondaryOf(predictedSaltValue)))
         }
 
         if (saltInitializer matches BYTE_ARRAY_CONSTRUCTOR_SIZE_ARG_ONLY &&
-            saltArg.isInitializedPredictably(predictedSaltValue, bindingContext)
+            saltArg.isInitializedPredictably(predictedSaltValue)
         ) {
             kotlinFileContext.reportIssue(
                 saltArg,
@@ -103,6 +100,6 @@ class UnpredictableHashSaltCheck : CallAbstractCheck() {
     /**
      * Checks whether the first argument of the call is an integer that is at least 16
      */
-    private fun ResolvedCall<*>.byteArrayInitSizeTooSmall(bindingContext: BindingContext) =
-        (this.simpleArgExpressionOrNull(0)?.predictRuntimeIntValue(bindingContext) ?: 16) < 16
+    private fun KaFunctionCall<*>.byteArrayInitSizeTooSmall() =
+        (getFirstArgumentExpression()?.predictRuntimeIntValue() ?: 16) < 16
 }