Add filter for Java assert statement

[Godin]

For Example.java

//
class Example {

    void example(boolean b) {
        assert(b);
    }

    public static void main(String[] args) {
        new Example().example(true);
    }

}

execution of

javac Example.java
java -ea -javaagent:jacoco/lib/jacocoagent.jar Example
java -jar jacoco/lib/jacococli.jar report jacoco.exec --classfiles Example.class --sourcefiles . --html report

before this change produces

and after

[marchof]

What instructions are left on this line? Shouldn't it be empty?

[Godin]

@marchof we ignore only part of static initializer that corresponds to initialization of field $assertionsDisabled, so in this case there is also return with line number of class:

  static {};
    descriptor: ()V
    flags: (0x0008) ACC_STATIC
    Code:
      stack=1, locals=0, args_size=0
         0: ldc           #7                  // class org/jacoco/core/test/validation/java5/targets/AssertTarget
         2: invokevirtual #8                  // Method java/lang/Class.desiredAssertionStatus:()Z
         5: ifne          12
         8: iconst_1
         9: goto          13
        12: iconst_0
        13: putstatic     #2                  // Field $assertionsDisabled:Z
        16: return
      LineNumberTable:
        line 20: 0

[marchof]

I wonder whether we should also ignore the return if there is no other code. Otherwise we have this line highlighted only if there is a assertion statement in this class.

[marchof]

Also if I add a static initializer the return is accounted to that line and the highlighting of the class definition disappears. I think it is more consistent if we ignore the return statement if there is no other code:

static Object C = new Object();

[Godin]

this line highlighted only if there is a assertion statement

Also at least default constructor has line of class declaration and part of static initializer generated for enums.

if we ignore the return statement if there is no other code

This will also exclude line of explicitly written empty static initializer in class with assert statements:

//
class Example { // line 2
    static {
    } // line 4

    void example(boolean b) {
        assert(b);
    }
}

  Example();
    descriptor: ()V
    flags: (0x0000)
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokespecial #1                  // Method java/lang/Object."<init>":()V
         4: return
      LineNumberTable:
        line 2: 0

  static {};
    descriptor: ()V
    flags: (0x0008) ACC_STATIC
    Code:
      stack=1, locals=0, args_size=0
         0: ldc           #5                  // class Example
         2: invokevirtual #6                  // Method java/lang/Class.desiredAssertionStatus:()Z
         5: ifne          12
         8: iconst_1
         9: goto          13
        12: iconst_0
        13: putstatic     #2                  // Field $assertionsDisabled:Z
        16: return
      LineNumberTable:
        line 2: 0
        line 4: 16

I believe that our users are more concerned about branch that without this change is displayed on a line of class declaration. And not sure whether they will be concerned about just highlighting of this line or about absense of highlighting for explicitly written empty static initializers.

Should we then also for consistency also filter explicitly written empty static initializers in absence of assert statements? Is it worth it? I'm not against both, just wondering how deep this rabbit hole can go to not overlook something - wondering if there might be other cases that we currently overlooking?

[marchof]

Ok, looks like we will end up with too many corner cases. We leave it like it is!

[morganga]

Should we be ignoring the entire assert test here?
output.ignore(cursor, cursor) it's only ignoring the branch instruction, not the assert test.

I still get a "1 of 2 branches missed" for my assert statements. The only way to cover both branches is to run the test twice, once with assertions disabled, then again with assertions enabled, all in the same test suite.

How about output.ignore(cursor, ((JumpInsnNode) cursor).label);

I've tested the above code fragment and it marks the assert line as green, without complaining about 1 of 2 branches missed. (it also doesn't think the line has multiple branches either)