Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report code coverage correctly for Kotlin methods with default arguments #774

Merged
merged 23 commits into from
Nov 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
Godin marked this conversation as resolved.
Show resolved Hide resolved
Godin marked this conversation as resolved.
Show resolved Hide resolved
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