Skip to content

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

Merged
Godin merged 7 commits intojacoco:masterfrom
Godin:kotlin_serialization
Oct 7, 2025
Merged

Add filter for bytecode generated by Kotlin serialization compiler plugin#1885
Godin merged 7 commits intojacoco:masterfrom
Godin:kotlin_serialization

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