diff --git a/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinSafeCallOperatorTarget.kt b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinSafeCallOperatorTarget.kt index 9b40a227e5..176f5d05cd 100644 --- a/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinSafeCallOperatorTarget.kt +++ b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinSafeCallOperatorTarget.kt @@ -12,6 +12,8 @@ *******************************************************************************/ package org.jacoco.core.test.validation.kotlin.targets +import org.jacoco.core.test.validation.targets.Stubs.nop + /** * Test target for [safe call operator (`?.`)](https://kotlinlang.org/docs/null-safety.html#safe-call-operator). */ @@ -52,10 +54,33 @@ object KotlinSafeCallOperatorTarget { fullCoverage(A(B(""))) } + private fun safeCallChainMultiline() { + fun nullOnly(a: A?): String? = + a?.also { // assertPartlyCovered(1, 1) + nop(it) // assertNotCovered() + }?.b?.c // assertPartlyCovered(1, 1) + + fun nonNullOnly(a: A?): String? = + a?.also { // assertPartlyCovered(1, 1) + nop(it) // assertFullyCovered() + }?.b?.c // assertFullyCovered(1, 1) + + fun fullCoverage(a: A?): String? = + a?.also { // assertFullyCovered(0, 2) + nop(it) // assertFullyCovered() + }?.b?.c // assertFullyCovered(0, 2) + + nullOnly(null) + nonNullOnly(A(B(""))) + fullCoverage(null) + fullCoverage(A(B(""))) + } + @JvmStatic fun main(args: Array) { safeCall() safeCallChain() + safeCallChainMultiline() } } diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinSafeCallOperatorFilterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinSafeCallOperatorFilterTest.java index 8c908f9459..3ec105136b 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinSafeCallOperatorFilterTest.java +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinSafeCallOperatorFilterTest.java @@ -41,7 +41,7 @@ public class KotlinSafeCallOperatorFilterTest extends FilterTestBase { * https://github.com/JetBrains/kotlin/commit/0a67ab54fec635f82e0507cbdd4299ae0dbe71b0 */ @Test - public void should_filter_safe_call_chain() { + public void should_filter_optimized_safe_call_chain() { context.classAnnotations .add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC); final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0, @@ -80,4 +80,55 @@ public void should_filter_safe_call_chain() { assertReplacedBranches(expected); } + /** + *
+	 * data class A(val b: B)
+	 * data class B(val c: String)
+	 * fun example(a: A?): String? =
+	 *     a
+	 *         ?.b
+	 *         ?.c
+	 * 
+ */ + @Test + public void should_filter_unoptimized_safe_call_chain() { + context.classAnnotations + .add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC); + final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0, + "example", "(LA;)Ljava/lang/String;", null, null); + m.visitVarInsn(Opcodes.ALOAD, 0); + final Label label1 = new Label(); + final Label label2 = new Label(); + + m.visitJumpInsn(Opcodes.IFNULL, label1); + final AbstractInsnNode i1 = m.instructions.getLast(); + m.visitVarInsn(Opcodes.ALOAD, 0); + m.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "A", "getB", "()LB;", false); + + m.visitVarInsn(Opcodes.ASTORE, 1); + m.visitVarInsn(Opcodes.ALOAD, 1); + m.visitJumpInsn(Opcodes.IFNULL, label1); + final AbstractInsnNode i2 = m.instructions.getLast(); + m.visitVarInsn(Opcodes.ALOAD, 1); + final HashSet r = new HashSet(); + r.add(m.instructions.getLast()); + m.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "B", "getC", + "()Ljava/lang/String;", false); + + m.visitJumpInsn(Opcodes.GOTO, label2); + + m.visitLabel(label1); + m.visitInsn(Opcodes.ACONST_NULL); + r.add(m.instructions.getLast()); + m.visitLabel(label2); + + filter.filter(m, context, output); + + assertIgnored(); + final HashMap> expected = new HashMap>(); + expected.put(i1, r); + expected.put(i2, r); + assertReplacedBranches(expected); + } + } diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinSafeCallOperatorFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinSafeCallOperatorFilter.java index 7297e4e642..50b7173467 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinSafeCallOperatorFilter.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinSafeCallOperatorFilter.java @@ -22,6 +22,7 @@ import org.objectweb.asm.tree.JumpInsnNode; import org.objectweb.asm.tree.LabelNode; import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.VarInsnNode; /** * Filters bytecode that Kotlin compiler generates for chains of safe call @@ -46,6 +47,8 @@ public void filter(final MethodNode methodNode, } /** + * "optimized" chain: + * *
 	 * DUP
 	 * IFNULL label
@@ -60,26 +63,61 @@ public void filter(final MethodNode methodNode,
 	 * label:
 	 * POP
 	 * 
+ * + * "unoptimized" chain: + * + *
+	 * ALOAD v0
+	 * IFNULL label
+	 * ... // call 0
+	 *
+	 * ...
+	 *
+	 * ASTORE v1
+	 * ALOAD v1
+	 * IFNULL label
+	 * ... // call N
+	 *
+	 * label:
+	 * ACONST_NULL
+	 * 
*/ private static Collection> findChains( final MethodNode methodNode) { final HashMap> chains = new HashMap>(); for (final AbstractInsnNode i : methodNode.instructions) { - if (i.getOpcode() == Opcodes.IFNULL - && i.getPrevious().getOpcode() == Opcodes.DUP) { - final JumpInsnNode jump = (JumpInsnNode) i; - final LabelNode label = jump.label; - if (AbstractMatcher.skipNonOpcodes(label.getNext()) - .getOpcode() != Opcodes.POP) { + if (i.getOpcode() != Opcodes.IFNULL) { + continue; + } + final JumpInsnNode jump = (JumpInsnNode) i; + final LabelNode label = jump.label; + final AbstractInsnNode target = AbstractMatcher + .skipNonOpcodes(label); + ArrayList chain = chains.get(label); + if (target.getOpcode() == Opcodes.POP) { + if (i.getPrevious().getOpcode() != Opcodes.DUP) { + continue; + } + } else if (target.getOpcode() == Opcodes.ACONST_NULL) { + if (i.getPrevious().getOpcode() != Opcodes.ALOAD) { continue; } - ArrayList chain = chains.get(label); - if (chain == null) { - chain = new ArrayList(); - chains.put(label, chain); + if (chain != null) { + final AbstractInsnNode p1 = i.getPrevious(); + final AbstractInsnNode p2 = p1.getPrevious(); + if (p2 == null || p2.getOpcode() != Opcodes.ASTORE + || ((VarInsnNode) p1).var != ((VarInsnNode) p2).var) { + continue; + } } - chain.add(jump); + } else { + continue; + } + if (chain == null) { + chain = new ArrayList(); + chains.put(label, chain); } + chain.add(jump); } return chains.values(); } diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html index f3b316c087..d17f65a493 100644 --- a/org.jacoco.doc/docroot/doc/changes.html +++ b/org.jacoco.doc/docroot/doc/changes.html @@ -55,7 +55,8 @@

New Features

(GitHub #1769).
  • Part of bytecode generated by the Kotlin compiler for chains of safe call operators is filtered out during generation of report - (GitHub #1810).
  • + (GitHub #1810, + #1818).
  • Method getEntries generated by the Kotlin compiler for enum classes is filtered out during generation of report (GitHub #1625).