Skip to content

Commit

Permalink
Get rid of repetitive execution of isKotlinClass in Kotlin filters (#…
Browse files Browse the repository at this point in the history 
…1809)

Co-authored-by: Evgeny Mandrikov <[email protected]>
  • Loading branch information
@marchof @Godin
marchof and Godin authored Dec 17, 2024
1 parent 1e37ea8 commit 22ead5e
Show file tree
Hide file tree
Showing 19 changed files with 94 additions and 170 deletions.
11 changes: 0 additions & 11 deletions ...e.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,6 @@ public void should_filter() {
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,
Expand Down
14 changes: 0 additions & 14 deletions ...ore.test/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultMethodsFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,4 @@ public void should_not_filter_when_instructions_do_not_match() {
assertIgnored();
}

@Test
public void should_not_filter_when_not_kotlin() {
final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0,
"m", "()V", null, null);
m.visitVarInsn(Opcodes.ALOAD, 0);
m.visitMethodInsn(Opcodes.INVOKESTATIC, "Target$DefaultImpls", "m",
"(LTarget;)V", false);
m.visitInsn(Opcodes.RETURN);

filter.filter(m, context, output);

assertIgnored();
}

}
15 changes: 0 additions & 15 deletions org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinEnumFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,6 @@ public void should_not_filter_when_not_Enum() {
assertIgnored();
}

@Test
public void should_not_filter_when_not_Kotlin() {
context.superClassName = "java/lang/Enum";
final MethodNode m = new MethodNode(
Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "getEntries",
"()Lkotlin/enums/EnumEntries;", null, null);
m.visitFieldInsn(Opcodes.GETSTATIC, "Example", "$ENTRIES",
"Lkotlin/enums/EnumEntries;");
m.visitInsn(Opcodes.ARETURN);

filter.filter(m, context, output);

assertIgnored();
}

@Test
public void should_not_filter_when_not_getEntries_name() {
context.superClassName = "java/lang/Enum";
Expand Down
12 changes: 0 additions & 12 deletions ...oco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinGeneratedFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,6 @@ public void testWithLinesForKotlinWithDebug() {
assertIgnored();
}

@Test
public void testNoLinesNonKotlinWithDebug() {
final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0,
"hashCode", "()I", null, null);
m.visitInsn(Opcodes.ICONST_0);
m.visitInsn(Opcodes.IRETURN);

filter.filter(m, context, output);

assertIgnored();
}

@Test
public void testNoLinesForKotlinNoDebug() {
final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0,
Expand Down
12 changes: 0 additions & 12 deletions ...o.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinInlineClassFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,4 @@ public void should_not_filter_when_no_JvmInline_annotation() {
assertIgnored();
}

@Test
public void should_not_filter_when_not_kotlin() {
context.classAnnotations.add("Lkotlin/jvm/JvmInline;");
final MethodNode m = new MethodNode(0, "getValue",
"()Ljava/lang/String;", null, null);
m.visitInsn(Opcodes.NOP);

filter.filter(m, context, output);

assertIgnored();
}

}
12 changes: 0 additions & 12 deletions ...jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinInlineFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,18 +284,6 @@ public void should_filter_all_lines() {
assertIgnored(expectedRanges.toArray(new Range[0]));
}

@Test
public void should_not_parse_SourceDebugExtension_attribute_when_no_kotlin_metadata_annotation() {
context.sourceDebugExtension = "SMAP";

m.visitLineNumber(1, new Label());
m.visitInsn(Opcodes.RETURN);

filter.filter(m, context, output);

assertIgnored();
}

@Test
public void should_not_filter_when_no_SourceDebugExtension_attribute() {
context.classAnnotations
Expand Down
19 changes: 0 additions & 19 deletions ...test/src/org/jacoco/core/internal/analysis/filter/KotlinUnsafeCastOperatorFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,23 +146,4 @@ public void should_filter_Kotlin_1_6() {
assertIgnored(new Range(expectedFrom, expectedTo));
}

@Test
public void should_not_filter_when_not_kotlin() {
m.visitInsn(Opcodes.DUP);
final Label label = new Label();
m.visitJumpInsn(Opcodes.IFNONNULL, label);
m.visitTypeInsn(Opcodes.NEW, "java/lang/NullPointerException");
m.visitInsn(Opcodes.DUP);
m.visitLdcInsn("null cannot be cast to non-null type kotlin.String");
m.visitMethodInsn(Opcodes.INVOKESPECIAL,
"java/lang/NullPointerException", "<init>",
"(Ljava/lang/String;)V", false);
m.visitInsn(Opcodes.ATHROW);
m.visitLabel(label);

filter.filter(m, context, output);

assertIgnored();
}

}
35 changes: 35 additions & 0 deletions org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/FilterSet.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*******************************************************************************
* Copyright (c) 2009, 2024 Mountainminds GmbH & Co. KG and Contributors
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Evgeny Mandrikov - initial API and implementation
*
*******************************************************************************/
package org.jacoco.core.internal.analysis.filter;

