Do not add members into interfaces with only abstract and clinit methods (#441)

4 files changed, 84 insertions, 25 deletions

diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactoryTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactoryTest.java
index 3ecbd3ad..8dbcc253 100644
--- a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactoryTest.java
+++ b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeArrayStrategyFactoryTest.java
@@ -48,56 +48,72 @@ public class ProbeArrayStrategyFactoryTest {
 
 	@Test
 	public void testClass1() {
-		test(Opcodes.V1_1, 0, false, true);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_1, 0, false, true,
+				true);
+		assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass());
 		assertDataField(InstrSupport.DATAFIELD_ACC);
 		assertInitMethod(false);
 	}
 
 	@Test
 	public void testClass2() {
-		test(Opcodes.V1_2, 0, false, true);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_2, 0, false, true,
+				true);
+		assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass());
 		assertDataField(InstrSupport.DATAFIELD_ACC);
 		assertInitMethod(false);
 	}
 
 	@Test
 	public void testClass3() {
-		test(Opcodes.V1_3, 0, false, true);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_3, 0, false, true,
+				true);
+		assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass());
 		assertDataField(InstrSupport.DATAFIELD_ACC);
 		assertInitMethod(false);
 	}
 
 	@Test
 	public void testClass4() {
-		test(Opcodes.V1_4, 0, false, true);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_4, 0, false, true,
+				true);
+		assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass());
 		assertDataField(InstrSupport.DATAFIELD_ACC);
 		assertInitMethod(false);
 	}
 
 	@Test
 	public void testClass5() {
-		test(Opcodes.V1_5, 0, false, true);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_5, 0, false, true,
+				true);
+		assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass());
 		assertDataField(InstrSupport.DATAFIELD_ACC);
 		assertInitMethod(false);
 	}
 
 	@Test
 	public void testClass6() {
-		test(Opcodes.V1_6, 0, false, true);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_6, 0, false, true,
+				true);
+		assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass());
 		assertDataField(InstrSupport.DATAFIELD_ACC);
 		assertInitMethod(true);
 	}
 
 	@Test
 	public void testClass7() {
-		test(Opcodes.V1_7, 0, false, true);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_7, 0, false, true,
+				true);
+		assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass());
 		assertDataField(InstrSupport.DATAFIELD_ACC);
 		assertInitMethod(true);
 	}
 
 	@Test
 	public void testClass8() {
-		final IProbeArrayStrategy strategy = test(Opcodes.V1_8, 0, false, true);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_8, 0, false, true,
+				true);
+		assertEquals(ClassFieldProbeArrayStrategy.class, strategy.getClass());
 		assertDataField(InstrSupport.DATAFIELD_ACC);
 		assertInitMethod(true);
 
@@ -107,14 +123,18 @@ public class ProbeArrayStrategyFactoryTest {
 
 	@Test
 	public void testInterface7() {
-		test(Opcodes.V1_7, Opcodes.ACC_INTERFACE, true, false);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_7,
+				Opcodes.ACC_INTERFACE, true, false, true);
+		assertEquals(LocalProbeArrayStrategy.class, strategy.getClass());
 		assertNoDataField();
 		assertNoInitMethod();
 	}
 
 	@Test
 	public void testEmptyInterface7() {
-		test(Opcodes.V1_7, Opcodes.ACC_INTERFACE, false, false);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_7,
+				Opcodes.ACC_INTERFACE, false, false, false);
+		assertEquals(NoneProbeArrayStrategy.class, strategy.getClass());
 		assertNoDataField();
 		assertNoInitMethod();
 	}
@@ -122,7 +142,7 @@ public class ProbeArrayStrategyFactoryTest {
 	@Test(expected = UnsupportedOperationException.class)
 	public void testEmptyInterface7StoreInstance() {
 		IProbeArrayStrategy strategy = test(Opcodes.V1_7, Opcodes.ACC_INTERFACE,
-				false, false);
+				false, false, false);
 		strategy.storeInstance(null, false, 0);
 	}
 
