Skip to content

KotlinSerializableFilter should filter more methods #1971

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 4 commits into from
Oct 10, 2025
Merged

KotlinSerializableFilter should filter more methods #1971

Godin merged 4 commits into from
Oct 10, 2025

Conversation

@Godin
Copy link
Member

@Godin Godin commented Oct 10, 2025 •
edited
Loading

This was overlooked in #1885

See #1969 (comment)

For example in case of

@kotlinx.serialization.Serializable
enum class Example {
    V
}

or

@kotlinx.serialization.Serializable
sealed class Example {
        @kotlinx.serialization.Serializable
        data class A(val data: String): Example()
}
  • Example$Companion.get$cachedSerializer()
  • Example$Companion.serializer()
  • Example._init_$_anonymous_() - see JetBrains/kotlin@3f034e8

In case of

@kotlinx.serialization.Serializable
object Example
  • Example.get$cachedSerializer()
  • Example.serializer()

And in case of

@kotlinx.serialization.Serializable
data class Example(
    val data1: List<String>,
    val data2: List<String>,
)

@Godin Godin added this to the 0.8.14 milestone Oct 9, 2025
@Godin Godin self-assigned this Oct 9, 2025
@Godin Godin added this to Filtering Oct 9, 2025
@github-project-automation github-project-automation bot moved this to Awaiting triage in Filtering Oct 9, 2025
@Godin Godin moved this from Awaiting triage to To Do in Filtering Oct 9, 2025
@Godin Godin mentioned this pull request Oct 9, 2025
Closed
@Godin Godin moved this from To Do to In Progress in Filtering Oct 10, 2025
@Godin Godin mentioned this pull request Oct 10, 2025
Open
@Godin Godin requested a review from leveretka October 10, 2025 23:00
leveretka
leveretka approved these changes Oct 10, 2025
View reviewed changes
Copy link
Collaborator

@leveretka leveretka left a comment •
edited by Godin
Loading

Choose a reason for hiding this comment

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

Discussed this change offline.

The case with generic types can be handled later - #1972, other 3 cases needs to be covered to improve the behavior in case of serialization plugin.

@Godin Godin merged commit de76181 into jacoco:master Oct 10, 2025
36 checks passed
@Godin Godin deleted the KotlinSerializableFilter branch October 10, 2025 23:32
@github-project-automation github-project-automation bot moved this from In Progress to Done in Filtering Oct 10, 2025
@Godin
Copy link
Member Author

Godin commented Oct 11, 2025 •
edited
Loading

Since there are many other projects waiting JaCoCo release with upgraded ASM (#1965), we are postponing the only so far found unhandled remaining and not so easy to handle case of @kotlinx.serialization.Serializable #1971, and staged 0.8.14 to be released with this change as latest. CC @merks @iloveeclipse

@Dukoff92 @ajeihala Even if we are very confident in absence of NPEs and in this change in general, would it be possible to test latest snapshot to check our theory that with this change even without #1971 situation is already much better than before, and there are no other remaining cases than #1971 ?

@Dukoff92
Copy link

Dukoff92 commented Oct 11, 2025

@Godin

Thanks again for looking into the issue and providing a fix. I can confirm that everything looks good with the latest snapshot version. I’ve checked all sealed classes with the @Serializable annotation in our codebase, and I don’t see any generated methods in the report.

I also looked at the generated methods when data class parameters are of type List, and didn’t find any generated ones for those either.

We don’t have any enum classes using serialization, so I can’t speak to that, but I see that it’s been addressed in the PR above. Maybe @ajeihala can test it out and provide more clarity.

@Godin
Copy link
Member Author

Godin commented Oct 11, 2025

@Dukoff92 Thank you very much for your fast tests! ❤️

@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.

3 participants

@Godin @Dukoff92 @leveretka