jacoco / jacoco

:microscope: Java Code Coverage Library
https://www.jacoco.org/jacoco/
Other
4.21k stars 1.15k forks source link

Jacoco includes coverage of pattern switch exception paths which may not be possible to cover. #1514

Closed WarpspeedSCP closed 1 year ago

WarpspeedSCP commented 1 year ago

Jacoco currently reports coverage on MatchException handling paths of Java 21 pattern matching switch blocks. Match exceptions are not easy to trigger when writing normal code, and I don't believe Jacoco should be reporting coverage for such exception paths.

Here's an example of the kind of bytecode pattern Jacoco seems to be labelling as not covered:

https://github.com/WarpspeedSCP/monadics21/blob/20be2fde393f59156f2097cff49814d39b73c7c5/src/main/java/dev/wscp/monadics/result/Result.java#L102-L112

    /**
     * Extracts the error of a result. throws an {@link UnwrapException} if there is no error value to return.
     * @return The unwrapped error if this is Err.
     * @throws UnwrapException if this is Ok.
     */
    default E unwrapError() {
        return switch (this) {
            case Err(E err) -> err;
            default -> throw new UnwrapException("Attempted to unwrapErr an ok value ");
        };
    }

This is the bytecode for this method:

  public default E unwrapError();
    descriptor: ()Ljava/lang/Object;
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=4, locals=6, args_size=1
         0: aload_0
         1: dup
         2: invokestatic  #3                  // Method java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
         5: pop
         6: astore_1
         7: iconst_0
         8: istore_2
         9: aload_1
        10: iload_2
        11: invokedynamic #67,  0             // InvokeDynamic #3:typeSwitch:(Ljava/lang/Object;I)I
        16: lookupswitch  { // 1
                       0: 36
                 default: 59
            }
        36: aload_1
        37: checkcast     #13                 // class dev/wscp/monadics/result/Err
        40: astore_3
        41: aload_3
        // v----- This is the only instruction in the Err branch of the pattern switch,  
        //           and this is the only place where if an exception is thrown, execution will go to a match block.
        42: invokevirtual #38                 // Method dev/wscp/monadics/result/Err.error:()Ljava/lang/Object;
        45: astore        5 
        47: aload         5
        49: astore        4
        51: aload         4
        53: checkcast     #36                 // class java/lang/Object
        56: goto          69
        59: new           #41                 // class dev/wscp/monadics/util/UnwrapException
        62: dup
        63: ldc           #68                 // String Attempted to unwrapErr an ok value
        65: invokespecial #45                 // Method dev/wscp/monadics/util/UnwrapException."<init>":(Ljava/lang/String;)V
        68: athrow
        69: areturn
        70: astore_1
        71: new           #28                 // class java/lang/MatchException
        74: dup
        75: aload_1
        76: invokevirtual #60                 // Method java/lang/Throwable.toString:()Ljava/lang/String;
        79: aload_1
        80: invokespecial #30                 // Method java/lang/MatchException."<init>":(Ljava/lang/String;Ljava/lang/Throwable;)V
        83: athrow
      Exception table:
         from    to  target type
            42    45    70   Class java/lang/Throwable
      // ...

Note that Java inserts a synthetic try-catch block around the Err case of the switch to catch any potential exceptions during destructuring. However, it is impossible to actually throw exceptions here unless the record getter implementation for Err.error is explicitly implemented to be incorrect. The catch block of this try statement will never be triggered under normal circumstances.

Steps to reproduce

Expected behaviour

Jacoco should ignore the control flow that corresponds to MatchException, as it would be nearly impossible to trigger that code normally.

Actual behaviour

Jacoco reports the improbable MatchException case as uncovered due to the natural inability to actually cause that control flow path to be taken.

Godin commented 1 year ago

Duplicate of #1473 which is fixed for the upcoming 0.8.11 version

WarpspeedSCP commented 1 year ago

oh, good to know.