Skip to content

Add filter for bytecode generated by Kotlin serialization compiler plugin #1885

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.

Sign up for GitHub

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

Jump to bottom
Merged
Godin merged 7 commits into from
Oct 7, 2025
Merged

Add filter for bytecode generated by Kotlin serialization compiler plugin #1885

Godin merged 7 commits into from
Oct 7, 2025
+379 −0

Conversation

@Godin
Copy link
Member

@Godin Godin commented Apr 29, 2025 •
edited
Loading

Closes #1855

@Godin Godin self-assigned this Apr 29, 2025
@Godin Godin added this to Filtering Apr 29, 2025
@github-project-automation github-project-automation bot moved this to Awaiting triage in Filtering Apr 29, 2025
@Godin Godin linked an issue Apr 29, 2025 that may be closed by this pull request
Code coverage incorrect for Kotlin data classes with kotlinx.serialization in JaCoCo 0.8.13 #1855
Closed
@Godin Godin moved this from Awaiting triage to In Progress in Filtering Apr 29, 2025
@Godin Godin force-pushed the kotlin_serialization branch from 62da77b to a1b4f51 Compare April 29, 2025 22:50
@Godin Godin force-pushed the kotlin_serialization branch from 0abbc62 to 5353fdd Compare August 25, 2025 20:10
@Godin Godin added this to the 0.8.14 milestone Aug 25, 2025
@Godin Godin marked this pull request as ready for review August 25, 2025 21:35
@Godin Godin requested a review from leveretka August 25, 2025 21:35
leveretka
leveretka approved these changes Sep 24, 2025
View reviewed changes
Copy link
Collaborator

@leveretka leveretka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for looking into the issue.

I was thinking if we could somehow prevent such regression as it's tough thinking in advance of other possible compiler plugins and their combinations. On the other hand, serialization plugin is a pretty big one and we now have a test for it.

org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinSerializableFilter.java
nextIs(Opcodes.ARETURN);
return cursor != null
&& getStaticInstruction.name.equals("INSTANCE")
&& (lineNumberInstruction instanceof LineNumberNode)
Copy link
Collaborator

@leveretka leveretka Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly we can't use pattern matching on instanceOf here (Java 14) :(

@Godin Godin enabled auto-merge (squash) October 7, 2025 22:25
@Godin Godin merged commit d07bc6b into jacoco:master Oct 7, 2025
36 checks passed
@Godin Godin deleted the kotlin_serialization branch October 7, 2025 22:33
@github-project-automation github-project-automation bot moved this from In Progress to Done in Filtering Oct 7, 2025
@ajeihala
Copy link

ajeihala commented Oct 8, 2025

Thanks a lot @Godin for taking care of this MR as Kotlinx Serialization caused a lot of troubles for me with synthetic methods and coverage.

I use a snapshot Jacoco version and today it broke for my project perhaps due to this commit (or another about ASM update). Please take a look if it's something relevant for a fix:

...
Caused by: java.io.IOException: Error while analyzing BillingAddressFieldType$Companion.class with JaCoCo 0.8.14.202510071157/0981128.
        at org.jacoco.core.analysis.Analyzer.analyzerError(Analyzer.java:163)
        at org.jacoco.core.analysis.Analyzer.analyzeClass(Analyzer.java:135)
        at org.jacoco.core.analysis.Analyzer.analyzeClass(Analyzer.java:158)
        at org.jacoco.core.analysis.Analyzer.analyzeAll(Analyzer.java:195)
        at org.jacoco.ant.ReportTask.createBundle(ReportTask.java:573)
        at org.jacoco.ant.ReportTask.createReport(ReportTask.java:545)
        at org.jacoco.ant.ReportTask.execute(ReportTask.java:496)
        ... 155 more
Caused by: java.lang.NullPointerException: Cannot invoke "org.objectweb.asm.tree.AbstractInsnNode.getPrevious()" because "this.cursor" is null
        at org.jacoco.core.internal.analysis.filter.KotlinSerializableFilter$Matcher.match(KotlinSerializableFilter.java:79)
        at org.jacoco.core.internal.analysis.filter.KotlinSerializableFilter.filter(KotlinSerializableFilter.java:44)
        at org.jacoco.core.internal.analysis.filter.FilterSet.filter(FilterSet.java:31)
        at org.jacoco.core.internal.analysis.filter.Filters$1.filter(Filters.java:45)
        at org.jacoco.core.internal.analysis.ClassAnalyzer.addMethodCoverage(ClassAnalyzer.java:131)
        at org.jacoco.core.internal.analysis.ClassAnalyzer.access$100(ClassAnalyzer.java:38)
        at org.jacoco.core.internal.analysis.ClassAnalyzer$1.accept(ClassAnalyzer.java:115)
        at org.jacoco.core.internal.flow.ClassProbesAdapter$2.visitEnd(ClassProbesAdapter.java:91)
        at org.objectweb.asm.ClassReader.readMethod(ClassReader.java:1516)
        at org.objectweb.asm.ClassReader.accept(ClassReader.java:745)
        at org.objectweb.asm.ClassReader.accept(ClassReader.java:425)
        at org.jacoco.core.analysis.Analyzer.analyzeClass(Analyzer.java:117)
        at org.jacoco.core.analysis.Analyzer.analyzeClass(Analyzer.java:133)

The class it complains about BillingAddressFieldType$Companion.class has an empty companion object declared without any body/fields (just companion object).

@Godin
Copy link
Member Author

Godin commented Oct 8, 2025

@ajeihala first of all thank you for testing snapshot version and for the report ❤
while there is already another report #1969 with the same stack trace
your example is different and

empty companion object declared without any body/fields (just companion object)

like

@Serializable
class EmptyCompanion {
    companion object
}

does not seem to be enough to trigger NPE, so could you please provide source code of your BillingAddressFieldType and/or class files BillingAddressFieldType.class and BillingAddressFieldType$Companion.class entirely to make sure that we won't miss some corner-case again?

@ajeihala
Copy link

ajeihala commented Oct 8, 2025

Perhaps the enum class makes the difference.

With this

@Serializable
enum class EmptyCompanion {

    @SerialName("TEST")
    TEST;

    companion object
}

it fails too with Error while analyzing EmptyCompanion$Companion.class with JaCoCo 0.8.14.202510071157/0981128 and the same Cannot invoke "org.objectweb.asm.tree.AbstractInsnNode.getPrevious()" because "this.cursor" is null.

@Godin
Copy link
Member Author

Godin commented Oct 8, 2025

@ajeihala thanks!

Perhaps the enum class makes the difference.

yes - it is
fix for #1969 will handle both,
so we will communicate updates in it but not here - you can subscribe to it

This was referenced Oct 8, 2025
Merged
Closed
@Godin Godin mentioned this pull request Oct 10, 2025
Merged
@marcin-arciszewski-gds marcin-arciszewski-gds mentioned this pull request Oct 17, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leveretka leveretka leveretka approved these changes

Assignees

@Godin Godin

Labels

component: core.filters language: Kotlin

Projects

Filtering
Status: Done

Milestone

0.8.14

Development

Successfully merging this pull request may close these issues.

Code coverage incorrect for Kotlin data classes with kotlinx.serialization in JaCoCo 0.8.13

3 participants

@Godin @ajeihala @leveretka