Skip to content
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

@MustBeDocumented for @Serializable #2056

Closed
lukellmann opened this issue Oct 11, 2022 · 3 comments
Closed

@MustBeDocumented for @Serializable #2056

lukellmann opened this issue Oct 11, 2022 · 3 comments
Assignees
Labels

Comments

@lukellmann
Copy link
Contributor

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

#2048 promotes typealiases for use with @Serializable.

We already use it in our library and it works as expected:

/** A [Duration] that is [serializable][Serializable] with [DurationInSecondsSerializer]. */
public typealias DurationInSeconds = @Serializable(with = DurationInSecondsSerializer::class) Duration

However when using Dokka, the output looks like this:
Dokka output without annotation

This might be confusing for users of our library as it doesn't seem to make any sense to use this typealias. The important information that a custom serializer is added with this annotation is missing.

Describe the solution you'd like

To support this, annotation class Serializable would simply have to be annotated with the MustBeDocumented meta-annotation:

@MustBeDocumented
@Target(AnnotationTarget.PROPERTY, AnnotationTarget.CLASS, AnnotationTarget.TYPE)
public annotation class Serializable(val with: KClass<out KSerializer<*>> = KSerializer::class)

The Dokka output then looks like we would like to:
Dokka output with annotation

Other effects of this solution that should be considered

@Serializable can be used in other places too:

  • on classes -> I would argue that it is also desirable that Dokka output shows the information that some class is serializable.
    @Serializable // would be documented
    data class User(val name: String)
  • on properties -> I'm not sure if it is desirable that Dokka output shows the information which exact serializers are used.
    @Serializable
    data class User(
        @Serializable(with = MyCustomStringSerializer::class) // would be documented
        val name: String
    )
  • on types -> Same as for properties, I'm not sure if it should be shown.
    @Serializable
    data class User(    // would be documented
        val names: List<@Serializable(with = MyCustomStringSerializer::class) String>
    )

Other annotations that might be worth documenting

Maybe @MustBeDocumented could also be added to some other annotations, I'm thinking of @SerialName, @Required and @Transient.

@qwwdfsad
Copy link
Member

qwwdfsad commented Oct 12, 2022

Thanks for the detailed write up!

sandwwraith added a commit that referenced this issue Oct 12, 2022
Only annotations that affect code generation/add declarations on class-level were selected

Other annotations that affect mainly properties (SerialName,Required,Transient) are left for further consideration.

File-level annotations like `@UseSerializers` likely shouldn't be documented.

See #2056
sandwwraith added a commit that referenced this issue Oct 13, 2022
Only annotations that affect code generation/add declarations on class-level were selected

Other annotations that affect mainly properties (SerialName,Required,Transient) are left for further consideration.

File-level annotations like `@UseSerializers` likely shouldn't be documented.

See #2056
@sandwwraith sandwwraith self-assigned this Oct 13, 2022
@sandwwraith
Copy link
Member

I'll close this for now because it is fixed in #2059.
However, you may create a new issue for @SerialName/@JsonClassDiscriminator if you have reasons for them to be documented too

@lukellmann
Copy link
Contributor Author

@SerialName, @Required, @Transient and @EncodeDefault are now also annotated with @MustBeDocumented. Issue: #2386, PR: #2407

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

4 participants