Coverage of default methods

[realdadfish]

Steps to reproduce

JaCoCo version: 0.8.3
Operating system: macOS
Tool integration: Gradle

interface Callback {
     fun onFoo() = Unit
     fun onBar() = Unit
     fun onBaz() = Unit
}

class CallbackImpl : Callback {
      fun onFoo() { /* ... */ }
}

Expected behaviour

I'd like to see that Jacoco reports 100% coverage, because all methods in CallbackImpl are in fact covered and the default method implementations might technically be part of the class, but serve no purpose.

Actual behaviour

For a test of CallbackImpl, Jacoco will tell me that 66% of the class' methods are not covered (and one instruction per missed method as well).

One could argue that one could test to call the other methods to check if they lead to no behaviour change, but being default implementations from some base class with no actual implementation in this class I don't see why I should unit-test them here.

[Godin]

In case of Example.kt

interface Callback {
  fun onFoo() = Unit // line 2
  fun onBar() = Unit // line 3
}

class CallbackImpl : Callback { // line 6
  override fun onFoo() { } // line 7
}

kotlinc version 1.3.60 for non-overridden method onBar generates method
(javap -v -p CallbackImpl.class)

  public void onFoo();
    descriptor: ()V
    flags: ACC_PUBLIC
    Code:
      stack=0, locals=1, args_size=1
         0: return
      LineNumberTable:
        line 7: 0

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

  public void onBar();
    descriptor: ()V
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: invokestatic  #19                 // Method Callback$DefaultImpls.onBar:(LCallback;)V
         4: return
      LineNumberTable:
        line 6: 0

which delegates to default implementation (javap -v -p Callback$DefaultImpls)

  public static void onFoo(Callback);
    descriptor: (LCallback;)V
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=0, locals=1, args_size=1
         0: return
      LineNumberTable:
        line 2: 0

  public static void onBar(Callback);
    descriptor: (LCallback;)V
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=0, locals=1, args_size=1
         0: return
      LineNumberTable:
        line 3: 0

and which contains line number (6 in the above example),
whereas for example methods toString and hashCode generated for data classes does not contain line number and therefore filtered by JaCoCo - see #689. I don't see reasons for such inconsistency and for presence of line number in such delegating methods.

So @realdadfish could you please report this to developers of Kotlin compiler in their issue tracker with cross-reference to/from this ticket?

[Godin]

@realdadfish @sellmair have you reported this to the developers of Kotlin compiler in their issue tracker?

[realdadfish]

