Klib .api files should sort members above types

[JakeWharton]

Currently it seems like fake overrides and constructors sort above nested types which sort above declared members.

Today:

final class app.cash.redwood.protocol/Event { // app.cash.redwood.protocol/Event|null[0]
    constructor <init>(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/EventTag, kotlin.collections/List<kotlinx.serialization.json/JsonElement> =...) // app.cash.redwood.protocol/Event.<init>|<init>(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.EventTag;kotlin.collections.List<kotlinx.serialization.json.JsonElement>){}[0]
    final fun equals(kotlin/Any?): kotlin/Boolean // app.cash.redwood.protocol/Event.equals|equals(kotlin.Any?){}[0]
    final fun hashCode(): kotlin/Int // app.cash.redwood.protocol/Event.hashCode|hashCode(){}[0]
    final fun toString(): kotlin/String // app.cash.redwood.protocol/Event.toString|toString(){}[0]
    final object $serializer : kotlinx.serialization.internal/GeneratedSerializer<app.cash.redwood.protocol/Event> { // app.cash.redwood.protocol/Event.$serializer|null[0]
        final fun childSerializers(): kotlin/Array<kotlinx.serialization/KSerializer<*>> // app.cash.redwood.protocol/Event.$serializer.childSerializers|childSerializers(){}[0]
        final fun deserialize(kotlinx.serialization.encoding/Decoder): app.cash.redwood.protocol/Event // app.cash.redwood.protocol/Event.$serializer.deserialize|deserialize(kotlinx.serialization.encoding.Decoder){}[0]
        final fun serialize(kotlinx.serialization.encoding/Encoder, app.cash.redwood.protocol/Event) // app.cash.redwood.protocol/Event.$serializer.serialize|serialize(kotlinx.serialization.encoding.Encoder;app.cash.redwood.protocol.Event){}[0]
        final val descriptor // app.cash.redwood.protocol/Event.$serializer.descriptor|{}descriptor[0]
            final fun <get-descriptor>(): kotlinx.serialization.descriptors/SerialDescriptor // app.cash.redwood.protocol/Event.$serializer.descriptor.<get-descriptor>|<get-descriptor>(){}[0]
    }
    final object Companion { // app.cash.redwood.protocol/Event.Companion|null[0]
        final fun serializer(): kotlinx.serialization/KSerializer<app.cash.redwood.protocol/Event> // app.cash.redwood.protocol/Event.Companion.serializer|serializer(){}[0]
        final val $childSerializers // app.cash.redwood.protocol/Event.Companion.$childSerializers|{}$childSerializers[0]
    }
    final val args // app.cash.redwood.protocol/Event.args|{}args[0]
        final fun <get-args>(): kotlin.collections/List<kotlinx.serialization.json/JsonElement> // app.cash.redwood.protocol/Event.args.<get-args>|<get-args>(){}[0]
    final val id // app.cash.redwood.protocol/Event.id|{}id[0]
        final fun <get-id>(): app.cash.redwood.protocol/Id // app.cash.redwood.protocol/Event.id.<get-id>|<get-id>(){}[0]
    final val tag // app.cash.redwood.protocol/Event.tag|{}tag[0]
        final fun <get-tag>(): app.cash.redwood.protocol/EventTag // app.cash.redwood.protocol/Event.tag.<get-tag>|<get-tag>(){}[0]
}

Desired:

