Merge pull request #686 from jborgers/pmd7-issue-651

Pmd7 issue 651: false positive on AvoidMultipleConcatStatements

Author: jborgers

src/main/resources/category/kotlin/common.xml

@@ -398,21 +398,31 @@ class Foo {
             <property name="tag" value="jpinpoint-rule" type="String" description="for-sonar"/>
             <property name="xpath">
                 <value><![CDATA[
-//FunctionBody[count(.//Assignment[.//(AssignmentAndOperator/T-ADD_ASSIGNMENT|AdditiveOperator/T-ADD)]/(AssignableExpression|DirectlyAssignableExpression)//T-Identifier[@Text=
-(: property a string literal :)
-ancestor::FunctionBody//PropertyDeclaration[(
-    Expression[not(.//Expression)]//StringLiteral or
-    (: or explicit type String:)
-    VariableDeclaration/Type//T-Identifier[@Text='String'] or
-    (: or assigned parameter of type String :)
-    Expression[.//T-Identifier[@Text = ancestor::FunctionDeclaration/FunctionValueParameters/FunctionValueParameter[Parameter/Type//T-Identifier[@Text='String']]//T-Identifier/@Text]] or
-    (: or assigned non-nested function of explicit type String :) (: known issue: ignores parameters, may give false positives with overloaded methods :)
-    Expression[.//T-Identifier[not(ancestor::CallSuffix)][@Text = ancestor::ClassMemberDeclarations//FunctionDeclaration[Type//T-Identifier[@Text='String']]//T-Identifier/@Text]]
-)]
-/VariableDeclaration//T-Identifier/@Text]) > 1]
+//FunctionBody[
+  count(
+    .//Assignment[
+      .//(AssignmentAndOperator/T-ADD_ASSIGNMENT|AdditiveOperator/T-ADD)
+      and
+      (AssignableExpression|DirectlyAssignableExpression)//T-Identifier[
+        (: property a string literal :)
+        @Text = ancestor::FunctionBody//PropertyDeclaration[
+          (
+            Expression[not(.//Expression)]//StringLiteral
+            (: explicit type String:)
+            or VariableDeclaration/Type//T-Identifier[@Text='String']
+            (: assigned parameter of type String :)
+            or Expression[.//T-Identifier[@Text = ancestor::FunctionDeclaration/FunctionValueParameters/FunctionValueParameter[Parameter/Type//T-Identifier[@Text='String']]//T-Identifier/@Text]]
+            (: assigned non-nested function of explicit type String :) (: known issue: ignores parameters, may give false positives with overloaded methods :)
+            or Expression[.//T-Identifier[not(ancestor::CallSuffix)][@Text = ancestor::ClassMemberDeclarations//FunctionDeclaration[Type//T-Identifier[@Text='String']]//T-Identifier/@Text]]
+          )
+          and not(Expression//T-Identifier[@Text='toMutableList'])
+        ]/VariableDeclaration//T-Identifier/@Text
+      ]
+    ]
+  ) > 1
+]
 //Statement[Assignment//(T-ADD_ASSIGNMENT|T-ADD)][position()=last()]
-
-	]]></value>
+                ]]></value>
             </property>
         </properties>
         <example>

src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidMultipleConcatStatements.xml

@@ -203,4 +203,101 @@ class AvoidMultipleConcatStatementsKt {
         ]]></code>
     </test-code>
 
+    <test-code>
+        <description>Fix Request: Kotlin AvoidMultipleConcatStatements false positive on MutableList #651</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+class Foo(private val isActive: Boolean, private val canCreate: Boolean) {
+
+    private fun calculate(myList: List<String>): List<String> {
+
+        val mutableList = myList.toMutableList()
+
+        if (isActive) {
+            mutableList += "one" // false positive, not two lines with += are needed to trigger this rule
+            if (canCreate) {
+                mutableList += listOf("two", "three") // false positive
+            }
+        } else {
+            mutableList.removeAll(listOf("one", "two"))
+        }
+        return myList
+    }
+}
+     ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Kotlin non-String plus operator usages should not trigger AvoidMultipleConcatStatements</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.math.BigDecimal
+
+data class Vec2(val x: Int, val y: Int) {
+    operator fun plus(other: Vec2): Vec2 = Vec2(this.x + other.x, this.y + other.y)
+}
+
+class NonStringPlusUsages {
+
+    fun numericPlus(): Int {
+        var a = 10
+        a += 5
+        a = a + 2
+        val b = 3 + 4
+        return a + b
+    }
+
+    fun bigDecimalPlus(): BigDecimal {
+        var total = BigDecimal.ZERO
+        total += BigDecimal("1.5")
+        total = total + BigDecimal("2.25")
+        return total + BigDecimal("3.0")
+    }
+
+    fun listPlus(x: List<Int>, y: List<Int>): List<Int> {
+        var base = listOf(1, 2)
+        base = base + 3
+        base += listOf(4, 5)
+        val more = base + listOf(6)
+        val evenMore = x + y
+        return more
+    }
+
+    fun mutableListPlusAssign(): MutableList<Int> {
+        val ml = mutableListOf(1)
+        ml += 2
+        ml += listOf(3, 4)
+        return ml
+    }
+
+    fun setUnion(): Set<Int> {
+        val s1 = setOf(1, 2)
+        val s2 = setOf(2, 3)
+        val s3 = s1 + s2
+        return s3 + 4
+    }
+
+    fun mapPlus(): Map<String, Int> {
+        val m1 = mapOf("a" to 1)
+        val m2 = m1 + ("b" to 2)
+        val m3 = m2 + mapOf("c" to 3)
+        return m3
+    }
+
+    fun arrayPlus(): IntArray {
+        val arr = intArrayOf(1, 2)
+        val extended = arr + 3
+        return extended + intArrayOf(4, 5)
+    }
+
+    fun customOperatorPlus(): Vec2 {
+        var v = Vec2(1, 2)
+        v += Vec2(3, 4)
+        v = v + Vec2(5, 6)
+        return v + Vec2(7, 8)
+    }
+}
+    ]]></code>
+    </test-code>
+
 </test-data>