Skip to content

Commit

Permalink
Report code coverage correctly for Kotlin methods with default argume…
Browse files Browse the repository at this point in the history
…nts (#774)
  • Loading branch information
Godin authored and marchof committed Nov 27, 2018
1 parent 276e1cb commit 78b28dc
Show file tree
Hide file tree
Showing 10 changed files with 316 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -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);
}

}
Original file line number Diff line number Diff line change
@@ -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<String>) {
f(a = "a")
f(b = "b")
/* next invocation doesn't use synthetic method: */
f("a", "b")

branch(false)
branch(true)
}

}
Original file line number Diff line number Diff line change
@@ -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();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
@@ -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:
*
* <pre>
* ILOAD maskVar
* ICONST_x, BIPUSH, SIPUSH, LDC or LDC_W
* IAND
* IFEQ label
* default argument
* label:
* </pre>
*
* Where <code>maskVar</code> 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 <code>IFEQ</code>
* 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<AbstractInsnNode> ignore = new HashSet<AbstractInsnNode>();
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);
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand All @@ -32,7 +36,7 @@ public void filter(final MethodNode methodNode,
return;
}

if (!context.getClassAnnotations().contains(KOTLIN_METADATA_DESC)) {
if (!isKotlinClass(context)) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

}
6 changes: 6 additions & 0 deletions org.jacoco.doc/docroot/doc/changes.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ <h3>New Features</h3>
(GitHub <a href="https://github.com/jacoco/jacoco/issues/761">#761</a>).</li>
</ul>

<h3>Fixed Bugs</h3>
<ul>
<li>Report code coverage correctly for Kotlin methods with default arguments
(GitHub <a href="https://github.com/jacoco/jacoco/issues/774">#774</a>).</li>
</ul>

<h3>Non-functional Changes</h3>
<ul>
<li>JaCoCo now depends on ASM 7.0
Expand Down

0 comments on commit 78b28dc

Please sign in to comment.