@@ -130,7 +150,9 @@ public class ProbeArrayStrategyFactoryTest {
 	public void testInterface8() {
 		cv.isInterface = true;
 		final IProbeArrayStrategy strategy = test(Opcodes.V1_8,
-				Opcodes.ACC_INTERFACE, false, true);
+				Opcodes.ACC_INTERFACE, false, true, true);
+		assertEquals(InterfaceFieldProbeArrayStrategy.class,
+				strategy.getClass());
 		assertDataField(InstrSupport.DATAFIELD_INTF_ACC);
 		assertInitAndClinitMethods();
 
@@ -140,7 +162,9 @@ public class ProbeArrayStrategyFactoryTest {
 
 	@Test
 	public void testEmptyInterface8() {
-		test(Opcodes.V1_8, Opcodes.ACC_INTERFACE, false, false);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_8,
+				Opcodes.ACC_INTERFACE, false, false, false);
+		assertEquals(NoneProbeArrayStrategy.class, strategy.getClass());
 		assertNoDataField();
 		assertNoInitMethod();
 	}
@@ -148,13 +172,27 @@ public class ProbeArrayStrategyFactoryTest {
 	@Test(expected = UnsupportedOperationException.class)
 	public void testEmptyInterface8StoreInstance() {
 		final IProbeArrayStrategy strategy = test(Opcodes.V1_8,
-				Opcodes.ACC_INTERFACE, false, false);
+				Opcodes.ACC_INTERFACE, false, false, false);
 		strategy.storeInstance(null, false, 0);
 	}
 
 	@Test
+	public void testClinitAndAbstractMethodsInterface8() {
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_8,
+				Opcodes.ACC_INTERFACE, true, false, true);
+		assertEquals(LocalProbeArrayStrategy.class, strategy.getClass());
+		assertNoDataField();
+		assertNoInitMethod();
+
+		strategy.storeInstance(cv.visitMethod(0, null, null, null, null), false,
+				0);
+	}
+
+	@Test
 	public void testClinitInterface8() {
-		test(Opcodes.V1_8, Opcodes.ACC_INTERFACE, true, false);
+		final IProbeArrayStrategy strategy = test(Opcodes.V1_8,
+				Opcodes.ACC_INTERFACE, true, false, false);
+		assertEquals(LocalProbeArrayStrategy.class, strategy.getClass());
 		assertNoDataField();
 		assertNoInitMethod();
 	}
