Improve filter for Kotlin when-expressions with String (#1156)

4 files changed, 111 insertions, 10 deletions

diff --git a/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinWhenExpressionTarget.kt b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinWhenExpressionTarget.kt
index 2cb73f2e..882bac63 100644
--- a/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinWhenExpressionTarget.kt
+++ b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinWhenExpressionTarget.kt
@@ -58,6 +58,17 @@ object KotlinWhenExpressionTarget {
         else -> 5 // assertFullyCovered()
     } // assertFullyCovered()
 
+    /**
+     * Unlike [whenString]
+     * in this example first case is the only case with biggest hashCode value.
+     */
+    private fun whenStringBiggestHashCodeFirst(p: String): Int = when (p) { // assertFullyCovered(0, 4)
+        "b" -> 1 // assertFullyCovered()
+        "a" -> 2 // assertFullyCovered()
+        "\u0000a" -> 3 // assertFullyCovered()
+        else -> 4 // assertFullyCovered()
+    } // assertFullyCovered()
+
     @JvmStatic
     fun main(args: Array<String>) {
         whenSealed(Sealed.Sealed1)
@@ -77,6 +88,11 @@ object KotlinWhenExpressionTarget {
         whenString("b")
         whenString("\u0000a")
         whenString("\u0000b")
+
+        whenStringBiggestHashCodeFirst("")
+        whenStringBiggestHashCodeFirst("a")
+        whenStringBiggestHashCodeFirst("b")
+        whenStringBiggestHashCodeFirst("\u0000a")
     }
 
 }
diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinWhenStringFilterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinWhenStringFilterTest.java
index a61f3f8d..f3a548c5 100644
--- a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinWhenStringFilterTest.java
+++ b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinWhenStringFilterTest.java
@@ -106,6 +106,82 @@ public class KotlinWhenStringFilterTest extends FilterTestBase {
 		assertIgnored(new Range(expectedFromInclusive, expectedToInclusive));
 	}
 
+	/**
+	 * <pre>
+	 * fun example(p: String) {
+	 *   when (p) {
+	 *     "b" -> return
+	 *     "a" -> return
+	 *     "\u0000a" -> return
+	 *   }
+	 * }
+	 * </pre>
+	 */
+	@Test
+	public void should_filter_when_biggest_hashCode_first() {
+		final Set<AbstractInsnNode> expectedNewTargets = new HashSet<AbstractInsnNode>();
+
+		final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0,
+				"example", "(Ljava/lang/String;)V", null, null);
+
+		final Label h1 = new Label();
+		final Label sameHash = new Label();
+		final Label h2 = new Label();
+		final Label case1 = new Label();
+		final Label case2 = new Label();
+		final Label case3 = new Label();
+		final Label defaultCase = new Label();
+
+		m.visitVarInsn(Opcodes.ALOAD, 2);
+		m.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "java/lang/String", "hashCode",
+				"()I", false);
+		m.visitTableSwitchInsn(97, 98, defaultCase, h1, h2);
+
+		m.visitLabel(h1);
+		final AbstractInsnNode expectedFromInclusive = m.instructions.getLast();
+		m.visitVarInsn(Opcodes.ALOAD, 2);
+		m.visitLdcInsn("a");
+		m.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "java/lang/String", "equals",
+				"(Ljava/lang/Object;)Z", false);
+		m.visitJumpInsn(Opcodes.IFEQ, sameHash);
+		m.visitJumpInsn(Opcodes.GOTO, case2);
+
+		m.visitLabel(sameHash);
+		m.visitVarInsn(Opcodes.ALOAD, 2);
+		m.visitLdcInsn("\u0000a");
+		m.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "java/lang/String", "equals",
+				"(Ljava/lang/Object;)Z", false);
+		m.visitJumpInsn(Opcodes.IFEQ, defaultCase);
+		m.visitJumpInsn(Opcodes.GOTO, case3);
+
+		m.visitLabel(h2);
+		m.visitVarInsn(Opcodes.ALOAD, 2);
+		m.visitLdcInsn("b");
+		m.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "java/lang/String", "equals",
+				"(Ljava/lang/Object;)Z", false);
+		m.visitJumpInsn(Opcodes.IFEQ, defaultCase);
+		final AbstractInsnNode expectedToInclusive = m.instructions.getLast();
+
+		m.visitLabel(case1);
+		m.visitInsn(Opcodes.RETURN);
+		expectedNewTargets.add(m.instructions.getLast());
+		m.visitLabel(case2);
+		m.visitInsn(Opcodes.RETURN);
+		expectedNewTargets.add(m.instructions.getLast());
+		m.visitLabel(case3);
+		m.visitInsn(Opcodes.RETURN);
+		expectedNewTargets.add(m.instructions.getLast());
+		m.visitLabel(defaultCase);
+		m.visitInsn(Opcodes.RETURN);
+		expectedNewTargets.add(m.instructions.getLast());
+
+		filter.filter(m, context, output);
+
+		assertIgnored(new Range(expectedFromInclusive, expectedToInclusive));
+		assertReplacedBranches(expectedFromInclusive.getPrevious(),
+				expectedNewTargets);
+	}
+
 	@Test
 	public void should_not_filter_empty_lookup_switch() {
 		final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0,
diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinWhenStringFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinWhenStringFilter.java
index 37d394f9..5fd20874 100644
--- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinWhenStringFilter.java
+++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinWhenStringFilter.java
@@ -74,7 +74,7 @@ public final class KotlinWhenStringFilter implements IFilter {
 			final Set<AbstractInsnNode> replacements = new HashSet<AbstractInsnNode>();
 			replacements.add(skipNonOpcodes(defaultLabel));
 
-			for (int i = 0; i < hashCodes; i++) {
+			for (int i = 1; i <= hashCodes; i++) {
 				while (true) {
 					nextIsVar(Opcodes.ALOAD, "s");
 					nextIs(Opcodes.LDC);
@@ -83,18 +83,24 @@ public final class KotlinWhenStringFilter implements IFilter {
 					// jump to next comparison or default case
 					nextIs(Opcodes.IFEQ);
 					final JumpInsnNode jump = (JumpInsnNode) cursor;
-					// jump to case
-					nextIs(Opcodes.GOTO);
+					next();
 					if (cursor == null) {
 						return;
-					}
-
-					replacements
-							.add(skipNonOpcodes(((JumpInsnNode) cursor).label));
-
-					if (jump.label == defaultLabel) {
-						// end of comparisons for same hashCode
+					} else if (cursor.getOpcode() == Opcodes.GOTO) {
+						// jump to case body
+						replacements.add(
+								skipNonOpcodes(((JumpInsnNode) cursor).label));
+						if (jump.label == defaultLabel) {
+							// end of comparisons for same hashCode
+							break;
+						}
+					} else if (i == hashCodes && jump.label == defaultLabel) {
+						// case body
+						replacements.add(cursor);
+						cursor = jump;
 						break;
+					} else {
+						return;
 					}
 				}
 			}
diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html
index 30369bd8..58be8389 100644
--- a/org.jacoco.doc/docroot/doc/changes.html
+++ b/org.jacoco.doc/docroot/doc/changes.html
@@ -33,6 +33,9 @@
   <li>Branches added by the Kotlin compiler version 1.4.20 and above for suspending
       lambdas are filtered out during generation of report
       (GitHub <a href="https://github.com/jacoco/jacoco/issues/1149">#1149</a>).</li>
+  <li>Improved filtering of bytecode generated by Kotlin compiler for
+      <code>when</code> expressions on <code>kotlin.String</code> values
+      (GitHub <a href="https://github.com/jacoco/jacoco/issues/1156">#1156</a>).</li>
 </ul>
 
 <h3>Non-functional Changes</h3>