From 66b5fa9216577bfe86fb29e83b031a3cde10cc08 Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov <138671+Godin@users.noreply.github.com> Date: Sun, 21 Feb 2021 19:41:48 +0100 Subject: [PATCH] Improve filter for Kotlin when-expressions with String (#1156) --- .../targets/KotlinWhenExpressionTarget.kt | 16 ++++ .../filter/KotlinWhenStringFilterTest.java | 76 +++++++++++++++++++ .../filter/KotlinWhenStringFilter.java | 26 ++++--- org.jacoco.doc/docroot/doc/changes.html | 3 + 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 2cb73f2e8a..882bac63af 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) { 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 a61f3f8df9..f3a548c552 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 void should_filter() { assertIgnored(new Range(expectedFromInclusive, expectedToInclusive)); } + /** + * 
+	 * fun example(p: String) {
+	 *   when (p) {
+	 *     "b" -> return
+	 *     "a" -> return
+	 *     "\u0000a" -> return
+	 *   }
+	 * }
+	 *
+ */ + @Test + public void should_filter_when_biggest_hashCode_first() { + final Set expectedNewTargets = new HashSet(); + + 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 37d394f978..5fd208742b 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 void match(final AbstractInsnNode start, final Set replacements = new HashSet(); 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 void match(final AbstractInsnNode start, // 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 30369bd88c..58be8389dd 100644 --- a/org.jacoco.doc/docroot/doc/changes.html +++ b/org.jacoco.doc/docroot/doc/changes.html @@ -33,6 +33,9 @@

New Features

  • Branches added by the Kotlin compiler version 1.4.20 and above for suspending lambdas are filtered out during generation of report (GitHub #1149).
    • +
  • Improved filtering of bytecode generated by Kotlin compiler for + when expressions on kotlin.String values + (GitHub #1156).

    • Non-functional Changes