Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kotlin compiler should mark anonymous classes for callable references as synthetic #791

Closed
thimmwork opened this issue Nov 19, 2018 · 8 comments

Comments

@thimmwork
Copy link

thimmwork commented Nov 19, 2018

Kotlin generates additional classes when using reflection. These classes are neither marked as covered nor filtered out by jacoco.

Steps to reproduce

Add maven dependency

        <dependency>
            <groupId>org.jetbrains.kotlin</groupId>
            <artifactId>kotlin-reflect</artifactId>
            <version>${kotlin.version}</version>
        </dependency>

and create a function which gets a reflect.Method reference to a function like this:

fun minimalFunction() {
    // this will make jacoco fail due to a class generated by kotlin
    ::minimalFunction.javaMethod
}

when using .javaMethod, Kotlin generates an anonymous class which will be checked for line coverage by jacoco

JaCoCo version: 0.8.2
Operating system: any (tested on Mac OS 10.13.6 and Ubuntu)
Tool integration: Maven

Expected behaviour

Code generated by Kotlin compiler should be marked as covered or filtered out

Actual behaviour

Line coverage for generated class is 0.00
Rule violated for class SomeController.getAddressWithGeoInformation.javaMethod.1: lines covered ratio is 0.00, but expected minimum is 0.85

image

This issue may or may not affect other reflection mechanisms as well. I tested only .javaMethod as we use this for generating HATEOAS links in our web controller classes.

@Godin
Copy link
Member

Godin commented Nov 19, 2018

Generation of class is not caused by .javaMethod, but by a method reference:

fun main(args: Array<String>) {
    println(::main)
}

@thimmwork following questions must be answered before considering implementation of filter: why Kotlin compiler adds line numbers into generated class? and most importantly why it is not marked as synthetic? CC @goodwinnk

@Godin Godin added the feedback pending Further information is requested label Nov 19, 2018
@goodwinnk
Copy link
Contributor

goodwinnk commented Nov 26, 2018

I have just created https://youtrack.jetbrains.com/issue/KT-28453 and will let you know about updates.

@Godin
Copy link
Member

Godin commented Jan 7, 2019

@goodwinnk I noticed that there is already change that marks method as synthetic. Thanks a lot for this! 👍 ❤️

I'm wondering why was decided to keep line number in this method, while you was questioning its necessity in description of KT-28453 ? Could also be noted that the only known to me so far examples of synthetic methods with line numbers are for default arguments and for suspending functions, and they should not be filtered by JaCoCo - see #774 and #804 , whereas all methods without line numbers should be filtered - see yours #689 And hence maybe removal of line number will make things more consistent in design of code generator? 😉 And in case of such consistency we can simplify code of filtering.

BTW maybe you can point me on other examples of synthetic methods in Kotlin to grow our validation test suite and knowledge? Will appreciate this.

@goodwinnk
Copy link
Contributor

@Godin You're right that current behavior is inconsistent. I have created another issue about removing line numbers from invoke() methods generated for function references: https://youtrack.jetbrains.com/issue/KT-29483.

@Godin
Copy link
Member

Godin commented Jan 28, 2019

@goodwinnk thanks!

@Godin
Copy link
Member

Godin commented Mar 1, 2019

@thimmwork could you please try Kotlin 1.3.30 EAP that contains fix for KT-28453 ? According to my tests callable references do not cause partial coverage after this fix.

@thimmwork
Copy link
Author

@Godin I updated kotlin-maven-plugin and stdlib to 1.3.30-eap-11 and it works. 😃
Jacoco no longer reports the generated class as uncovered.
However, for some reason it still reports the source line with the method reference as uncovered.
But this may or may not be a different issue. Also it is easy to find a workaround as it now affects hand-written classes.

Thank you very much @goodwinnk and @Godin !

@Godin Godin changed the title Add filter for Kolin-reflect Kotlin compiler should mark anonymous classes for callable references as synthetic Apr 17, 2019
@Godin Godin added component: core.filters and removed feedback pending Further information is requested labels Apr 17, 2019
@Godin Godin added this to To Do in Filtering via automation Apr 17, 2019
@Godin
Copy link
Member

Godin commented Apr 17, 2019

Added validation test which shows that after upgrade to Kotlin 1.3.30 callable references do not cause partial coverage - d3c4765

@Godin Godin closed this as completed Apr 17, 2019
Filtering automation moved this from To Do to Done Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Filtering
  
Done
Development

No branches or pull requests

3 participants