import org.objectweb.asm.tree.MethodNode;

/**
* Filter that combines other filters.
*/
class FilterSet implements IFilter {

private final IFilter[] filters;

FilterSet(final IFilter... filters) {
this.filters = filters;
}

public void filter(final MethodNode methodNode,
final IFilterContext context, final IFilterOutput output) {
for (final IFilter filter : filters) {
filter.filter(methodNode, context, output);
}
}

}
90 changes: 55 additions & 35 deletions org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/Filters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,56 +15,76 @@
import org.objectweb.asm.tree.MethodNode;

/**
* Filter that combines other filters.
* Factory for all JaCoCo filters.
*/
public final class Filters implements IFilter {
public final class Filters {

private Filters() {
// no instances
}

/**
* Filter that does nothing.
*/
public static final IFilter NONE = new Filters();

private final IFilter[] filters;
public static final IFilter NONE = new FilterSet();

/**
* Creates filter that combines all other filters.
* Creates a filter that combines all filters.
*
* @return filter that combines all other filters
* @return filter that combines all filters
*/
public static IFilter all() {
return new Filters(new EnumFilter(), new SyntheticFilter(),
new BridgeFilter(), new SynchronizedFilter(),
new TryWithResourcesJavac11Filter(),
new TryWithResourcesJavacFilter(),
new TryWithResourcesEcjFilter(), new FinallyFilter(),
new PrivateEmptyNoArgConstructorFilter(), new AssertFilter(),
new StringSwitchJavacFilter(), new StringSwitchFilter(),
new EnumEmptyConstructorFilter(), new RecordsFilter(),
return new FilterSet( //
allCommonFilters(), //
allKotlinFilters());
}

private static IFilter allCommonFilters() {
return new FilterSet( //
new EnumFilter(), //
new SyntheticFilter(), //
new BridgeFilter(), //
new SynchronizedFilter(), //
new TryWithResourcesJavac11Filter(), //
new TryWithResourcesJavacFilter(), //
new TryWithResourcesEcjFilter(), //
new FinallyFilter(), //
new PrivateEmptyNoArgConstructorFilter(), //
new AssertFilter(), //
new StringSwitchJavacFilter(), //
new StringSwitchFilter(), //
new EnumEmptyConstructorFilter(), //
new RecordsFilter(), //
new ExhaustiveSwitchFilter(), //
new RecordPatternFilter(), //
new AnnotationGeneratedFilter(), new KotlinGeneratedFilter(),
new AnnotationGeneratedFilter());
}

private static IFilter allKotlinFilters() {
return new FilterSet( //
new KotlinGeneratedFilter(), //
new KotlinSyntheticAccessorsFilter(), //
new KotlinEnumFilter(), //
new KotlinSafeCallOperatorFilter(), //
new KotlinLateinitFilter(), new KotlinWhenFilter(),
new KotlinWhenStringFilter(),
new KotlinUnsafeCastOperatorFilter(),
new KotlinNotNullOperatorFilter(),
new KotlinInlineClassFilter(),
new KotlinDefaultArgumentsFilter(), new KotlinInlineFilter(),
new KotlinCoroutineFilter(), new KotlinDefaultMethodsFilter(),
new KotlinComposeFilter());
}

private Filters(final IFilter... filters) {
this.filters = filters;
}

public void filter(final MethodNode methodNode,
final IFilterContext context, final IFilterOutput output) {
for (final IFilter filter : filters) {
filter.filter(methodNode, context, output);
}
new KotlinLateinitFilter(), //
new KotlinWhenFilter(), //
new KotlinWhenStringFilter(), //
new KotlinUnsafeCastOperatorFilter(), //
new KotlinNotNullOperatorFilter(), //
new KotlinInlineClassFilter(), //
new KotlinDefaultArgumentsFilter(), //
new KotlinInlineFilter(), //
new KotlinCoroutineFilter(), //
new KotlinDefaultMethodsFilter(), //
new KotlinComposeFilter()) {
@Override
public void filter(final MethodNode methodNode,
final IFilterContext context, final IFilterOutput output) {
if (isKotlinClass(context)) {
super.filter(methodNode, context, output);
}
}
};
}

/**
Expand Down
3 changes: 0 additions & 3 deletions org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinComposeFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ final class KotlinComposeFilter implements IFilter {

public void filter(final MethodNode methodNode,
final IFilterContext context, final IFilterOutput output) {
if (!Filters.isKotlinClass(context)) {
return;
}
if (!isComposable(methodNode)) {
return;
}
Expand Down
5 changes: 0 additions & 5 deletions org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinCoroutineFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ final class KotlinCoroutineFilter implements IFilter {

public void filter(final MethodNode methodNode,
final IFilterContext context, final IFilterOutput output) {

if (!Filters.isKotlinClass(context)) {
return;
}

new Matcher().match(methodNode, output);
new Matcher().matchOptimizedTailCall(methodNode, output);
}
Expand Down
6 changes: 1 addition & 5 deletions ...acoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultArgumentsFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,6 @@ public void filter(final MethodNode methodNode,
if ((methodNode.access & Opcodes.ACC_SYNTHETIC) == 0) {
return;
}
if (!Filters.isKotlinClass(context)) {
return;
}

if (isDefaultArgumentsMethod(methodNode)) {
new Matcher().match(methodNode, output, false);
} else if (isDefaultArgumentsConstructor(methodNode)) {
Expand Down Expand Up @@ -133,7 +129,7 @@ public void match(final MethodNode methodNode,
skipNonOpcodes();
}

for (AbstractInsnNode i : ignore) {
for (final AbstractInsnNode i : ignore) {
output.ignore(i, i);
}
}
Expand Down
3 changes: 0 additions & 3 deletions org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinDefaultMethodsFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ final class KotlinDefaultMethodsFilter implements IFilter {

public void filter(final MethodNode methodNode,
final IFilterContext context, final IFilterOutput output) {
if (!Filters.isKotlinClass(context)) {
return;
}
new Matcher().match(methodNode, output);
}

Expand Down
3 changes: 0 additions & 3 deletions org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinEnumFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ public void filter(final MethodNode methodNode,
if (!"java/lang/Enum".equals(context.getSuperClassName())) {
return;
}
if (!Filters.isKotlinClass(context)) {
return;
}
if (!"getEntries".equals(methodNode.name)) {
return;
}
Expand Down
5 changes: 0 additions & 5 deletions org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinGeneratedFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ public void filter(final MethodNode methodNode,
// disabled filtering as all methods might be erroneously skipped
return;
}

if (!Filters.isKotlinClass(context)) {
return;
}

if (hasLineNumber(methodNode)) {
for (final AbstractInsnNode i : methodNode.instructions) {
if (AbstractInsnNode.LINE == i.getType()) {
Expand Down
3 changes: 0 additions & 3 deletions org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinInlineClassFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ final class KotlinInlineClassFilter implements IFilter {

public void filter(final MethodNode methodNode,
final IFilterContext context, final IFilterOutput output) {
if (!Filters.isKotlinClass(context)) {
return;
}
if (!context.getClassAnnotations().contains("Lkotlin/jvm/JvmInline;")) {
return;
}
Expand Down
8 changes: 2 additions & 6 deletions org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinInlineFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ public void filter(final MethodNode methodNode,
return;
}

if (!Filters.isKotlinClass(context)) {
return;
}

if (firstGeneratedLineNumber == -1) {
firstGeneratedLineNumber = getFirstGeneratedLineNumber(
context.getClassName(), context.getSourceFileName(),
Expand All @@ -53,8 +49,8 @@ public void filter(final MethodNode methodNode,
private static int getFirstGeneratedLineNumber(final String className,
final String sourceFileName, final String smap) {
int min = Integer.MAX_VALUE;
for (KotlinSMAP.Mapping mapping : new KotlinSMAP(sourceFileName, smap)
.mappings()) {
for (final KotlinSMAP.Mapping mapping : new KotlinSMAP(sourceFileName,
smap).mappings()) {
if (className.equals(mapping.inputClassName())
&& mapping.inputStartLine() == mapping.outputStartLine()) {
continue;
Expand Down
Loading

0 comments on commit 22ead5e

Please sign in to comment.