From 8b187a40b3df31e8f253a0a8dc65cba87e33b77b Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Fri, 4 Jun 2021 14:14:45 +0200 Subject: [PATCH 1/5] (WIP) add validation test --- .../test/validation/java5/AssertTest.java | 27 +++++++++++++++++ .../java5/targets/AssertTarget.java | 29 +++++++++++++++++++ .../test/validation/ValidationTestBase.java | 1 + 3 files changed, 57 insertions(+) create mode 100644 org.jacoco.core.test.validation.java5/src/org/jacoco/core/test/validation/java5/AssertTest.java create mode 100644 org.jacoco.core.test.validation.java5/src/org/jacoco/core/test/validation/java5/targets/AssertTarget.java diff --git a/org.jacoco.core.test.validation.java5/src/org/jacoco/core/test/validation/java5/AssertTest.java b/org.jacoco.core.test.validation.java5/src/org/jacoco/core/test/validation/java5/AssertTest.java new file mode 100644 index 0000000000..780b2fdb31 --- /dev/null +++ b/org.jacoco.core.test.validation.java5/src/org/jacoco/core/test/validation/java5/AssertTest.java @@ -0,0 +1,27 @@ +/******************************************************************************* + * Copyright (c) 2009, 2021 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.test.validation.java5; + +import org.jacoco.core.test.validation.ValidationTestBase; +import org.jacoco.core.test.validation.java5.targets.AssertTarget; + +/** + * Test of code coverage in {@link AssertTarget}. + */ +public class AssertTest extends ValidationTestBase { + + public AssertTest() { + super(AssertTarget.class); + } + +} diff --git a/org.jacoco.core.test.validation.java5/src/org/jacoco/core/test/validation/java5/targets/AssertTarget.java b/org.jacoco.core.test.validation.java5/src/org/jacoco/core/test/validation/java5/targets/AssertTarget.java new file mode 100644 index 0000000000..316cfc7735 --- /dev/null +++ b/org.jacoco.core.test.validation.java5/src/org/jacoco/core/test/validation/java5/targets/AssertTarget.java @@ -0,0 +1,29 @@ +/******************************************************************************* + * Copyright (c) 2009, 2021 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.test.validation.java5.targets; + +import static org.jacoco.core.test.validation.targets.Stubs.t; + +/** + * This target exercises assert statement. + */ +public class AssertTarget { // assertFullyCovered() + + private AssertTarget() { + } + + public static void main(String[] args) { + assert t() : "msg"; // assertPartlyCovered(1, 1) + } + +} diff --git a/org.jacoco.core.test/src/org/jacoco/core/test/validation/ValidationTestBase.java b/org.jacoco.core.test/src/org/jacoco/core/test/validation/ValidationTestBase.java index ab8be47381..1d35126890 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/test/validation/ValidationTestBase.java +++ b/org.jacoco.core.test/src/org/jacoco/core/test/validation/ValidationTestBase.java @@ -71,6 +71,7 @@ public void setup() throws Exception { private ExecutionDataStore execute() throws Exception { loader = new InstrumentingLoader(target); + loader.setDefaultAssertionStatus(true); run(loader.loadClass(target.getName())); return loader.collect(); } From ddd061b7f2ac762fd4dd02c389d9dd707e25917e Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Sat, 5 Jun 2021 09:46:15 +0200 Subject: [PATCH 2/5] (WIP) add unit tests --- .../analysis/filter/AssertFilterTest.java | 145 ++++++++++++++++++ .../analysis/filter/AssertFilter.java | 27 ++++ 2 files changed, 172 insertions(+) create mode 100644 org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/AssertFilterTest.java create mode 100644 org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/AssertFilterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/AssertFilterTest.java new file mode 100644 index 0000000000..52803c9a12 --- /dev/null +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/AssertFilterTest.java @@ -0,0 +1,145 @@ +/******************************************************************************* + * Copyright (c) 2009, 2021 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.jacoco.core.internal.instr.InstrSupport; +import org.junit.Test; +import org.objectweb.asm.Label; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; +import org.objectweb.asm.tree.MethodNode; + +/** + * Unit tests for {@link AssertFilter}. + */ +public class AssertFilterTest extends FilterTestBase { + + private final AssertFilter filter = new AssertFilter(); + + /** + *
+	 * class Example {
+	 *   void example(boolean b) {
+	 *     ...
+	 *     assert b : "message";
+	 *     ...
+	 *   }
+	 * }
+	 * 
+ */ + @Test + public void should_filter_static_initializer() { + context.className = "Example"; + final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, + Opcodes.ACC_STATIC, "", "()V", null, null); + + m.visitLdcInsn(Type.getType("LExample;")); + final Range range = new Range(); + range.fromInclusive = m.instructions.getLast(); + m.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "java/lang/Class", + "desiredAssertionStatus", "()Z", false); + final Label label1 = new Label(); + final Label label2 = new Label(); + m.visitJumpInsn(Opcodes.IFNE, label1); + m.visitInsn(Opcodes.ICONST_1); + m.visitJumpInsn(Opcodes.GOTO, label2); + m.visitLabel(label1); + m.visitInsn(Opcodes.ICONST_0); + m.visitLabel(label2); + m.visitFieldInsn(Opcodes.PUTSTATIC, "Example", "$assertionsDisabled", + "Z"); + range.toInclusive = m.instructions.getLast(); + m.visitInsn(Opcodes.RETURN); + + filter.filter(m, context, output); + + assertIgnored(range); + } + + /** + *
+	 * class Example {
+	 *   static final f;
+	 *
+	 *   static {
+	 *     f = !Example.class.desiredAssertionStatus();
+	 *   }
+	 * }
+	 * 
+ */ + @Test + public void should_not_filter_static_initializer_when_field_name_does_not_match() { + context.className = "Example"; + final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, + Opcodes.ACC_STATIC, "", "()V", null, null); + + m.visitLdcInsn(Type.getType("LExample;")); + m.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "java/lang/Class", + "desiredAssertionStatus", "()Z", false); + final Label label1 = new Label(); + final Label label2 = new Label(); + m.visitJumpInsn(Opcodes.IFNE, label1); + m.visitInsn(Opcodes.ICONST_1); + m.visitJumpInsn(Opcodes.GOTO, label2); + m.visitLabel(label1); + m.visitInsn(Opcodes.ICONST_0); + m.visitLabel(label2); + m.visitFieldInsn(Opcodes.PUTSTATIC, "Foo", "f", "Z"); + m.visitInsn(Opcodes.RETURN); + + filter.filter(m, context, output); + + assertIgnored(); + } + + /** + *
+	 * class Example {
+	 *   void example(boolean b) {
+	 *     ...
+	 *     assert b : "message";
+	 *     ...
+	 *   }
+	 * }
+	 * 
+ */ + @Test + public void should_filter_assert() { + context.className = "Example"; + final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0, + "example", "()V", null, null); + + m.visitInsn(Opcodes.NOP); + m.visitFieldInsn(Opcodes.GETSTATIC, "Example", "$assertionsDisabled", + "Z"); + final Label label = new Label(); + m.visitJumpInsn(Opcodes.IFNE, label); + final Range range = new Range(m.instructions.getLast(), + m.instructions.getLast()); + m.visitVarInsn(Opcodes.ILOAD, 1); + m.visitJumpInsn(Opcodes.IFNE, label); + m.visitTypeInsn(Opcodes.NEW, "java/lang/AssertionError"); + m.visitInsn(Opcodes.DUP); + m.visitLdcInsn("message"); + m.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/AssertionError", + "", "(Ljava/lang/Object;)V", false); + m.visitInsn(Opcodes.ATHROW); + m.visitLabel(label); + m.visitInsn(Opcodes.NOP); + + filter.filter(m, context, output); + + assertIgnored(range); + } + +} diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java new file mode 100644 index 0000000000..a0604abfb3 --- /dev/null +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java @@ -0,0 +1,27 @@ +/******************************************************************************* + * Copyright (c) 2009, 2021 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; + +/** + * Filters code that is generated for an assert statement. + */ +final class AssertFilter implements IFilter { + + public void filter(final MethodNode methodNode, + final IFilterContext context, final IFilterOutput output) { + // TODO + } + +} From 5cb71a2b5f42ef5a7673cc563be14ac68398bf32 Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Fri, 4 Jun 2021 14:52:37 +0200 Subject: [PATCH 3/5] (WIP) implement --- .../analysis/filter/AssertFilter.java | 51 ++++++++++++++++++- .../internal/analysis/filter/Filters.java | 2 +- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java index a0604abfb3..efae9d44d5 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java @@ -12,6 +12,9 @@ *******************************************************************************/ package org.jacoco.core.internal.analysis.filter; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.FieldInsnNode; import org.objectweb.asm.tree.MethodNode; /** @@ -21,7 +24,53 @@ final class AssertFilter implements IFilter { public void filter(final MethodNode methodNode, final IFilterContext context, final IFilterOutput output) { - // TODO + final Matcher matcher = new Matcher(); + if ("".equals(methodNode.name)) { + for (final AbstractInsnNode i : methodNode.instructions) { + matcher.matchSet(context.getClassName(), i, output); + } + } + for (final AbstractInsnNode i : methodNode.instructions) { + matcher.matchGet(context.getClassName(), i, output); + } + } + + private static class Matcher extends AbstractMatcher { + public void matchSet(final String className, + final AbstractInsnNode start, final IFilterOutput output) { + cursor = start; + nextIsInvoke(Opcodes.INVOKEVIRTUAL, "java/lang/Class", + "desiredAssertionStatus", "()Z"); + nextIs(Opcodes.IFNE); + nextIs(Opcodes.ICONST_1); + nextIs(Opcodes.GOTO); + nextIs(Opcodes.ICONST_0); + nextIsField(Opcodes.PUTSTATIC, className); + if (cursor != null) { + output.ignore(start, cursor); + } + } + + public void matchGet(final String className, + final AbstractInsnNode start, final IFilterOutput output) { + cursor = start; + nextIsField(Opcodes.GETSTATIC, className); + nextIs(Opcodes.IFNE); + if (cursor != null) { + output.ignore(cursor, cursor); + } + } + + private void nextIsField(final int opcode, final String owner) { + nextIs(opcode); + final FieldInsnNode f = (FieldInsnNode) cursor; + if (f != null && f.owner.equals(owner) + && f.name.equals("$assertionsDisabled") + && f.desc.equals("Z")) { + return; + } + cursor = null; + } } } 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 484ccc0032..fbfe522cfd 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 @@ -37,7 +37,7 @@ public static IFilter all() { new TryWithResourcesJavac11Filter(), new TryWithResourcesJavacFilter(), new TryWithResourcesEcjFilter(), new FinallyFilter(), - new PrivateEmptyNoArgConstructorFilter(), + new PrivateEmptyNoArgConstructorFilter(), new AssertFilter(), new StringSwitchJavacFilter(), new StringSwitchFilter(), new EnumEmptyConstructorFilter(), new RecordsFilter(), new AnnotationGeneratedFilter(), new KotlinGeneratedFilter(), From a2ec4859a63f85f3e88ecdea285fc8c4d2664ea6 Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Sat, 5 Jun 2021 10:00:44 +0200 Subject: [PATCH 4/5] (WIP) update changelog --- org.jacoco.doc/docroot/doc/changes.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html index c616997315..435bfd5591 100644 --- a/org.jacoco.doc/docroot/doc/changes.html +++ b/org.jacoco.doc/docroot/doc/changes.html @@ -20,6 +20,13 @@

