From 78b28dc8463c301b207c60f0394251d4f2ae9aba Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Tue, 27 Nov 2018 13:16:14 +0100 Subject: [PATCH] Report code coverage correctly for Kotlin methods with default arguments (#774) --- .../kotlin/KotlinDefaultArgumentsTest.java | 26 +++++ .../targets/KotlinDefaultArgumentsTarget.kt | 36 +++++++ .../KotlinDefaultArgumentsFilterTest.java | 98 +++++++++++++++++++ .../analysis/filter/SyntheticFilterTest.java | 26 +++++ .../analysis/filter/AbstractMatcher.java | 6 +- .../internal/analysis/filter/Filters.java | 3 +- .../filter/KotlinDefaultArgumentsFilter.java | 97 ++++++++++++++++++ .../filter/KotlinGeneratedFilter.java | 6 +- .../analysis/filter/SyntheticFilter.java | 19 +++- org.jacoco.doc/docroot/doc/changes.html | 6 ++ 10 files changed, 316 insertions(+), 7 deletions(-) create mode 100644 org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/KotlinDefaultArgumentsTest.java create mode 100644 org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinDefaultArgumentsTarget.kt create mode 100644 org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java create mode 100644 org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java diff --git a/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/KotlinDefaultArgumentsTest.java b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/KotlinDefaultArgumentsTest.java new file mode 100644 index 000000000..4199cf7c8 --- /dev/null +++ b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/KotlinDefaultArgumentsTest.java @@ -0,0 +1,26 @@ +/******************************************************************************* + * Copyright (c) 2009, 2018 Mountainminds GmbH & Co. KG and Contributors + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Evgeny Mandrikov - initial API and implementation + * + *******************************************************************************/ +package org.jacoco.core.test.validation.kotlin; + +import org.jacoco.core.test.validation.ValidationTestBase; +import org.jacoco.core.test.validation.kotlin.targets.KotlinDefaultArgumentsTarget; + +/** + * Test of functions with default arguments. + */ +public class KotlinDefaultArgumentsTest extends ValidationTestBase { + + public KotlinDefaultArgumentsTest() { + super(KotlinDefaultArgumentsTarget.class); + } + +} diff --git a/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinDefaultArgumentsTarget.kt b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinDefaultArgumentsTarget.kt new file mode 100644 index 000000000..6943f2067 --- /dev/null +++ b/org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinDefaultArgumentsTarget.kt @@ -0,0 +1,36 @@ +/******************************************************************************* + * Copyright (c) 2009, 2018 Mountainminds GmbH & Co. KG and Contributors + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Evgeny Mandrikov - initial API and implementation + * + *******************************************************************************/ +package org.jacoco.core.test.validation.kotlin.targets + +/** + * Test target for functions with default arguments. + */ +object KotlinDefaultArgumentsTarget { + + private fun f(a: String = "a", b: String = "b") { // assertFullyCovered(0, 0) + } + + private fun branch(a: Boolean, b: String = if (a) "a" else "b") { // assertFullyCovered(0, 2) + } + + @JvmStatic + fun main(args: Array) { + f(a = "a") + f(b = "b") + /* next invocation doesn't use synthetic method: */ + f("a", "b") + + branch(false) + branch(true) + } + +} diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java new file mode 100644 index 000000000..89c54c1d7 --- /dev/null +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java @@ -0,0 +1,98 @@ +/******************************************************************************* + * Copyright (c) 2009, 2018 Mountainminds GmbH & Co. KG and Contributors + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Evgeny Mandrikov - initial API and implementation + * + *******************************************************************************/ +package org.jacoco.core.internal.analysis.filter; + +import org.jacoco.core.internal.instr.InstrSupport; +import org.junit.Test; +import org.objectweb.asm.Label; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.tree.MethodNode; + +/** + * Unit test for {@link KotlinDefaultArgumentsFilter}. + */ +public class KotlinDefaultArgumentsFilterTest extends FilterTestBase { + + private final IFilter filter = new KotlinDefaultArgumentsFilter(); + + private static MethodNode createMethod(final int access, final String name, + final String descriptor) { + final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, + access, name, descriptor, null, null); + + m.visitVarInsn(Opcodes.ILOAD, 2); + m.visitInsn(Opcodes.ICONST_1); + m.visitInsn(Opcodes.IAND); + final Label label = new Label(); + m.visitJumpInsn(Opcodes.IFEQ, label); + // default argument + m.visitLdcInsn(Integer.valueOf(42)); + m.visitVarInsn(Opcodes.ISTORE, 1); + m.visitLabel(label); + + m.visitVarInsn(Opcodes.ALOAD, 0); + m.visitVarInsn(Opcodes.ILOAD, 1); + m.visitMethodInsn(Opcodes.INVOKESPECIAL, "Target", "origin", "(I)V", + false); + m.visitInsn(Opcodes.RETURN); + + return m; + } + + @Test + public void should_filter() { + final MethodNode m = createMethod(Opcodes.ACC_SYNTHETIC, + "origin$default", "(LTarget;IILjava/lang/Object;)V"); + context.classAnnotations + .add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC); + + filter.filter(m, context, output); + + assertIgnored(new Range(m.instructions.get(3), m.instructions.get(3))); + } + + @Test + public void should_not_filter_when_not_kotlin() { + final MethodNode m = createMethod(Opcodes.ACC_SYNTHETIC, + "not_kotlin_synthetic$default", + "(LTarget;IILjava/lang/Object;)V"); + + filter.filter(m, context, output); + + assertIgnored(); + } + + @Test + public void should_not_filter_when_suffix_absent() { + final MethodNode m = createMethod(Opcodes.ACC_SYNTHETIC, + "synthetic_without_suffix", "(LTarget;IILjava/lang/Object;)V"); + context.classAnnotations + .add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC); + + filter.filter(m, context, output); + + assertIgnored(); + } + + @Test + public void should_not_filter_when_not_synthetic() { + final MethodNode m = createMethod(0, "not_synthetic$default", + "(LTarget;IILjava/lang/Object;)V"); + context.classAnnotations + .add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC); + + filter.filter(m, context, output); + + assertIgnored(); + } + +} diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/SyntheticFilterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/SyntheticFilterTest.java index 011fc5427..9b70b70a1 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/SyntheticFilterTest.java +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/SyntheticFilterTest.java @@ -56,4 +56,30 @@ public void testLambda() { assertIgnored(); } + @Test + public void should_not_filter_method_with_suffix_default_in_kotlin_classes() { + final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, + Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE, "example$default", + "(LTarget;Ljava/lang/String;Ijava/lang/Object;)V", null, null); + context.classAnnotations + .add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC); + m.visitInsn(Opcodes.NOP); + + filter.filter(m, context, output); + + assertIgnored(); + } + + @Test + public void should_filter_synthetic_method_with_suffix_default_in_non_kotlin_classes() { + final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, + Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE, "example$default", + "(LTarget;Ljava/lang/String;Ijava/lang/Object;)V", null, null); + m.visitInsn(Opcodes.NOP); + + filter.filter(m, context, output); + + assertMethodIgnored(m); + } + } diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java index 9b01e777b..a4dd6208c 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java @@ -154,7 +154,11 @@ final void next() { skipNonOpcodes(); } - private void skipNonOpcodes() { + /** + * Moves {@link #cursor} through {@link AbstractInsnNode#FRAME}, + * {@link AbstractInsnNode#LABEL}, {@link AbstractInsnNode#LINE}. + */ + final void skipNonOpcodes() { while (cursor != null && (cursor.getType() == AbstractInsnNode.FRAME || cursor.getType() == AbstractInsnNode.LABEL || cursor.getType() == AbstractInsnNode.LINE)) { diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/Filters.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/Filters.java index f6a7a1dfb..76b050f4f 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/Filters.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/Filters.java @@ -35,7 +35,8 @@ public final class Filters implements IFilter { new EnumEmptyConstructorFilter(), new AnnotationGeneratedFilter(), new KotlinGeneratedFilter(), new KotlinLateinitFilter(), new KotlinWhenFilter(), new KotlinWhenStringFilter(), - new KotlinUnsafeCastOperatorFilter()); + new KotlinUnsafeCastOperatorFilter(), + new KotlinDefaultArgumentsFilter()); private final IFilter[] filters; diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java new file mode 100644 index 000000000..8c2bdebad --- /dev/null +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java @@ -0,0 +1,97 @@ +/******************************************************************************* + * Copyright (c) 2009, 2018 Mountainminds GmbH & Co. KG and Contributors + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Evgeny Mandrikov - initial API and implementation + * + *******************************************************************************/ +package org.jacoco.core.internal.analysis.filter; + +import java.util.HashSet; +import java.util.Set; + +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.JumpInsnNode; +import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.VarInsnNode; + +/** + * Filters branches that Kotlin compiler generates for default arguments. + * + * For each default argument Kotlin compiler generates following bytecode to + * determine if it should be used or not: + * + * 
+ * ILOAD maskVar
+ * ICONST_x, BIPUSH, SIPUSH, LDC or LDC_W
+ * IAND
+ * IFEQ label
+ * default argument
+ * label:
+ *
+ * + * Where maskVar is penultimate argument of synthetic method with + * suffix "$default". And its value can't be zero - invocation with all + * arguments uses original non synthetic method, thus IFEQ + * instructions should be ignored. + */ +public final class KotlinDefaultArgumentsFilter implements IFilter { + + static boolean isDefaultArgumentsMethodName(final String methodName) { + return methodName.endsWith("$default"); + } + + public void filter(final MethodNode methodNode, + final IFilterContext context, final IFilterOutput output) { + if ((methodNode.access & Opcodes.ACC_SYNTHETIC) == 0) { + return; + } + if (!isDefaultArgumentsMethodName(methodNode.name)) { + return; + } + if (!KotlinGeneratedFilter.isKotlinClass(context)) { + return; + } + + new Matcher().match(methodNode, output); + } + + private static class Matcher extends AbstractMatcher { + public void match(final MethodNode methodNode, + final IFilterOutput output) { + cursor = methodNode.instructions.getFirst(); + + final Set ignore = new HashSet(); + final int maskVar = Type.getMethodType(methodNode.desc) + .getArgumentTypes().length - 2; + while (true) { + if (cursor.getOpcode() != Opcodes.ILOAD) { + break; + } + if (((VarInsnNode) cursor).var != maskVar) { + break; + } + next(); + nextIs(Opcodes.IAND); + nextIs(Opcodes.IFEQ); + if (cursor == null) { + return; + } + ignore.add(cursor); + cursor = ((JumpInsnNode) cursor).label; + skipNonOpcodes(); + } + + for (AbstractInsnNode i : ignore) { + output.ignore(i, i); + } + } + } + +} diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinGeneratedFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinGeneratedFilter.java index 683314923..722fd7166 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinGeneratedFilter.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinGeneratedFilter.java @@ -23,6 +23,10 @@ public class KotlinGeneratedFilter implements IFilter { static final String KOTLIN_METADATA_DESC = "Lkotlin/Metadata;"; + static boolean isKotlinClass(final IFilterContext context) { + return context.getClassAnnotations().contains(KOTLIN_METADATA_DESC); + } + public void filter(final MethodNode methodNode, final IFilterContext context, final IFilterOutput output) { @@ -32,7 +36,7 @@ public void filter(final MethodNode methodNode, return; } - if (!context.getClassAnnotations().contains(KOTLIN_METADATA_DESC)) { + if (!isKotlinClass(context)) { return; } diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/SyntheticFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/SyntheticFilter.java index 52d38bd2d..5dfeecf5e 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/SyntheticFilter.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/SyntheticFilter.java @@ -21,11 +21,22 @@ public final class SyntheticFilter implements IFilter { public void filter(final MethodNode methodNode, final IFilterContext context, final IFilterOutput output) { - if ((methodNode.access & Opcodes.ACC_SYNTHETIC) != 0 - && !methodNode.name.startsWith("lambda$")) { - output.ignore(methodNode.instructions.getFirst(), - methodNode.instructions.getLast()); + if ((methodNode.access & Opcodes.ACC_SYNTHETIC) == 0) { + return; } + + if (methodNode.name.startsWith("lambda$")) { + return; + } + + if (KotlinDefaultArgumentsFilter + .isDefaultArgumentsMethodName(methodNode.name) + && KotlinGeneratedFilter.isKotlinClass(context)) { + return; + } + + output.ignore(methodNode.instructions.getFirst(), + methodNode.instructions.getLast()); } } diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html index bfe6d8c34..48e03ec4b 100644 --- a/org.jacoco.doc/docroot/doc/changes.html +++ b/org.jacoco.doc/docroot/doc/changes.html @@ -29,6 +29,12 @@

New Features

(GitHub #761). +

Fixed Bugs

+
    +
  • Report code coverage correctly for Kotlin methods with default arguments + (GitHub #774).
    • +
+

Non-functional Changes

  • JaCoCo now depends on ASM 7.0