@@ -163,7 +201,9 @@ public class ProbeArrayStrategyFactoryTest {
 	public void testClinitAndMethodsInterface8() {
 		cv.isInterface = true;
 		final IProbeArrayStrategy strategy = test(Opcodes.V1_8,
-				Opcodes.ACC_INTERFACE, true, true);
+				Opcodes.ACC_INTERFACE, true, true, true);
+		assertEquals(InterfaceFieldProbeArrayStrategy.class,
+				strategy.getClass());
 		assertDataField(InstrSupport.DATAFIELD_INTF_ACC);
 		assertInitAndClinitMethods();
 
@@ -172,25 +212,32 @@ public class ProbeArrayStrategyFactoryTest {
 	}
 
 	private IProbeArrayStrategy test(int version, int access, boolean clinit,
-			boolean method) {
-		ClassWriter writer = new ClassWriter(0);
+			boolean method, boolean abstractMethod) {
+		final ClassWriter writer = new ClassWriter(0);
 		writer.visit(version, access, "Foo", "java/lang/Object", null, null);
 		if (clinit) {
-			MethodVisitor mv = writer.visitMethod(Opcodes.ACC_PUBLIC
-					| Opcodes.ACC_STATIC, "<clinit>", "()V", null, null);
+			final MethodVisitor mv = writer.visitMethod(
+					Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "<clinit>", "()V",
+					null, null);
 			mv.visitCode();
 			mv.visitInsn(Opcodes.RETURN);
 			mv.visitMaxs(0, 0);
 			mv.visitEnd();
 		}
 		if (method) {
-			MethodVisitor mv = writer.visitMethod(Opcodes.ACC_PUBLIC
-					| Opcodes.ACC_STATIC, "doit", "()V", null, null);
+			final MethodVisitor mv = writer.visitMethod(
+					Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, "doit", "()V",
+					null, null);
 			mv.visitCode();
 			mv.visitInsn(Opcodes.RETURN);
 			mv.visitMaxs(0, 0);
 			mv.visitEnd();
 		}
+		if (abstractMethod) {
+			final MethodVisitor mv = writer.visitMethod(Opcodes.ACC_ABSTRACT,
+					"foo", "()V", null, null);
+			mv.visitEnd();
+		}
 		writer.visitEnd();
 
 		final IProbeArrayStrategy strategy = ProbeArrayStrategyFactory
diff --git a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeCounterTest.java b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeCounterTest.java
index ed304d27..0b831be5 100644
--- a/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeCounterTest.java
+++ b/org.jacoco.core.test/src/org/jacoco/core/internal/instr/ProbeCounterTest.java
@@ -18,6 +18,7 @@ import static org.junit.Assert.assertTrue;
 
 import org.junit.Before;
 import org.junit.Test;
+import org.objectweb.asm.Opcodes;
 
 /**
  * Unit tests for {@link ProbeCounter}.
@@ -50,6 +51,12 @@ public class ProbeCounterTest {
 	}
 
 	@Test
+	public void testVisitAbstractMethod() {
+		counter.visitMethod(Opcodes.ACC_ABSTRACT, "foo", null, null, null);
+		assertFalse(counter.hasMethods());
+	}
+
+	@Test
 	public void testVisitMethod() {
 		assertNull(counter.visitMethod(0, "foo", null, null, null));
 		assertTrue(counter.hasMethods());
diff --git a/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeCounter.java b/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeCounter.java
index d45a8f9d..66adf029 100644
--- a/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeCounter.java
+++ b/org.jacoco.core/src/org/jacoco/core/internal/instr/ProbeCounter.java
@@ -13,6 +13,7 @@ package org.jacoco.core.internal.instr;
 
 import org.jacoco.core.internal.flow.ClassProbesVisitor;
 import org.jacoco.core.internal.flow.MethodProbesVisitor;
+import org.objectweb.asm.Opcodes;
 
 /**
  * Internal class to remember the total number of probes required for a class.
@@ -30,7 +31,8 @@ class ProbeCounter extends ClassProbesVisitor {
 	@Override
 	public MethodProbesVisitor visitMethod(final int access, final String name,
 			final String desc, final String signature, final String[] exceptions) {
-		if (!InstrSupport.CLINIT_NAME.equals(name)) {
+		if (!InstrSupport.CLINIT_NAME.equals(name)
+				&& (access & Opcodes.ACC_ABSTRACT) == 0) {
 			methods = true;
 		}
 		return null;
@@ -46,8 +48,8 @@ class ProbeCounter extends ClassProbesVisitor {
 	}
 
 	/**
-	 * @return <code>true</code> if the class has other methods than a static
-	 *         initializer
+	 * @return <code>true</code> if the class has non-abstract methods other
+	 *         than a static initializer
 	 */
 	boolean hasMethods() {
 		return methods;
diff --git a/org.jacoco.doc/docroot/doc/changes.html b/org.jacoco.doc/docroot/doc/changes.html
index e62b4a80..51a58ed3 100644
--- a/org.jacoco.doc/docroot/doc/changes.html
+++ b/org.jacoco.doc/docroot/doc/changes.html
@@ -29,6 +29,9 @@
 
 <h3>Fixed Bugs</h3>
 <ul>
+  <li>Do not add useless members into Java 8 interfaces that have only interface
+      initialization and abstract methods
+      (GitHub <a href="https://github.com/jacoco/jacoco/issues/441">#441</a>).</li>
   <li>Fix instrumentation to not violate Java Virtual Machine Specification regarding
       initialization of final fields, otherwise <code>IllegalAccessError</code>
       will be thrown starting from OpenJDK 9 EA b127