Do not use !isEmpty methods from the collections – need to use isNotE…

…mpty instead (#704)

* feature/check-isEmpty-method(#692)

### What's done:
  * Added new rule
  * Added warn tests
  * Added fix tests

diktat-analysis.yml

@@ -413,4 +413,7 @@
   enabled: true
 # If there is stateless class it is preferred to use object
 - name: OBJECT_IS_PREFERRED
+  enabled: true
+# If there exists negated version of function you should prefer it instead of !functionCall
+- name: INVERSE_FUNCTION_PREFERRED
   enabled: true

diktat-rules/src/main/kotlin/generated/WarningNames.kt

@@ -229,4 +229,6 @@ public object WarningNames {
     public const val AVOID_USING_UTILITY_CLASS: String = "AVOID_USING_UTILITY_CLASS"
 
     public const val OBJECT_IS_PREFERRED: String = "OBJECT_IS_PREFERRED"
+
+    public const val INVERSE_FUNCTION_PREFERRED: String = "INVERSE_FUNCTION_PREFERRED"
 }

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt

@@ -147,6 +147,7 @@ enum class Warnings(
     NO_CORRESPONDING_PROPERTY(false, "6.1.7", "backing property should have the same name, but there is no corresponding property"),
     AVOID_USING_UTILITY_CLASS(false, "6.4.1", "avoid using utility classes/objects, use extensions functions"),
     OBJECT_IS_PREFERRED(true, "6.4.2", "it is better to use object for stateless classes"),
+    INVERSE_FUNCTION_PREFERRED(true, "5.1.4", "it is better to use inverse function"),
     ;
 
     /**

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/CheckInverseMethodRule.kt

@@ -0,0 +1,73 @@
+package org.cqfn.diktat.ruleset.rules
+
+import org.cqfn.diktat.common.config.rules.RulesConfig
+import org.cqfn.diktat.ruleset.constants.EmitType
+import org.cqfn.diktat.ruleset.constants.Warnings.INVERSE_FUNCTION_PREFERRED
+
+import com.pinterest.ktlint.core.Rule
+import com.pinterest.ktlint.core.ast.ElementType.BLOCK_COMMENT
+import com.pinterest.ktlint.core.ast.ElementType.CALL_EXPRESSION
+import com.pinterest.ktlint.core.ast.ElementType.IDENTIFIER
+import com.pinterest.ktlint.core.ast.ElementType.LPAR
+import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE
+import com.pinterest.ktlint.core.ast.ElementType.REFERENCE_EXPRESSION
+import com.pinterest.ktlint.core.ast.ElementType.RPAR
+import com.pinterest.ktlint.core.ast.ElementType.VALUE_ARGUMENT_LIST
+import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
+import org.jetbrains.kotlin.com.intellij.lang.ASTNode
+import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
+import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
+import org.jetbrains.kotlin.psi.psiUtil.siblings
+
+/**
+ * This rule checks if inverse method can be used.
+ * For example if there is !isEmpty() on collection call that it changes it to isNotEmpty()
+ */
+class CheckInverseMethodRule(private val configRules: List<RulesConfig>) : Rule("inverse-method") {
+    private var isFixMode: Boolean = false
+    private lateinit var emitWarn: EmitType
+
+    override fun visit(node: ASTNode,
+                       autoCorrect: Boolean,
+                       emit: EmitType) {
+        emitWarn = emit
+        isFixMode = autoCorrect
+
+        if (node.elementType == CALL_EXPRESSION && node.text in methodMap.keys) {
+            checkCallExpressionName(node)
+        }
+    }
+
+    private fun checkCallExpressionName(node: ASTNode) {
+        val operationRef = node
+            .treeParent
+            .siblings(forward = false)
+            .takeWhile { it.elementType in intermediateTokens }
+            .firstOrNull { it.elementType == OPERATION_REFERENCE}
+        if (operationRef?.text == "!") {
+            INVERSE_FUNCTION_PREFERRED.warnAndFix(configRules, emitWarn, isFixMode, "${methodMap[node.text]} instead of !${node.text}", node.startOffset, node) {
+                val callExpression = CompositeElement(CALL_EXPRESSION)
+                val referenceExp = CompositeElement(REFERENCE_EXPRESSION)
+                val argList = CompositeElement(VALUE_ARGUMENT_LIST)
+                node.treeParent.addChild(callExpression, node)
+                callExpression.addChild(referenceExp)
+                callExpression.addChild(argList)
+                referenceExp.addChild(LeafPsiElement(IDENTIFIER, "${methodMap[node.text]}".dropLast(2)))
+                argList.addChild(LeafPsiElement(LPAR, "("))
+                argList.addChild(LeafPsiElement(RPAR, ")"))
+                node.treeParent.treeParent.removeChild(node.treeParent.treePrev)  // removing OPERATION_EXPRESSION - !
+                node.treeParent.removeChild(node)
+            }
+        }
+    }
+
+    companion object {
+        val methodMap = mapOf(
+            "isEmpty()" to "isNotEmpty()",
+            "isBlank()" to "isNotBlank()",
+            "isNotEmpty()" to "isEmpty()",
+            "isNotBlank()" to "isBlank()"
+        )
+        val intermediateTokens = listOf(WHITE_SPACE, OPERATION_REFERENCE, BLOCK_COMMENT)
+    }
+}

diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt

@@ -111,6 +111,7 @@ class DiktatRuleSetProvider(private var diktatConfigFile: String = DIKTAT_ANALYS
             ::CustomGetterSetterRule,
             ::CompactInitialization,
             // other rules
+            ::CheckInverseMethodRule,
             ::StatelessClassesRule,
             ::ImplicitBackingPropertyRule,
             ::StringTemplateFormatRule,

diktat-rules/src/main/resources/diktat-analysis-huawei.yml

@@ -413,4 +413,7 @@
   enabled: true
 # If there is stateless class it is preferred to use object
 - name: OBJECT_IS_PREFERRED
+  enabled: true
+# If there exists negated version of function you should prefer it instead of !functionCall
+- name: INVERSE_FUNCTION_PREFERRED
   enabled: true

diktat-rules/src/main/resources/diktat-analysis.yml

@@ -410,4 +410,7 @@
   enabled: true
 # If there is stateless class it is preferred to use object
 - name: OBJECT_IS_PREFERRED
+  enabled: true
+# If there exists negated version of function you should prefer it instead of !functionCall
+- name: INVERSE_FUNCTION_PREFERRED
   enabled: true

diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/CheckInverseMethodRuleFixTest.kt

@@ -0,0 +1,16 @@
+package org.cqfn.diktat.ruleset.chapter5
+
+import org.cqfn.diktat.ruleset.rules.CheckInverseMethodRule
+import org.cqfn.diktat.util.FixTestBase
+
+import generated.WarningNames.INVERSE_FUNCTION_PREFERRED
+import org.junit.jupiter.api.Tag
+import org.junit.jupiter.api.Test
+
+class CheckInverseMethodRuleFixTest : FixTestBase("test/paragraph5/method_call_names", ::CheckInverseMethodRule) {
+    @Test
+    @Tag(INVERSE_FUNCTION_PREFERRED)
+    fun `should fix method calls`() {
+        fixAndCompare("ReplaceMethodCallNamesExpected.kt", "ReplaceMethodCallNamesTest.kt")
+    }
+}

diktat-rules/src/test/kotlin/org/cqfn/diktat/ruleset/chapter5/CheckInverseMethodRuleWarnTest.kt

@@ -0,0 +1,74 @@
+package org.cqfn.diktat.ruleset.chapter5
+
+import org.cqfn.diktat.ruleset.constants.Warnings
+import org.cqfn.diktat.ruleset.rules.CheckInverseMethodRule
+import org.cqfn.diktat.ruleset.rules.DIKTAT_RULE_SET_ID
+import org.cqfn.diktat.util.LintTestBase
+
+import com.pinterest.ktlint.core.LintError
+import generated.WarningNames.INVERSE_FUNCTION_PREFERRED
+import org.junit.jupiter.api.Tag
+import org.junit.jupiter.api.Test
+
+class CheckInverseMethodRuleWarnTest : LintTestBase(::CheckInverseMethodRule) {
+    private val ruleId = "$DIKTAT_RULE_SET_ID:inverse-method"
+
+    @Test
+    @Tag(INVERSE_FUNCTION_PREFERRED)
+    fun `should not raise warning`() {
+        lintMethod(
+            """
+                    |fun some() {
+                    |   if (list.isEmpty()) {
+                    |       // some cool logic 
+                    |   }
+                    |}
+            """.trimMargin()
+        )
+    }
+
+    @Test
+    @Tag(INVERSE_FUNCTION_PREFERRED)
+    fun `should raise warning`() {
+        lintMethod(
+            """
+                    |fun some() {
+                    |   if (!list.isEmpty()) {
+                    |       // some cool logic 
+                    |   }
+                    |}
+            """.trimMargin(),
+            LintError(2, 14, ruleId, "${Warnings.INVERSE_FUNCTION_PREFERRED.warnText()} isNotEmpty() instead of !isEmpty()", true)
+        )
+    }
+
+    @Test
+    @Tag(INVERSE_FUNCTION_PREFERRED)
+    fun `should consider white spaces between ! and call expression`() {
+        lintMethod(
+            """
+                    |fun some() {
+                    |   if (!  list.isEmpty()) {
+                    |       // some cool logic 
+                    |   }
+                    |}
+            """.trimMargin(),
+            LintError(2, 16, ruleId, "${Warnings.INVERSE_FUNCTION_PREFERRED.warnText()} isNotEmpty() instead of !isEmpty()", true)
+        )
+    }
+
+    @Test
+    @Tag(INVERSE_FUNCTION_PREFERRED)
+    fun `should consider comments between ! and call expression`() {
+        lintMethod(
+            """
+                    |fun some() {
+                    |   if (! /*cool comment*/ list.isEmpty()) {
+                    |       // some cool logic 
+                    |   }
+                    |}
+            """.trimMargin(),
+            LintError(2, 32, ruleId, "${Warnings.INVERSE_FUNCTION_PREFERRED.warnText()} isNotEmpty() instead of !isEmpty()", true)
+        )
+    }
+}

diktat-rules/src/test/resources/test/paragraph5/method_call_names/ReplaceMethodCallNamesExpected.kt

@@ -0,0 +1,25 @@
+package test.chapter6.method_call_names
+
+fun coolFunction() {
+    val list = listOf(1, 2, 3)
+    val testStr = ""
+
+    if (list.isEmpty()) { }
+
+    if (list.isNotEmpty()) { }
+
+    if (list.isNotEmpty()) { }
+
+    if (list.isEmpty()) { }
+
+    if (testStr.isBlank()) { }
+
+    if (testStr.isNotBlank()) { }
+
+    if (testStr.isNotBlank()) { }
+
+    if (testStr.isBlank()) { }
+
+    if (list.isNotEmpty() && testStr.isBlank()) { }
+}
+

diktat-rules/src/test/resources/test/paragraph5/method_call_names/ReplaceMethodCallNamesTest.kt

@@ -0,0 +1,25 @@
+package test.chapter6.method_call_names
+
+fun coolFunction() {
+    val list = listOf(1, 2, 3)
+    val testStr = ""
+
+    if (list.isEmpty()) { }
+
+    if (list.isNotEmpty()) { }
+
+    if (!list.isEmpty()) { }
+
+    if (!list.isNotEmpty()) { }
+
+    if (testStr.isBlank()) { }
+
+    if (testStr.isNotBlank()) { }
+
+    if (!testStr.isBlank()) { }
+
+    if (!testStr.isNotBlank()) { }
+
+    if (!list.isEmpty() && !testStr.isNotBlank()) { }
+}
+

info/available-rules.md

@@ -91,6 +91,7 @@
 |    5  |     5.1.1  |  TOO_LONG_FUNCTION                       |  Check: if length of function is too long                                                                                                                                      |   no  |  maxFunctionLength isIncludeHeader |
 |    5  |     5.1.2  |  NESTED_BLOCK                            |  Check if function has more nested blocks than expected                                                                                                                        |   no  |  maxNestedBlockQuantit|
 |    5  |     5.1.3  |  AVOID_NESTED_FUNCTIONS                  |  Check: if there are any nested functions<br>Fix: declare function in the outer scope                                                                                          |  yes  |  no  |  + |
+|    5  |     5.1.4  |  INVERSE_FUNCTION_PREFERRED              |  Check: if function call with "!" can be rewritten to the inverse function (!isEmpty() -> isNotEmpty())<br>Fix: Rewrites function call                                         |  yes  | - | - |
 |    5  |     5.2.1  |  LAMBDA_IS_NOT_LAST_PARAMETER            |  Checks that lambda inside function parameters isn't in the end                                                                                                                |  no   |  no |
 |    5  |     5.2.2  |  TOO_MANY_PARAMETERS                     |  Check: if function contains more parameters than allowed                                                                                                                      |   no  |  maxParameterListSize |
 |    5  |     5.2.3  |  WRONG_OVERLOADING_FUNCTION_ARGUMENTS    |  Check: function has overloading instead use default arguments                                                                                                                 |   no  |  no |

info/guide/diktat-coding-convention.md

@@ -1885,6 +1885,30 @@ fun foo() {
     println("Nested Output: ${nested()}") 
 } 
 ```  
+#### <a name="r5.1.4"></a> 5.1.4 Negated function calls
+Don't use negated function calls if it can be replaced with negated version of this function
+
+**Invalid example**:
+```kotlin
+fun foo() { 
+    val list = listOf(1, 2, 3)
+
+    if (!list.isEmpty()) {
+        // Some cool logic
+    }
+} 
+``` 
+
+**Valid example**:
+```kotlin
+fun foo() { 
+    val list = listOf(1, 2, 3)
+
+    if (list.isNotEmpty()) {
+        // Some cool logic
+    }
+} 
+``` 
 
 <!-- =============================================================================== -->
 ### <a name="c5.2"></a> 5.2 Function arguments

info/guide/guide-TOC.md

@@ -82,7 +82,8 @@ I [Preface](#c0)
 * [5.1 Function design](#c5.1)       
     * [5.1.1 Avoid functions that are too long ](#r5.1.1)
     * [5.1.2 Avoid deep nesting of function code blocks, limiting to four levels](#r5.1.2) 
-    * [5.1.3 Avoid using nested functions](#r5.1.3)        
+    * [5.1.3 Avoid using nested functions](#r5.1.3)
+    * [5.1.4 Negated function calls](#r5.1.4)        
 * [5.2 Function arguments](#c5.2)              
     * [5.2.1 The lambda parameter of the function should be placed at the end of the argument list](#r5.2.1)
     * [5.2.2 Number of function parameters should be limited to five](#r5.2.2) 

info/guide/guide-chapter-5.md

@@ -59,6 +59,30 @@ fun foo() {
     println("Nested Output: ${nested()}") 
 } 
 ```  
+#### <a name="r5.1.4"></a> 5.1.4 Negated function calls
+Don't use negated function calls if it can be replaced with negated version of this function
+
+**Invalid example**:
+```kotlin
+fun foo() { 
+    val list = listOf(1, 2, 3)
+
+    if (!list.isEmpty()) {
+        // Some cool logic
+    }
+} 
+``` 
+
+**Valid example**:
+```kotlin
+fun foo() { 
+    val list = listOf(1, 2, 3)
+
+    if (list.isNotEmpty()) {
+        // Some cool logic
+    }
+} 
+``` 
 
 <!-- =============================================================================== -->
 ### <a name="c5.2"></a> 5.2 Function arguments