Do not sort T::Struct fields (prop and const attributes)
#140
Conversation
Sorbet defines the initializer depending on the declaration order of the fields. When we sort the fields, the initializer ends up not matching the declaration expected by Sorbet. This is especially problematic if the initializer of the struct subclass is overriden, since the static type checker will raise an error if the order of the parameters in the initializer does not match the order that the props were declared, which is file order. An example can be seen here: https://sorbet.run/#%23%20typed%3A%20true%0A%0Aclass%20Foo%20%3C%20T%3A%3AStruct%0A%20%20const%20%3Abar%2C%20Integer%0A%20%20const%20%3Afoo%2C%20String%0A%0A%20%20sig%20%7B%20params%28foo%3A%20String%2C%20bar%3A%20Integer%29.void%20%7D%0A%20%20def%20initialize%28foo%3A%2C%20bar%3A%29%0A%20%20end%0Aend%0A%0A
For stability of output. If we don't sort to an expected order, then any change in the order of RBI node additions would cause the output to be different in spurious ways. For example, a class that changes the order in which its methods are declared, but nothing else, might end up causing an ordering change in its RBI file. Or, if two DSL compilers run in a different order, the resulting RBI file might look slightly different. Having said this, I think we should make the nodes declare if they are sortable or not (or even better, what their sort key should be), so that we don't do this ad-hoc case/when over node types. But that is an improvement for another day. |
|
@paracycle Makes sense! Thanks for the explanation. |
Related to: https://github.com/Shopify/hedwig/pull/521
Sorbet defines the initializer depending on the declaration order of the fields. When we sort the fields, the initializer ends up not matching the declaration expected by Sorbet.
This is especially problematic if the initializer of the struct subclass is overriden, since the static type checker will raise an error if the order of the parameters in the initializer does not match the order that the props were declared, which is file order. An example can be seen
here. Since we generate the initializer params in document order, we should also generate props in document order as well.
Note: As I was removing
TStructFieldfrom thenode_namemethod case statement, I realized that we never really explicitly show which nodes should not be sorted by name, so I added an explicitnilreturning case for those.