Change History

Snapshot Build @qualified.bundle.version@ (@build.date@)

+

New Features

+
    +
  • Part of bytecode generated by the Java compilers for assert + statement is filtered out during generation of report + (GitHub #1196).
  • +
+

Fixed bugs

  • Fixed NullPointerException during filtering From 6dd25ffc9a76d6abcfd9d796a704e5412191aea2 Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Sat, 5 Jun 2021 16:25:19 +0200 Subject: [PATCH 5/5] (WIP) refactor --- .../analysis/filter/AbstractMatcherTest.java | 36 +++++++++++++++++++ .../analysis/filter/AbstractMatcher.java | 20 +++++++++++ .../analysis/filter/AssertFilter.java | 18 +++------- 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/AbstractMatcherTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/AbstractMatcherTest.java index be11d7e318..2aeb3d8e1a 100644 --- a/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/AbstractMatcherTest.java +++ b/org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/AbstractMatcherTest.java @@ -137,6 +137,42 @@ public void nextIsVar() { matcher.nextIsVar(Opcodes.ILOAD, "name"); } + @Test + public void nextIsField() { + m.visitInsn(Opcodes.NOP); + m.visitFieldInsn(Opcodes.PUTSTATIC, "owner", "name", "Z"); + + // should set cursor to null when opcode mismatch + matcher.cursor = m.instructions.getFirst(); + matcher.nextIsField(Opcodes.GETSTATIC, "owner", "name", "Z"); + assertNull(matcher.cursor); + + // should set cursor to null when owner mismatch + matcher.cursor = m.instructions.getFirst(); + matcher.nextIsField(Opcodes.PUTSTATIC, "another_owner", "name", "Z"); + assertNull(matcher.cursor); + + // should set cursor to null when name mismatch + matcher.cursor = m.instructions.getFirst(); + matcher.nextIsField(Opcodes.PUTSTATIC, "owner", "another_name", "Z"); + assertNull(matcher.cursor); + + // should set cursor to null when descriptor mismatch + matcher.cursor = m.instructions.getFirst(); + matcher.nextIsField(Opcodes.PUTSTATIC, "owner", "name", + "another_descriptor"); + assertNull(matcher.cursor); + + // should set cursor to next instruction when match + matcher.cursor = m.instructions.getFirst(); + matcher.nextIsField(Opcodes.PUTSTATIC, "owner", "name", "Z"); + assertSame(m.instructions.getLast(), matcher.cursor); + + // should not do anything when cursor is null + matcher.cursor = null; + matcher.nextIsField(Opcodes.PUTSTATIC, "owner", "name", "Z"); + } + @Test public void nextIsInvoke() { m.visitInsn(Opcodes.NOP); 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 837278e974..e6b98fb4de 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 @@ -17,6 +17,7 @@ import org.objectweb.asm.Opcodes; import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.FieldInsnNode; import org.objectweb.asm.tree.MethodInsnNode; import org.objectweb.asm.tree.MethodNode; import org.objectweb.asm.tree.TypeInsnNode; @@ -76,6 +77,25 @@ final void nextIsInvoke(final int opcode, final String owner, cursor = null; } + /** + * Moves {@link #cursor} to next instruction if it is {@link FieldInsnNode} + * with given opcode, owner, name and descriptor, otherwise sets it to + * null. + */ + final void nextIsField(final int opcode, final String owner, + final String name, final String descriptor) { + nextIs(opcode); + if (cursor == null) { + return; + } + final FieldInsnNode f = (FieldInsnNode) cursor; + if (owner.equals(f.owner) && name.equals(f.name) + && descriptor.equals(f.desc)) { + return; + } + cursor = null; + } + final void nextIsVar(final int opcode, final String name) { nextIs(opcode); if (cursor == null) { diff --git a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java index efae9d44d5..044a672932 100644 --- a/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java +++ b/org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AssertFilter.java @@ -14,7 +14,6 @@ import org.objectweb.asm.Opcodes; import org.objectweb.asm.tree.AbstractInsnNode; -import org.objectweb.asm.tree.FieldInsnNode; import org.objectweb.asm.tree.MethodNode; /** @@ -45,7 +44,8 @@ public void matchSet(final String className, nextIs(Opcodes.ICONST_1); nextIs(Opcodes.GOTO); nextIs(Opcodes.ICONST_0); - nextIsField(Opcodes.PUTSTATIC, className); + nextIsField(Opcodes.PUTSTATIC, className, "$assertionsDisabled", + "Z"); if (cursor != null) { output.ignore(start, cursor); } @@ -54,23 +54,13 @@ public void matchSet(final String className, public void matchGet(final String className, final AbstractInsnNode start, final IFilterOutput output) { cursor = start; - nextIsField(Opcodes.GETSTATIC, className); + nextIsField(Opcodes.GETSTATIC, className, "$assertionsDisabled", + "Z"); nextIs(Opcodes.IFNE); if (cursor != null) { output.ignore(cursor, cursor); } } - - private void nextIsField(final int opcode, final String owner) { - nextIs(opcode); - final FieldInsnNode f = (FieldInsnNode) cursor; - if (f != null && f.owner.equals(owner) - && f.name.equals("$assertionsDisabled") - && f.desc.equals("Z")) { - return; - } - cursor = null; - } } }