final class app.cash.redwood.protocol/Event { // app.cash.redwood.protocol/Event|null[0]
    constructor <init>(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/EventTag, kotlin.collections/List<kotlinx.serialization.json/JsonElement> =...) // app.cash.redwood.protocol/Event.<init>|<init>(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.EventTag;kotlin.collections.List<kotlinx.serialization.json.JsonElement>){}[0]
    final fun equals(kotlin/Any?): kotlin/Boolean // app.cash.redwood.protocol/Event.equals|equals(kotlin.Any?){}[0]
    final fun hashCode(): kotlin/Int // app.cash.redwood.protocol/Event.hashCode|hashCode(){}[0]
    final fun toString(): kotlin/String // app.cash.redwood.protocol/Event.toString|toString(){}[0]
    final val args // app.cash.redwood.protocol/Event.args|{}args[0]
        final fun <get-args>(): kotlin.collections/List<kotlinx.serialization.json/JsonElement> // app.cash.redwood.protocol/Event.args.<get-args>|<get-args>(){}[0]
    final val id // app.cash.redwood.protocol/Event.id|{}id[0]
        final fun <get-id>(): app.cash.redwood.protocol/Id // app.cash.redwood.protocol/Event.id.<get-id>|<get-id>(){}[0]
    final val tag // app.cash.redwood.protocol/Event.tag|{}tag[0]
        final fun <get-tag>(): app.cash.redwood.protocol/EventTag // app.cash.redwood.protocol/Event.tag.<get-tag>|<get-tag>(){}[0]
    final object $serializer : kotlinx.serialization.internal/GeneratedSerializer<app.cash.redwood.protocol/Event> { // app.cash.redwood.protocol/Event.$serializer|null[0]
        final fun childSerializers(): kotlin/Array<kotlinx.serialization/KSerializer<*>> // app.cash.redwood.protocol/Event.$serializer.childSerializers|childSerializers(){}[0]
        final fun deserialize(kotlinx.serialization.encoding/Decoder): app.cash.redwood.protocol/Event // app.cash.redwood.protocol/Event.$serializer.deserialize|deserialize(kotlinx.serialization.encoding.Decoder){}[0]
        final fun serialize(kotlinx.serialization.encoding/Encoder, app.cash.redwood.protocol/Event) // app.cash.redwood.protocol/Event.$serializer.serialize|serialize(kotlinx.serialization.encoding.Encoder;app.cash.redwood.protocol.Event){}[0]
        final val descriptor // app.cash.redwood.protocol/Event.$serializer.descriptor|{}descriptor[0]
            final fun <get-descriptor>(): kotlinx.serialization.descriptors/SerialDescriptor // app.cash.redwood.protocol/Event.$serializer.descriptor.<get-descriptor>|<get-descriptor>(){}[0]
    }
    final object Companion { // app.cash.redwood.protocol/Event.Companion|null[0]
        final fun serializer(): kotlinx.serialization/KSerializer<app.cash.redwood.protocol/Event> // app.cash.redwood.protocol/Event.Companion.serializer|serializer(){}[0]
        final val $childSerializers // app.cash.redwood.protocol/Event.Companion.$childSerializers|{}$childSerializers[0]
    }
}

[fzhinkin]

It's something I deliberately decided not to do, but why not.

[fzhinkin]

For declarations having different targets, we'll continue ranking more common declarations (i.e. having a broader targets set) higher.
For declarations having the same set of declarations we can:

• on the top level:
  • print interfaces, classes and objects first
  • then properties
  • then functions
• in the nested scope (i.e. within a class):
  • print properties first
  • then constructors
  • then all other functions
  • then interfaces, classes and objects
  • then enum entries

Not sure if we need to rank declarations within the same "group" using declaration type/modality/some other attribute, for instance, by placing annotations before interfaces, placing vals before vars, or ranking final classifiers above open ones.

[JakeWharton]

For declarations having different targets, we'll continue ranking more common declarations (i.e. having a broader targets set) higher.

Does this mean that adding support for more targets to an existing declaration moves the location of a function in the file? From the perspective of reviewing a diff (since the tool currently does not do compatibility validation), having the signature remain in a fixed location seems more important. This way you can easily ensure the signature has not changed, only the targets. If the signature jumps tens or hundreds of lines in the file then I have to be very careful to ensure both match.

Of course such a thing is completely a non-issue if the tool switches to perform compatibility validation itself.

For declarations having the same set of declarations we can:

* on the top level:
  
  * print interfaces, classes and objects first
  * then properties
  * then functions

* in the nested scope (i.e. within a class):
  
  * print properties first
  * then constructors
  * then all other functions
  * then interfaces, classes and objects
  * then enum entries

Any fixed order is fine, of course.

If you want me to bikeshed this, though, I would choose

• Enum entries
• Constructors
• Properties
• Functions
• Nested types

Constructors are most important everywhere except enums where entries are the most important. Then it's properties and functions. Then nested types.

Not sure if we need to rank declarations within the same "group" using declaration type/modality/some other attribute, for instance, by placing annotations before interfaces, placing vals before vars, or ranking final classifiers above open ones.

I wouldn't choose any sorting key that could be changed while still retaining compatibility. For example, val to var is a compatible migration, but I wouldn't want such a change to move the signature in the file causing a noisy diff. Similarly, final to open is a compatible change and I wouldn't want it to move as a result.

Changing class to interface is not compatible, so having them be separate isn't a huge deal. They would only jump for incompatible changes. Now unfortunately class to object and object to class can both be done compatibly, so perhaps they should be considered the same.

[fzhinkin]

Does this mean that adding support for more targets to an existing declaration moves the location of a function in the file?

It does, yes.

To avoid that issue, we should not be consider targets when sorting declarations. That would result in declarations belonging to different targets being interleaved. Then, it might be harder to grasp actual targets set for top-level declarations available for all targets (as these declarations without // Targets may be surrounded with declarations having their // Targets).

I would expect that for a regular library declarations should not change their targets too often to annoy reviewers.

On the other hand, grouping by targets gives a dump more structure.

Constructors are most important everywhere except enums where entries are the most important. Then its properties and functions. Then nested types.

Makes perfect sense, thanks.

For example, val to var is a compatible migration

As a side note, it is currently considered incompatible for klibs. :'(
Motivating issue: https://youtrack.jetbrains.com/issue/KT-44605