Fixed false positive findings [PARAMETER_NAME_IN_OUTER_LAMBDA] (#1891)

* ### What's done: - Fixed false positive findings when lambda operation at the same level - Added warn tests Closes #1880
saveourtool · Dec 27, 2023 · 435e499 · 435e499
1 parent 8636247
commit 435e499
Show file tree

Hide file tree

Showing 6 changed files with 189 additions and 29 deletions.
diff --git a/diktat-analysis.yml b/diktat-analysis.yml
@@ -451,6 +451,8 @@
 # Checks that outer lambda has explicit parameter name
 - name: PARAMETER_NAME_IN_OUTER_LAMBDA
   enabled: true
+  configuration:
+    strictMode: true # don't let outer lambdas have `it` as parameter
 # Checks that property in constructor doesn't contain comment
 - name: KDOC_NO_CONSTRUCTOR_PROPERTY
   enabled: true

diff --git a/...in/kotlin/com/saveourtool/diktat/ruleset/rules/chapter5/ParameterNameInOuterLambdaRule.kt b/...in/kotlin/com/saveourtool/diktat/ruleset/rules/chapter5/ParameterNameInOuterLambdaRule.kt
@@ -1,10 +1,15 @@
 package com.saveourtool.diktat.ruleset.rules.chapter5
 
+import com.saveourtool.diktat.common.config.rules.RuleConfiguration
 import com.saveourtool.diktat.common.config.rules.RulesConfig
+import com.saveourtool.diktat.common.config.rules.getRuleConfig
 import com.saveourtool.diktat.ruleset.constants.Warnings.PARAMETER_NAME_IN_OUTER_LAMBDA
 import com.saveourtool.diktat.ruleset.rules.DiktatRule
 import com.saveourtool.diktat.ruleset.utils.doesLambdaContainIt
 import com.saveourtool.diktat.ruleset.utils.findAllDescendantsWithSpecificType
+import com.saveourtool.diktat.ruleset.utils.hasExplicitIt
+import com.saveourtool.diktat.ruleset.utils.hasItInLambda
+import com.saveourtool.diktat.ruleset.utils.hasNoParameters
 
 import org.jetbrains.kotlin.KtNodeTypes
 import org.jetbrains.kotlin.com.intellij.lang.ASTNode
@@ -17,16 +22,34 @@ class ParameterNameInOuterLambdaRule(configRules: List<RulesConfig>) : DiktatRul
     configRules,
     listOf(PARAMETER_NAME_IN_OUTER_LAMBDA)
 ) {
+    /**
+     * Configuration for the rule ParameterNameInOuterLambda
+     */
+    private val configuration by lazy {
+        ParameterNameInOuterLambdaConfiguration(
+            configRules.getRuleConfig(PARAMETER_NAME_IN_OUTER_LAMBDA)?.configuration
+                ?: emptyMap())
+    }
+
     override fun logic(node: ASTNode) {
         if (node.elementType == KtNodeTypes.LAMBDA_EXPRESSION) {
             checkLambda(node)
         }
     }
 
     private fun checkLambda(node: ASTNode) {
-        val hasInnerLambda = node
-            .findAllDescendantsWithSpecificType(KtNodeTypes.LAMBDA_EXPRESSION, false)
-            .isNotEmpty()
+        val strictMode = configuration.strictMode
+
+        val innerLambdaList = node.findAllDescendantsWithSpecificType(KtNodeTypes.LAMBDA_EXPRESSION, false)
+        val hasInnerLambda = innerLambdaList.isNotEmpty()
+
+        val outerLambdaHasNoParameterName = hasNoParameters(node) || node.hasExplicitIt()
+        val innerLambdasHasNoIt = !hasInnerLambda ||
+                innerLambdaList.all { innerLambda -> !hasItInLambda(innerLambda) }
+
+        if (!strictMode && outerLambdaHasNoParameterName && innerLambdasHasNoIt) {
+            return
+        }
         if (hasInnerLambda && doesLambdaContainIt(node)) {
             PARAMETER_NAME_IN_OUTER_LAMBDA.warn(
                 configRules, emitWarn,
@@ -35,6 +58,19 @@ class ParameterNameInOuterLambdaRule(configRules: List<RulesConfig>) : DiktatRul
             )
         }
     }
+
+    /**
+     * ParameterNameInOuterLambdaConfiguration used when we need to allow the usage of 'it' in outer lambda
+     *
+     * @param config - map of strings with configuration options for a Parameter Name In Outer Lambda rule
+     */
+    class ParameterNameInOuterLambdaConfiguration(config: Map<String, String>) : RuleConfiguration(config) {
+        /**
+         * Flag (when false) allows to use `it` in outer lambda, if in inner lambdas would be no `it`
+         */
+        val strictMode = config["strictMode"]?.toBoolean() ?: true
+    }
+
     companion object {
         const val NAME_ID = "parameter-name-in-outer-lambda"
     }

diff --git a/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/utils/AstNodeUtils.kt b/diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/utils/AstNodeUtils.kt
@@ -1014,6 +1014,22 @@ fun ASTNode.isPrivate(): Boolean = this.getFirstChildWithType(MODIFIER_LIST)?.ge
  */
 fun ASTNode.hasSetterOrGetter(): Boolean = this.getFirstChildWithType(KtNodeTypes.PROPERTY_ACCESSOR) != null
 
+/**
+ * Checks if ASTNode has parameter `it`
+ *
+ * @return true if this ASTNode has parameter `it`
+ */
+fun ASTNode.hasExplicitIt(): Boolean {
+    require(elementType == LAMBDA_EXPRESSION) { "Method can be called only for lambda" }
+    val parameterList = findChildByType(FUNCTION_LITERAL)
+        ?.findChildByType(VALUE_PARAMETER_LIST)
+        ?.psi
+            as KtParameterList?
+    return parameterList?.parameters
+        ?.any { it.name == "it" }
+        ?: false
+}
+
 private fun ASTNode.isFollowedByNewlineCheck() =
     this.treeNext.elementType == WHITE_SPACE && this.treeNext.text.contains("\n")
 
@@ -1115,17 +1131,6 @@ private fun ASTNode.calculateLineNumber() = getRootNode()
         index + 1
     }
 
