Use-site annotation to require type is serializable

[rnett]

I could swear there was an issue for this somewhere, but I couldn't find it. May be a duplicate.

What is your use-case and why do you need this feature?

The serialization plugin has some very nice type checking to ensure that property types are all serializable when a class is annotationed as serializable. For example:

@Serializable
data class MySerializableClass(val a: Int, val b: MyNonSerializableClass)

would highlight MyNonSerializableClass as an error.

I would like the ability to extend this checking to user code (that is not just serializable class definitions). Why? Because often times you have a method that has an argument that you know you will be looking up a serializer for later. It would be nice to be able to have a compile-time error when passed a known non-serializable type rather than a runtime error.

For example:

(@RequireSerializable is what I'm proposing)

interface FileRepository<T> {

  fun <T> save(value: T)
  fun <T> load()
}

class DefaultFileRepository<T>(private val serializer: KSerializer<T>): FileRepository<T> { ... }

interface FileRepositoryFactory {
  fun create(type: KType): FileRepository
}

inline fun <refied @RequireSerializable T> FileRepositoryFactory.create() = create(typeOf<T>())

class DefaultFileRepositoryFactory @Inject constructor(private val module: SerializersModule): FileRepositoryFactory {
  fun create(type) = DefaultFileRepository(module.serializer(type))
}

// currently a runtime error
factory.create<MyNonSerializableClass>()

This comes up when you are using a DI framework that manages your serializers module and/or serialization format. In that case, you don't want to have to inject the module and get a serializer at the high level, but to pass the type down to the level actually doing the serialization. This is also important if you are supporting multiple serialization frameworks.

Another use-case is where you have an open interface, and want to ensure you are passed only serializable sub-classes:

interface Window {
}

@Serializable
data object MyBaseWindow : Window

@Serializable
data class GoodWindow(val a: Int): Window

data class BadWindow(val b: Int): Window

class WindowManager {
  private val windows = mutableListOf<Window>()

  private val json = Json

  fun open(window: @RequireSerializable Window){
    windows += window
  }

  fun save() {
    saveFile.writeText(json.encodeToString<List<Window>>(windows))
  }

}

// works fine
manager.open(GoodWindow(2))
manager.save()

// currently a runtime error
manager.open(BadWindow(3))
manager.save()

In this case, rather than being done manually, the later lookup is done by polymorphic serialization.
Another potential solution to this particular use-case is a way to have an open interface that requires all concrete implementations to be serializable.

Describe the solution you'd like

I would like a @RequireSerializable annotation that can be applied to types. This applies the "should be serializable" type checking the plugin already uses for serializable class properties to the type, and whenever it is used. For example, in the second example, it would be fine applying it to Window since it is an interface. However calling open(BadWindow(3)) would error, since semantically BadWindow is not of type @RequireSerializable Window (since it is not serializable).

I think a fine alternative for the second use case would be to require it to be a generic type, e.g.

  fun <@RequireSerializable T : Window> open(window: T){
    windows += window
  }

[sandwwraith]

I could swear there was an issue for this somewhere, but I couldn't find it. May be a duplicate.

The only one I can remember is #2522. It was not about annotation but about reporting an error in every case.
As I explained there, the problem with the always-error approach is that inliner and IDE cannot report problems in case we have already compiled library. Your approach is better w.r.t this problem because IDE can always look at the annotation of type parameters regardless of whether we need to inline function call later.

However, I'm not sure how you expect this annotation to work with the dynamic natures of the modules. In your example,
you have:

inline fun <refied @RequireSerializable T> FileRepositoryFactory.create() = create(typeOf<T>())

class DefaultFileRepositoryFactory @Inject constructor(private val module: SerializersModule): FileRepositoryFactory {
  fun create(type) = DefaultFileRepository(module.serializer(type))
}

// currently a runtime error, expected to be a compiler diagnostic
factory.create<MyNonSerializableClass>()

But this is incorrect. I can have

val factory = DefaultFileRepositoryFactory(serializersModuleOf(MyNonSerializableClassSerializer))
factory.create<MyNonSerializableClass>() // returns MyNonSerializableClassSerializer

Because if default serializer is not found, we always look into the module at runtime.
Therefore, we likely can annotate only module-independent overloads (e.g. serializer<T>(), but not SerializersModule.serializer<T>()). Given that Json.encodeToString() and similar functions always use overload with module, I'm not sure that introducing such annotation would be a significant improvement.

[rnett]

But this is incorrect. I can have

val factory = DefaultFileRepositoryFactory(serializersModuleOf(MyNonSerializableClassSerializer))
factory.create<MyNonSerializableClass>() // returns MyNonSerializableClassSerializer

Because if default serializer is not found, we always look into the module at runtime.

If this was a class and property, e.g.

data class MaybeSerializable(val prop: MyNonSerializableClass)

IIUC the compiler would complain, and it would require MyNonSerializableClass to be annotated with @Contextual.

I think behavior would make sense here, too, so that:

val factory = DefaultFileRepositoryFactory(serializersModuleOf(MyNonSerializableClassSerializer))
factory.create<MyNonSerializableClass>() // returns MyNonSerializableClassSerializer

would have an error, but

val factory = DefaultFileRepositoryFactory(serializersModuleOf(MyNonSerializableClassSerializer))
factory.create<@Contextual MyNonSerializableClass>() // returns MyNonSerializableClassSerializer

would work.

It matches the behavior we're used to with classes, where @Contextual is an opt-out of the static compile time checking.

[sandwwraith]

I think behavior would make sense here, too, so that
factory.create()
would have an error, but
factory.create<@contextual MyNonSerializableClass>()
would work.

I find this confusing. The factory already returns contextual serializer without additional annotations, why should we report an error on a valid code that returns the expected result?
Annotations like that are not stored anywhere (e.g. you can't get @Ann from typeOf<@Ann String>()) and are not able to affect runtime execution in any way.

[rnett]

I find this confusing. The factory already returns contextual serializer without additional annotations, why should we report an error on a valid code that returns the expected result?

Because the point of using @RequireSerializable is to guarantee that a serializer will be found, not to necessarily cover all cases where one would be. It's saying "give me something I know how to serialize (at compile time), or explicitly opt-out and look it up at runtime (@Contextual)". It's useful for the same reasons @Contextual isn't applied by default to serializable classes' fields (i.e. to prevent passing something non serializable without realizing it).

Just like for serializable classes, there may be cases where a serializer would be found, but it still errors, because it can only statically check things. @Contextual is the escape hatch to say "trust me, I'll provide a serializer at runtime". If it makes it clearer, you could name it something like @RequireStaticSerializer, but IMO since it uses the same logic as serializable classes it's clearer not to.

Annotations like that are not stored anywhere (e.g. you can't get @ann from typeOf<@ann String>()) and are not able to affect runtime execution in any way.

I don't think you'd need to, it's just a compile time check. @Contextual opts out of that check with the aforementioned "trust me".

[Fleshgrinder]

I second the desire for such an annotation. In fact I'd prefer it if the existing kotlinx.serialization.Serializable is extended for type parameter usage to reduce the amount of symbols, and to aid inexperienced users by directly navigating to the annotation they need. I think that being explicit as @rnett requests with the @Contextual for the factory example is a good thing. Compile time errors are always infinitely better than runtime errors.

However, I also understand that it can be considered a breaking change, because code that previously compiled suddenly doesn't anymore due to a missing annotation. It could thus be initially introduced as a warning and made an error in a future major release.