RecordsFilter should filter out accessors (#1393)

org.jacoco.core.test.validation.java16/src/org/jacoco/core/test/validation/java16/targets/RecordsTarget.java

@@ -21,14 +21,14 @@ public class RecordsTarget {
 	record WithoutFields() { // assertFullyCovered()
 	}
 
-	record WithFields( // assertPartlyCovered()
+	record WithFields( // assertFullyCovered()
 			int x // assertEmpty()
 	) {
 	}
 
 	record WithCustomMethods(int x) { // assertFullyCovered()
 		public int x() {
-			return x; // assertNotCovered()
+			return x; // assertEmpty()
 		}
 
 		public String toString() {

org.jacoco.core.test/src/org/jacoco/core/internal/analysis/filter/RecordsFilterTest.java

@@ -172,4 +172,74 @@ public void should_not_filter_non_records() {
 		assertIgnored();
 	}
 
+	@Test
+	public void should_filter_field_int() {
+		context.superClassName = "java/lang/Record";
+		final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0,
+				"foo", "()Z", null, null);
+		m.visitVarInsn(Opcodes.ALOAD, 0);
+		m.visitFieldInsn(Opcodes.GETFIELD, "Dunno", "foo", "Z");
+		m.visitInsn(Opcodes.IRETURN);
+
+		filter.filter(m, context, output);
+
+		assertMethodIgnored(m);
+	}
+
+	@Test
+	public void should_filter_field_object() {
+		context.superClassName = "java/lang/Record";
+		final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0,
+				"foo", "()Ljava/lang/String;", null, null);
+		m.visitVarInsn(Opcodes.ALOAD, 0);
+		m.visitFieldInsn(Opcodes.GETFIELD, "Dunno", "foo", "Ljava/lang/String");
+		m.visitInsn(Opcodes.LRETURN);
+
+		filter.filter(m, context, output);
+
+		assertMethodIgnored(m);
+	}
+
+	@Test
+	public void should_not_filter_redirect_method() {
+		context.superClassName = "java/lang/Record";
+		final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0,
+				"foo", "()Ljava/lang/String;", null, null);
+		m.visitVarInsn(Opcodes.ALOAD, 0);
+		// The method name is different from the field name
+		m.visitFieldInsn(Opcodes.GETFIELD, "Dunno", "bar", "Ljava/lang/String");
+		m.visitInsn(Opcodes.LRETURN);
+
+		filter.filter(m, context, output);
+
+		assertIgnored();
+	}
+
+	@Test
+	public void should_not_filter_noreturn_method() {
+		context.superClassName = "java/lang/Record";
+		final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0,
+				"foo", "()Ljava/lang/String;", null, null);
+		m.visitVarInsn(Opcodes.ALOAD, 0);
+		m.visitFieldInsn(Opcodes.GETFIELD, "Dunno", "foo", "Ljava/lang/String");
+
+		filter.filter(m, context, output);
+
+		assertIgnored();
+	}
+
+	@Test
+	public void should_not_filter_other_method() {
+		context.superClassName = "java/lang/Record";
+		final MethodNode m = new MethodNode(InstrSupport.ASM_API_VERSION, 0,
+				"foo", "()Ljava/lang/String;", null, null);
+		m.visitVarInsn(Opcodes.ALOAD, 0);
+		m.visitFieldInsn(Opcodes.GETFIELD, "Dunno", "foo", "Ljava/lang/String");
+		// Not a return statement
+		m.visitFieldInsn(Opcodes.GETFIELD, "Dunno", "foo", "Ljava/lang/String");
+
+		filter.filter(m, context, output);
+
+		assertIgnored();
+	}
 }

org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/RecordsFilter.java

@@ -14,6 +14,7 @@
 
 import org.objectweb.asm.Handle;
 import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.tree.FieldInsnNode;
 import org.objectweb.asm.tree.InvokeDynamicInsnNode;
 import org.objectweb.asm.tree.MethodNode;
 
@@ -30,7 +31,8 @@ public void filter(final MethodNode methodNode,
 		}
 		final Matcher matcher = new Matcher();
 		if (matcher.isEquals(methodNode) || matcher.isHashCode(methodNode)
-				|| matcher.isToString(methodNode)) {
+				|| matcher.isToString(methodNode)
+				|| matcher.isFieldAccessor(methodNode)) {
 			output.ignore(methodNode.instructions.getFirst(),
 					methodNode.instructions.getLast());
 		}
@@ -58,6 +60,48 @@ boolean isHashCode(final MethodNode m) {
 			return cursor != null;
 		}
 
+		/**
+		 * Criteria: method name == field name, only three instructions (aload0,
+		 * getField, return), and note that this class only happens in a record,
+		 * so it's safe to assume that this is the record field accessor
+		 * generated. It may happen that the code is explicitly written by the
+		 * developer and is intentionally kept the same as the default generated
+		 * format, but that's just trivial code, and it still makes sense to
+		 * filter them out anyway.
+		 * <p>
+		 * Exception: if the code is compiled within IntelliJ IDEA's Java
+		 * instrumentation, there will be extra null-assertion instructions
+		 * after the getField instruction. This case is <emph>ignored</emph>.
+		 */
+		boolean isFieldAccessor(final MethodNode m) {
+			if (!m.desc.startsWith("()")) {
+				// Method with parameter(s)
+				return false;
+			}
+			firstIsALoad0(m);
+			nextIs(Opcodes.GETFIELD);
+			if (!(cursor instanceof FieldInsnNode)) {
+				return false;
+			}
+			if (!((FieldInsnNode) cursor).name.equals(m.name)) {
+				return false;
+			}
+			next();
+			if (cursor == null) {
+				return false;
+			}
+			switch (cursor.getOpcode()) {
+			case Opcodes.IRETURN:
+			case Opcodes.LRETURN:
+			case Opcodes.FRETURN:
+			case Opcodes.DRETURN:
+			case Opcodes.ARETURN:
+				return true;
+			default:
+				return false;
+			}
+		}
+
 		boolean isEquals(final MethodNode m) {
 			if (!"equals".equals(m.name)
 					|| !"(Ljava/lang/Object;)Z".equals(m.desc)) {

org.jacoco.doc/docroot/doc/changes.html

@@ -30,6 +30,9 @@ <h3>New Features</h3>
   <li>Add parameter to include the current project in the <code>report-aggregate</code>
       Maven goal
       (GitHub <a href="https://github.com/jacoco/jacoco/issues/1007">#1007</a>).</li>
+  <li>Component accessors generated by the Java compilers for records are filtered
+      out during generation of report. Contributed by Tesla Zhang
+      (GitHub <a href="https://github.com/jacoco/jacoco/issues/1393">#1393</a>).</li>
 </ul>
 
 <h3>Non-functional Changes</h3>