-private fun ASTNode.hasExplicitIt(): Boolean {
-    require(elementType == LAMBDA_EXPRESSION) { "Method can be called only for lambda" }
-    val parameterList = findChildByType(FUNCTION_LITERAL)
-        ?.findChildByType(VALUE_PARAMETER_LIST)
-        ?.psi
-            as KtParameterList?
-    return parameterList?.parameters
-        ?.any { it.name == "it" }
-        ?: false
-}
-
 /**
  * Gets list of property nodes
  *
@@ -1163,14 +1168,10 @@ fun countCodeLines(node: ASTNode): Int {
 /**
  * Check that lambda contains `it`.
  *
- * Note: it checks only provided lambda and inner lambda with explicit parameters
- *
- * Note: this method can be called only for lambda
+ * @param lambdaNode ASTNode with type LAMBDA_EXPRESSION
+ * @return true if `it` contains in lambdaNode.text
  */
-@Suppress("FUNCTION_BOOLEAN_PREFIX")
-fun doesLambdaContainIt(lambdaNode: ASTNode): Boolean {
-    require(lambdaNode.elementType == LAMBDA_EXPRESSION) { "Method can be called only for lambda" }
-
+fun hasItInLambda(lambdaNode: ASTNode): Boolean {
     val excludeChildrenCondition = { node: ASTNode ->
         node.elementType == LAMBDA_EXPRESSION && (hasNoParameters(node) || node.hasExplicitIt())
     }
@@ -1181,7 +1182,30 @@ fun doesLambdaContainIt(lambdaNode: ASTNode): Boolean {
         .map { it.text }
         .contains("it")
 
-    return hasNoParameters(lambdaNode) && hasIt
+    return hasIt
+}
+
+/**
+ * @param lambdaNode
+ * @return true if lambdaNode has no parameters and has `it` in lambdaNode.text
+ */
+@Suppress("FUNCTION_BOOLEAN_PREFIX")
+fun doesLambdaContainIt(lambdaNode: ASTNode): Boolean {
+    require(lambdaNode.elementType == LAMBDA_EXPRESSION) { "Method can be called only for lambda" }
+    return hasNoParameters(lambdaNode) && hasItInLambda(lambdaNode)
+}
+
+/**
+ * Checks that lambda has no parameters
+ *
+ * @param lambdaNode ASTNode with type LAMBDA_EXPRESSION
+ * @return true if node has no parameters
+ */
+fun hasNoParameters(lambdaNode: ASTNode): Boolean {
+    require(lambdaNode.elementType == LAMBDA_EXPRESSION) { "Method can be called only for lambda" }
+    return null == lambdaNode
+        .findChildByType(FUNCTION_LITERAL)
+        ?.findChildByType(VALUE_PARAMETER_LIST)
 }
 
 /**
@@ -1254,10 +1278,3 @@ private fun hasAnySuppressorForInspection(
 
     foundSuppress || foundIgnoredAnnotation || isCompletelyIgnoredBlock
 }
-
-private fun hasNoParameters(lambdaNode: ASTNode): Boolean {
-    require(lambdaNode.elementType == LAMBDA_EXPRESSION) { "Method can be called only for lambda" }
-    return null == lambdaNode
-        .findChildByType(FUNCTION_LITERAL)
-        ?.findChildByType(VALUE_PARAMETER_LIST)
-}
diff --git a/diktat-rules/src/main/resources/diktat-analysis-huawei.yml b/diktat-rules/src/main/resources/diktat-analysis-huawei.yml
@@ -448,6 +448,8 @@
 # Check that lambda with inner lambda doesn't use implicit parameter
 - name: PARAMETER_NAME_IN_OUTER_LAMBDA
   enabled: true
+  configuration:
+    strictMode: true # don't let outer lambdas have `it` as parameter
 # Checks that property in constructor doesn't contains comment
 - name: KDOC_NO_CONSTRUCTOR_PROPERTY
   enabled: true

diff --git a/diktat-rules/src/main/resources/diktat-analysis.yml b/diktat-rules/src/main/resources/diktat-analysis.yml
@@ -455,6 +455,8 @@
 # Check that lambda with inner lambda doesn't use implicit parameter
 - name: PARAMETER_NAME_IN_OUTER_LAMBDA
   enabled: true
+  configuration:
+    strictMode: true # don't let outer lambdas have `it` as parameter
 # Checks that property in KDoc present in class
 - name: KDOC_EXTRA_PROPERTY
   enabled: true

diff --git a/.../kotlin/com/saveourtool/diktat/ruleset/chapter5/ParameterNameInOuterLambdaRuleWarnTest.kt b/.../kotlin/com/saveourtool/diktat/ruleset/chapter5/ParameterNameInOuterLambdaRuleWarnTest.kt
@@ -16,6 +16,12 @@ class ParameterNameInOuterLambdaRuleWarnTest : LintTestBase(::ParameterNameInOut
     private val rulesConfigList: List<RulesConfig> = listOf(
         RulesConfig(Warnings.PARAMETER_NAME_IN_OUTER_LAMBDA.name, true)
     )
+    private val rulesConfigParameterNameInOuterLambda = listOf(
+        RulesConfig(
+            Warnings.PARAMETER_NAME_IN_OUTER_LAMBDA.name, true, mapOf(
+            "strictMode" to "false"
+        ))
+    )
 
     @Test
     @Tag(WarningNames.PARAMETER_NAME_IN_OUTER_LAMBDA)
@@ -197,4 +203,99 @@ class ParameterNameInOuterLambdaRuleWarnTest : LintTestBase(::ParameterNameInOut
             rulesConfigList = rulesConfigList
         )
     }
+
+    @Test
+    @Tag(WarningNames.PARAMETER_NAME_IN_OUTER_LAMBDA)
+    fun `lambdas at the same level`() {
+        lintMethod(
+            """
+                |fun testA() {
+                |   val overrideFunctions: List <Int> = emptyList()
+                |   overrideFunctions.forEach { functionNameMap.compute(it.getIdentifierName()!!.text) { _, oldValue -> (oldValue ?: 0) + 1 } }
+                |}
+                """.trimMargin(),
+            rulesConfigList = this.rulesConfigParameterNameInOuterLambda
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.PARAMETER_NAME_IN_OUTER_LAMBDA)
+    fun `lambdas at the same level 2`() {
+        lintMethod(
+            """
+                |private fun isCheckNeeded(whiteSpace: PsiWhiteSpace) =
+                |    whiteSpace.parent
+                |        .node
+                |        .elementType
+                |        .let { it == VALUE_PARAMETER_LIST || it == VALUE_ARGUMENT_LIST } &&
+                |            whiteSpace.siblings(forward = false, withItself = false).none { it is PsiWhiteSpace && it.textContains('\n') } &&
+                |            whiteSpace.siblings(forward = true, withItself = false).any {
+                |                it.node.elementType.run { this == VALUE_ARGUMENT || this == VALUE_PARAMETER }
+                |            }
+                """.trimMargin(),
+            rulesConfigList = this.rulesConfigParameterNameInOuterLambda
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.PARAMETER_NAME_IN_OUTER_LAMBDA)
+    fun `lambdas at the same level 21`() {
+        lintMethod(
+            """
+                |private fun isCheckNeeded(w: PsiWhiteSpace, h: PsiWhiteSpace) =
+                |    w.let { it == VALUE_PARAMETER_LIST || it == VALUE_ARGUMENT_LIST } &&
+                |            h.none { it is PsiWhiteSpace && it.textContains('\n') } &&
+                |            h.any {
+                |                it.node.elementType.run { this == VALUE_ARGUMENT || this == VALUE_PARAMETER }
+                |            }
+                """.trimMargin(),
+            rulesConfigList = this.rulesConfigParameterNameInOuterLambda
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.PARAMETER_NAME_IN_OUTER_LAMBDA)
+    fun `lambdas at the same level 3`() {
+        lintMethod(
+            """
+                |private fun checkBlankLineAfterKdoc(node: ASTNode) {
+                |    commentType.forEach {
+                |        val kdoc = node.getFirstChildWithType(it)
+                |        kdoc?.treeNext?.let { nodeAfterKdoc ->
+                |            if (nodeAfterKdoc.elementType == WHITE_SPACE && nodeAfterKdoc.numNewLines() > 1) {
+                |                WRONG_NEWLINES_AROUND_KDOC.warnAndFix(configRules, emitWarn, isFixMode, "redundant blank line after ${'$'}{kdoc.text}", nodeAfterKdoc.startOffset, nodeAfterKdoc) {
+                |                    nodeAfterKdoc.leaveOnlyOneNewLine()
+                |                }
+                |            }
+                |        }
+                |    }
+                |}
+                """.trimMargin(),
+            rulesConfigList = this.rulesConfigParameterNameInOuterLambda
+        )
+    }
+
+    @Test
+    @Tag(WarningNames.PARAMETER_NAME_IN_OUTER_LAMBDA)
+    fun `lambdas at the same level 4`() {
+        lintMethod(
+            """
+                |private fun KSAnnotation.getArgumentValue(argumentName: String): String = arguments
+                |    .singleOrNull { it.name?.asString() == argumentName }
+                |    .let {
+                |        requireNotNull(it) {
+                |            "Not found ${'$'}argumentName in ${'$'}this"
+                |        }
+                |    }
+                |    .value
+                |    ?.let { it as? String }
+                |    .let {
+                |        requireNotNull(it) {
+                |            "Not found a value for ${'$'}argumentName in ${'$'}this"
+                |        }
+                |    }
+                """.trimMargin(),
+            rulesConfigList = this.rulesConfigParameterNameInOuterLambda
+        )
+    }
 }