Skip to content

Add a lint rule for encodeToString/decodeFromString to detect if the value has Serializable annotation #3010

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

Open
TimoPtr opened this issue May 27, 2025 · 8 comments
Labels

Comments

@TimoPtr
Copy link

TimoPtr commented May 27, 2025

What is your use-case and why do you need this feature?
The Home Assistant project is migrating to KotlinX serialization for JSON mostly. And while migrating we missed some spot where some classes where not annotated with @Serializable.
Describe the solution you'd like
We've made a very basic lint rule to detect if the field value of the method encodeToString or decodeFromString for the Json module. home-assistant/android#5368 it's not complete but it fits our usage. It would be nice to have something better and more robust.

@pdvrieze
Copy link
Contributor

@TimoPtr kotlinx.serialization is designed as a static serializer that does not use reflection (although it can do it to look up the root serializer). As such it should fail to compile (with nowadays a nice error) if elements are not serializable. The main thing you want to do is make sure the classes you call encodeToString on are serializable (use the 2-arg version and explicitly pass in the serializer: encodeToString(MyClass.serializer(), myClassInstance) will ensure that all of this exists.

@TimoPtr
Copy link
Author

TimoPtr commented May 27, 2025

I'm not just to get your point. Since it's not using reflection (and that's why we choose to use it btw) we thought that it already had everything in place to make sure that at build time you cannot write something like I've wrote in the test in the linked PR.

Something like that shouldn't compile

class Home

fun main() { 
Json.encodeToString(Home())
}

Since it misses the annotation. But today the build succeed and it fails only at runtime which is bad. That's why we've introduce the lint check so it fails earlier than when we release our app, it fails on CI.

@pdvrieze
Copy link
Contributor

@TimoPtr That code should fail (presuming that you have applied the serialization plugin - if you haven't it is valid code and the regular compiler knows nothing about (missing) serializers - the lookup is done by a compiler plugin intrinsic that replaces a best effort dynamic lookup.

@TimoPtr
Copy link
Author

TimoPtr commented May 28, 2025

IMO this code should fail yes. Yes I did add the serialization plugin you can see it in this PR home-assistant/android#5279 (check the first commits for the Gradle setup). But this code is not failing at build time it indeed fails at runtime that why I've made this lint rules otherwise you only find out too late.

Image
You can see in this small snippet that also the IDE is not warning you of anything without the lint rule. It should be possible to have the feedback from the IDE that the annotation is missing.

@TimoPtr
Copy link
Author

TimoPtr commented May 28, 2025

I've extend the lint rule to decodeFromString that also has the same issue. home-assistant/android#5375

@TimoPtr TimoPtr changed the title Add a lint rule for encodeToString to detect if the value has Serializable annotation Add a lint rule for encodeToString/decodeFromString to detect if the value has Serializable annotation May 28, 2025
@pdvrieze
Copy link
Contributor

I had a check in AS and intellij with your project and an independent context. You are right, serializer<MyType> will not signal if the type is not serializable (this function is the intrinsic that will substitute the serializer at compile time if it exists). Instead it defers to runtime lookup. It would be good to have the option in the compiler plugin to warn/fail if the type is known, but has no serializer.

@TimoPtr
Copy link
Author

TimoPtr commented May 29, 2025

I might take a look to see if I can help on the compiler to check this. But in the meantime the lint rules is the way to go for us.

@pdvrieze
Copy link
Contributor

@TimoPtr You want to search for intrinsic (for the serializer function) in the serialization plugin folder of the kotlin project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants