Managed Trailing Commas & Enums

[rileymichael]

Hello! We're looking at enabling managed trailing commas, but there's some interesting behavior w/enums which have member declarations. Given an enum like:

enum class ProtocolState {
  WAITING {
    override fun signal() = TALKING
  },
  TALKING {
    override fun signal() = WAITING
  },
  ;

  abstract fun signal(): ProtocolState
}

ktfmt with manageTrailingCommas = false will ignore the trailing comma / semi-placement, which allows for adding new entries where the diff is purely the added lines. This is also the case for ktlint (and in fact, if you omit the trailing comma before the semi, it will insert it). However, ktfmt with manageTrailingCommas = true will remove the trailing comma in favor of just the terminating semicolon, which results in diff churn when adding new entries.

Is there a rule / reasoning for this behavior, or could this be considered for change? If so I don't mind contributing the patch.

[hick209]

@nreid260, any thoughts on this?

[nreid260]

The most important thing is that there's exactly one right answer for any enum.

A corollary of that requirement is that the semicolon position also needs to be specified. It seems like you're asking for it to always be on a separate line, which is different from our current format, but not necessarily bad. I would prefer that there's no semi unless there are non-entry members, since that's a very common case, and also because ktfmt aggressively removes semis today.

---

Regarding implementation, I'll say that the enum behaviour took about 50% of the total time of implementing comma management. The PSI is just plain stupid for enum-entry lists, and you're fighting with that trying to coordinate the comma + semi removal.