Skip to content

Commit 78b28dc

Browse files
Godinmarchof
Godin
authored and
marchof
committed
Report code coverage correctly for Kotlin methods with default arguments (bazel-contrib#774)
1 parent 276e1cb commit 78b28dc

File tree

10 files changed

+316
-7
lines changed

10 files changed

+316
-7
lines changed

‎org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/KotlinDefaultArgumentsTest.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2009, 2018 Mountainminds GmbH & Co. KG and Contributors
3+
* All rights reserved. This program and the accompanying materials
4+
* are made available under the terms of the Eclipse Public License v1.0
5+
* which accompanies this distribution, and is available at
6+
* http://www.eclipse.org/legal/epl-v10.html
7+
*
8+
* Contributors:
9+
* Evgeny Mandrikov - initial API and implementation
10+
*
11+
*******************************************************************************/
12+
package org.jacoco.core.test.validation.kotlin;
13+
14+
import org.jacoco.core.test.validation.ValidationTestBase;
15+
import org.jacoco.core.test.validation.kotlin.targets.KotlinDefaultArgumentsTarget;
16+
17+
/**
18+
* Test of functions with default arguments.
19+
*/
20+
public class KotlinDefaultArgumentsTest extends ValidationTestBase {
21+
22+
public KotlinDefaultArgumentsTest() {
23+
super(KotlinDefaultArgumentsTarget.class);
24+
}
25+
26+
}

‎org.jacoco.core.test.validation.kotlin/src/org/jacoco/core/test/validation/kotlin/targets/KotlinDefaultArgumentsTarget.kt‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2009, 2018 Mountainminds GmbH & Co. KG and Contributors
3+
* All rights reserved. This program and the accompanying materials
4+
* are made available under the terms of the Eclipse Public License v1.0
5+
* which accompanies this distribution, and is available at
6+
* http://www.eclipse.org/legal/epl-v10.html
7+
*
8+
* Contributors:
9+
* Evgeny Mandrikov - initial API and implementation
10+
*
11+
*******************************************************************************/
12+
package org.jacoco.core.test.validation.kotlin.targets
13+
14+
/**
15+
* Test target for functions with default arguments.
16+
*/
17+
object KotlinDefaultArgumentsTarget {
18+
19+
private fun f(a: String = "a", b: String = "b") { // assertFullyCovered(0, 0)
20+
}
21+
22+
private fun branch(a: Boolean, b: String = if (a) "a" else "b") { // assertFullyCovered(0, 2)
23+
}
24+
25+
@JvmStatic
26+
fun main(args: Array<String>) {
27+
f(a = "a")
28+
f(b = "b")
29+
/* next invocation doesn't use synthetic method: */
30+
f("a", "b")
31+
32+
branch(false)
33+
branch(true)
34+
}
35+
36+
}

‎org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2009, 2018 Mountainminds GmbH & Co. KG and Contributors
3+
* All rights reserved. This program and the accompanying materials
4+
* are made available under the terms of the Eclipse Public License v1.0
5+
* which accompanies this distribution, and is available at
6+
* http://www.eclipse.org/legal/epl-v10.html
7+
*
8+
* Contributors:
9+
* Evgeny Mandrikov - initial API and implementation
10+
*
11+
*******************************************************************************/
12+
package org.jacoco.core.internal.analysis.filter;
13+
14+
import org.jacoco.core.internal.instr.InstrSupport;
15+
import org.junit.Test;
16+
import org.objectweb.asm.Label;
17+
import org.objectweb.asm.Opcodes;
18+
import org.objectweb.asm.tree.MethodNode;
19+
20+
/**
21+
* Unit test for {@link KotlinDefaultArgumentsFilter}.
22+
*/
23+
public class KotlinDefaultArgumentsFilterTest extends FilterTestBase {
24+
25+
private final IFilter filter = new KotlinDefaultArgumentsFilter();
26+
27+
private static MethodNode createMethod(final int access, final String name,
28+
final String descriptor) {
29+
final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION,
30+
access, name, descriptor, null, null);
31+
32+
m.visitVarInsn(Opcodes.ILOAD, 2);
33+
m.visitInsn(Opcodes.ICONST_1);
34+
m.visitInsn(Opcodes.IAND);
35+
final Label label = new Label();
36+
m.visitJumpInsn(Opcodes.IFEQ, label);
37+
// default argument
38+
m.visitLdcInsn(Integer.valueOf(42));
39+
m.visitVarInsn(Opcodes.ISTORE, 1);
40+
m.visitLabel(label);
41+
42+
m.visitVarInsn(Opcodes.ALOAD, 0);
43+
m.visitVarInsn(Opcodes.ILOAD, 1);
44+
m.visitMethodInsn(Opcodes.INVOKESPECIAL, "Target", "origin", "(I)V",
45+
false);
46+
m.visitInsn(Opcodes.RETURN);
47+
48+
return m;
49+
}
50+
51+
@Test
52+
public void should_filter() {
53+
final MethodNode m = createMethod(Opcodes.ACC_SYNTHETIC,
54+
"origin$default", "(LTarget;IILjava/lang/Object;)V");
55+
context.classAnnotations
56+
.add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC);
57+
58+
filter.filter(m, context, output);
59+
60+
assertIgnored(new Range(m.instructions.get(3), m.instructions.get(3)));
61+
}
62+
63+
@Test
64+
public void should_not_filter_when_not_kotlin() {
65+
final MethodNode m = createMethod(Opcodes.ACC_SYNTHETIC,
66+
"not_kotlin_synthetic$default",
67+
"(LTarget;IILjava/lang/Object;)V");
68+
69+
filter.filter(m, context, output);
70+
71+
assertIgnored();
72+
}
73+
74+
@Test
75+
public void should_not_filter_when_suffix_absent() {
76+
final MethodNode m = createMethod(Opcodes.ACC_SYNTHETIC,
77+
"synthetic_without_suffix", "(LTarget;IILjava/lang/Object;)V");
78+
context.classAnnotations
79+
.add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC);
80+
81+
filter.filter(m, context, output);
82+
83+
assertIgnored();
84+
}
85+
86+
@Test
87+
public void should_not_filter_when_not_synthetic() {
88+
final MethodNode m = createMethod(0, "not_synthetic$default",
89+
"(LTarget;IILjava/lang/Object;)V");
90+
context.classAnnotations
91+
.add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC);
92+
93+
filter.filter(m, context, output);
94+
95+
assertIgnored();
96+
}
97+
98+
}