@Godin No, but I actually asked about a year ago on the JetBrains (and Google) bugtracker to find some common ground for improving Kotlin support in Jacoco, without getting feedback (https://youtrack.jetbrains.com/issue/KT-29180). Last week on KotlinConf I stepped up and specifically asked Andrey again if there are any plans to get more in touch and he denied ("except bugfixes"), however I think even those are not very high priority for them sadly.

For example in recent Android Studio versions (up until recent 4.0 canaries) the inline display of Jacoco coverage results is broken / works unreliably, while the built-in coverage tool shows no issues (but is also of course far away from what Jacoco can provide).

After that event I'm frankly a bit disenchanted about the whole thing.

[Godin]

@qwwdfsad I suppose that the answer will be the same as #1002 (comment) , but anyway should ask:
can Kotlin compiler be updated to not produce line numbers in generated methods shown above?
or mark them as ACC_BRIDGE?

[qwwdfsad]

Surprisingly, it won't be the same :) The problem, in general, can be rephrased as "How can we distinguish auto-generated methods (that do not carry any new semantic or cognitive load) from the user-defined ones?" This is a problem worth addressing, though it is not clear how exactly we are going to do it.

We definitely want to add line numbers to all auto-generated methods (so stracktraces are always navigable), but in return, we will indicate that methods are compiler-generated.

Two options here:

• Add ACC_BRIDGE to all such methods. Should work, though requires additional investigation. E.g. whether it changes an observable behaviour of the method, does it change reflection interoperability etc.
• Add some kind of @javax.annotation.Generated with binary retention as a backup plan (not this particular one though because of the licensing issues).

We'll start discussing it, but it's unlikely to be implemented until the autumn and, unfortunately, I cannot make a commitment here.

As a temporary Kotlin-specific filter, it's probably worth attempting to match the byte-code against INVOKESTATIC Interface$DefaultImpls.method(Interface)

[Godin]

@qwwdfsad thank you for looking into this and for the response! ❤️

Add ACC_BRIDGE to all such methods. Should work, though requires additional investigation.

Agree.

Add some kind of @javax.annotation.Generated with binary retention as a backup plan (not this particular one though because of the licensing issues).

I would say that not javax.annotation.Generated mainly because it is not retained in class files 😉

As a note: if here or in other places Kotlin compiler will be adding annotation with RetentionPolicy.RUNTIME and RetentionPolicy.CLASS whose simple name contains Generated, then they already will be filtered thanks to already existing AnnotationGeneratedFilter. Known to us examples of such annotations - groovy.transform.Generated, lombok.Generated, org.immutables.value.Generated and org.apache.avro.specific.AvroGenerated.

There also might be other options, e.g. maybe SMAPs (JSR-045).

We'll start discussing it, but it's unlikely to be implemented until the autumn and, unfortunately, I cannot make a commitment here.

As a temporary Kotlin-specific filter, it's probably worth attempting to match the byte-code against INVOKESTATIC Interface$DefaultImpls.method(Interface)

This confirms my guesses, so draft of implementation was done already - see #1012 😉

At the same time, you probably know - "there is nothing more permanent than temporary" 😉 so thanks a lot for triggering discussions 👍 is there any ticket in Kotlin tracker about this which can be followed?

[bguns]

I seem to have run into a problem with this, although I'm not sure entirely sure whether it's a bug or a feature/limitation. I'm leaning towards the latter, as this looks very tricky to solve without further support from the Kotlin team.

Currently, it looks like the filter as implemented will only filter out DefaultImpl functions without any parameters.

It checks if the first instructions are ALOAD0 followed by an immediate INVOKESTATIC. However, any default function with actual function parameters will have to put these parameters on the stack before the INVOKESTATIC call, and will thus have ALOAD1/2/... calls in between ALOAD0 and INVOKESTATIC. Furthermore, these will not necessarily be the first instructions in the function, as Kotlin injects a checkNotNullParameter call for each non-nullable function parameter at the start of the function.

In my codebase I have an interface with several functions with default implementations, some without parameters, others with parameters. Those without parameters are correctly excluded from the report of any implementing classes. However, those that have parameters and are unused in the implementing class are reported as having 0% coverage.

I don't know if this is currently solvable. A quick and dirty test seems to indicate that any overridden implementation of foo(...) which contains a super.foo(...) call somewhere in it, will end up containing the same block of bytecode for an INVOKESTATIC call to the $DefaultImpls as the bytecode generated for a non-overridden function, so I personally don't see a way to distinguish between the two.

However, the release notes of 0.8.6 state that this problem is fixed in general, which appears to not be the case.

[Godin]

@bguns you're right - unfortunately methods with parameters were overlooked during implementation of #1012 in 0.8.6 so I created #1137 for 0.8.7. And thank you for reporting this 👍

---

And you're right that

interface I {
  fun m() = Unit
}

class C1 : I {
}

class C2 : I {
  override fun m() { super.m() }
}

class C3 : I {
  override fun m() {
    super.m()
    println()
  }
}

as long as compiler produces the same bytecode for generated method in C1 as for hand-written method in C2 they are indistinguishable for JaCoCo - see https://youtu.be/g-hvobCqncY?t=2126

However both are distinguishable from C3 as it contains more than just super call.

Hope one day implementation of https://youtrack.jetbrains.com/issue/KT-41906 or similar will make C1 distinguishable from C2. And for the time being there are only two options: either both generated method in C1 and hand-written method in C2 are filtered out from JaCoCo report, or both are present. First option seems better for the users, especially given that Kotlin compiler shows warning "Redundant overriding method" for the hand-written method in C2 with only super.

[qwwdfsad]

Hope one day implementation of https://youtrack.jetbrains.com/issue/KT-41906

Unfortunately, we are still not sure if we are going to implement this at all (I'll post the rationale in KT ticket as soon as it will be complete).

Meanwhile, you already can distinguish auto-generated methods from handwritten using kotlinx-metadata.

You can read all the methods and method m will be present for class C2, but not C1:

val a = C1::class.java.declaredAnnotations.filterIsInstance<Metadata>().single()
val metadata = KotlinClassMetadata.read(
            KotlinClassHeader(a.kind, a.metadataVersion, a.bytecodeVersion, a.data1,
            a.data2, a.extraString, a.packageName, a.extraInt))!! as KotlinClassMetadata.Class
println(metadata.toKmClass().functions.map { it.name }.toList())

Also, we have a repository with examples https://github.com/udalov/kotlinx-metadata-examples that read metadata from byte[] representation of classfiles

[Godin]

using kotlinx-metadata

@qwwdfsad thank you for the information, we are already aware of kotlinx-metadata- see #552 (comment) , but unfortunately it requires Java 6, whereas so far JaCoCo is Java 5 compatible

[qwwdfsad]

Okay, good to know. You probably still can use it only on Kotlin-related code paths (or even only in Kotlin-specific filters) because Kotlin itself is compatible with Java 6 and above. Then only Kotlin-specific part of JaCoCo will require Java 6

[pexa-asekar]

@Godin This coverage issue is recurring for the Kotlin version 1.6.10

[Godin]

@pexa-asekar yes we know - ticket #1137 is still open as you can see by yourself