‎org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/SyntheticFilterTest.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,30 @@ public void testLambda() {
5656
assertIgnored();
5757
}
5858

59+
@Test
60+
public void should_not_filter_method_with_suffix_default_in_kotlin_classes() {
61+
final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION,
62+
Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE, "example$default",
63+
"(LTarget;Ljava/lang/String;Ijava/lang/Object;)V", null, null);
64+
context.classAnnotations
65+
.add(KotlinGeneratedFilter.KOTLIN_METADATA_DESC);
66+
m.visitInsn(Opcodes.NOP);
67+
68+
filter.filter(m, context, output);
69+
70+
assertIgnored();
71+
}
72+
73+
@Test
74+
public void should_filter_synthetic_method_with_suffix_default_in_non_kotlin_classes() {
75+
final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION,
76+
Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE, "example$default",
77+
"(LTarget;Ljava/lang/String;Ijava/lang/Object;)V", null, null);
78+
m.visitInsn(Opcodes.NOP);
79+
80+
filter.filter(m, context, output);
81+
82+
assertMethodIgnored(m);
83+
}
84+
5985
}

‎org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractMatcher.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,11 @@ final void next() {
154154
skipNonOpcodes();
155155
}
156156

157-
private void skipNonOpcodes() {
157+
/**
158+
* Moves {@link #cursor} through {@link AbstractInsnNode#FRAME},
159+
* {@link AbstractInsnNode#LABEL}, {@link AbstractInsnNode#LINE}.
160+
*/
161+
final void skipNonOpcodes() {
158162
while (cursor != null && (cursor.getType() == AbstractInsnNode.FRAME
159163
|| cursor.getType() == AbstractInsnNode.LABEL
160164
|| cursor.getType() == AbstractInsnNode.LINE)) {

‎org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/Filters.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ public final class Filters implements IFilter {
3535
new EnumEmptyConstructorFilter(), new AnnotationGeneratedFilter(),
3636
new KotlinGeneratedFilter(), new KotlinLateinitFilter(),
3737
new KotlinWhenFilter(), new KotlinWhenStringFilter(),
38-
new KotlinUnsafeCastOperatorFilter());
38+
new KotlinUnsafeCastOperatorFilter(),
39+
new KotlinDefaultArgumentsFilter());
3940

4041
private final IFilter[] filters;
4142

‎org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2009, 2018 Mountainminds GmbH & Co. KG and Contributors
3+
* All rights reserved. This program and the accompanying materials
4+
* are made available under the terms of the Eclipse Public License v1.0
5+
* which accompanies this distribution, and is available at
6+
* http://www.eclipse.org/legal/epl-v10.html
7+
*
8+
* Contributors:
9+
* Evgeny Mandrikov - initial API and implementation
10+
*
11+
*******************************************************************************/
12+
package org.jacoco.core.internal.analysis.filter;
13+
14+
import java.util.HashSet;
15+
import java.util.Set;
16+
17+
import org.objectweb.asm.Opcodes;
18+
import org.objectweb.asm.Type;
19+
import org.objectweb.asm.tree.AbstractInsnNode;
20+
import org.objectweb.asm.tree.JumpInsnNode;
21+
import org.objectweb.asm.tree.MethodNode;
22+
import org.objectweb.asm.tree.VarInsnNode;
23+
24+
/**
25+
* Filters branches that Kotlin compiler generates for default arguments.
26+
*
27+
* For each default argument Kotlin compiler generates following bytecode to
28+
* determine if it should be used or not:
29+
*
30+
* <pre>
31+
* ILOAD maskVar
32+
* ICONST_x, BIPUSH, SIPUSH, LDC or LDC_W
33+
* IAND
34+
* IFEQ label
35+
* default argument
36+
* label:
37+
* </pre>
38+
*
39+
* Where <code>maskVar</code> is penultimate argument of synthetic method with
40+
* suffix "$default". And its value can't be zero - invocation with all
41+
* arguments uses original non synthetic method, thus <code>IFEQ</code>
42+
* instructions should be ignored.
43+
*/
44+
public final class KotlinDefaultArgumentsFilter implements IFilter {
45+
46+
static boolean isDefaultArgumentsMethodName(final String methodName) {
47+
return methodName.endsWith("$default");
48+
}
49+
50+
public void filter(final MethodNode methodNode,
51+
final IFilterContext context, final IFilterOutput output) {
52+
if ((methodNode.access & Opcodes.ACC_SYNTHETIC) == 0) {
53+
return;
54+
}
55+
if (!isDefaultArgumentsMethodName(methodNode.name)) {
56+
return;
57+
}
58+
if (!KotlinGeneratedFilter.isKotlinClass(context)) {
59+
return;
60+
}
61+
62+
new Matcher().match(methodNode, output);
63+
}
64+
65+
private static class Matcher extends AbstractMatcher {
66+
public void match(final MethodNode methodNode,
67+
final IFilterOutput output) {
68+
cursor = methodNode.instructions.getFirst();
69+
70+
final Set<AbstractInsnNode> ignore = new HashSet<AbstractInsnNode>();
71+
final int maskVar = Type.getMethodType(methodNode.desc)
72+
.getArgumentTypes().length - 2;
73+
while (true) {
74+
if (cursor.getOpcode() != Opcodes.ILOAD) {
75+
break;
76+
}
77+
if (((VarInsnNode) cursor).var != maskVar) {
78+
break;
79+
}
80+
next();
81+
nextIs(Opcodes.IAND);
82+
nextIs(Opcodes.IFEQ);
83+
if (cursor == null) {
84+
return;
85+
}
86+
ignore.add(cursor);
87+
cursor = ((JumpInsnNode) cursor).label;
88+
skipNonOpcodes();
89+
}
90+
91+
for (AbstractInsnNode i : ignore) {
92+
output.ignore(i, i);
93+
}
94+
}
95+
}
96+
97+
}

‎org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinGeneratedFilter.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ public class KotlinGeneratedFilter implements IFilter {
2323

2424
static final String KOTLIN_METADATA_DESC = "Lkotlin/Metadata;";
2525

26+
static boolean isKotlinClass(final IFilterContext context) {
27+
return context.getClassAnnotations().contains(KOTLIN_METADATA_DESC);
28+
}
29+
2630
public void filter(final MethodNode methodNode,
2731
final IFilterContext context, final IFilterOutput output) {
2832

@@ -32,7 +36,7 @@ public void filter(final MethodNode methodNode,
3236
return;
3337
}
3438

35-
if (!context.getClassAnnotations().contains(KOTLIN_METADATA_DESC)) {
39+
if (!isKotlinClass(context)) {
3640
return;
3741
}
3842

‎org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/SyntheticFilter.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,22 @@ public final class SyntheticFilter implements IFilter {
2121

2222
public void filter(final MethodNode methodNode,
2323
final IFilterContext context, final IFilterOutput output) {
24-
if ((methodNode.access & Opcodes.ACC_SYNTHETIC) != 0
25-
&& !methodNode.name.startsWith("lambda$")) {
26-
output.ignore(methodNode.instructions.getFirst(),
27-
methodNode.instructions.getLast());
24+
if ((methodNode.access & Opcodes.ACC_SYNTHETIC) == 0) {
25+
return;
2826
}
27+
28+
if (methodNode.name.startsWith("lambda$")) {
29+
return;
30+
}
31+
32+
if (KotlinDefaultArgumentsFilter
33+
.isDefaultArgumentsMethodName(methodNode.name)
34+
&& KotlinGeneratedFilter.isKotlinClass(context)) {
35+
return;
36+
}
37+
38+
output.ignore(methodNode.instructions.getFirst(),
39+
methodNode.instructions.getLast());
2940
}
3041

3142
}

‎org.jacoco.doc/docroot/doc/changes.html‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@ <h3>New Features</h3>
2929
(GitHub <a href="https://github.com/jacoco/jacoco/issues/761">#761</a>).</li>
3030
</ul>
3131

32+
<h3>Fixed Bugs</h3>
33+
<ul>
34+
<li>Report code coverage correctly for Kotlin methods with default arguments
35+
(GitHub <a href="https://github.com/jacoco/jacoco/issues/774">#774</a>).</li>
36+
</ul>
37+
3238
<h3>Non-functional Changes</h3>
3339
<ul>
3440
<li>JaCoCo now depends on ASM 7.0

0 commit comments

Comments
 (0)