Generalized support for fluent chains

[nreid260]

This is a master issue describing an architectural change that would fix many individual bugs

Thesis

• Humans recognize expressions known as call chains or fluent chains
  • A chain may not align with an evaluation-oriented AST
  • A chain is rooted on an arbitrary expression
  • A chain has one or more links following the root, each containing
    • A dereference operator {., ?., ::, ?::}
    • A member name
    • 0 or more left-associative operator-like terms {call, array access, !!, ++, etc.}
• Humans want chains to fit on one line
  • But if it won't fit, they want a break at each link
  • And if a chain ends in a call-link, they perceive that call to be block-like
    • Unless the member name would exceed the line length limit
• Humans recognize typenames (including static members) as distinct from the following links of a chain
  • Typenames (and static members) should break separately from other links

Points of clarification

1. All qualified expression are fluent chains but not all fluent chains are qualified expressions. A qualified expression has no operators (e.g. a.b.c) (well maybe it has single calls because the AST is strange (e.g. a.b().c) but I consider that an artifact). A fluent chain may contain operators (e.g. a.b[0]!!.c(1)(2))
2. Chains often don't look like a chains in the AST. My example looks something like:

CALL
  QUALIFIED `.`
    POSTFIX `!!`
      ARRAY_ACESS
        QUALIFIED `.`
          NAME `a`
          NAME `b`
        NUMBER `0`
    CALL // Making this a child of QUALIFIED is super strange IMO
      NAME `c`
      NUMBER `1`
  NUMER `2`

3. Blocklike means a pair of paren-like symbols where the closing paren aligns with an earlier/outer piece of syntax. This is the intuitive behaviour of the } in an if statement. Imagining the { ... } as a syntax level, the { is "cheating" to stay on the same line as the if condition.

// This is blocklike
all.these(names).fitOn[0].oneLine(
  but = theseDont,
)
// Notice the alignment of this paren

// Here's what would happen if it were expression-like
all
  .these(names)
  .fitOn[0]
  .oneLine(
    but = theseDont,
  )
// We need a structure that morphs between the two states depending on the lengths of the links.

See also

• https://github.com/facebookincubator/ktfmt/pull/294 Proposal PR
• https://github.com/facebookincubator/ktfmt/issues/297 Generalization of blocklike behaviour for calls, which is important for this proposal

[davidtorosyan]

This was at least partially addressed by f19bd22a97f5922168c1c32399c4efaac9ebdc7a (for ::) and c7fffbeac65e15a897d6fae09a4ce82bf7cd01be (for !! and other cases). I'm unsure how many issues remain though, so keeping this open.

[hick209]

@nreid260 are we there yet or are there still issues to be resolved?

What do you think about we create a new issue to track it, so it's clearer what are the current issues (if any remain from this issue)?

[hick209]

I'll close this now (as it's old) assuming you agree with me, but feel free to reopen in case you think we should keep this issue around to track things, but please do write something here to clarify what are the remaining issues you see and expect to be worked on

[nreid260]

I'm mostly satisfied with the current output, although I find the implementation hard to follow. I had proposed an intermediate representations for fluent chains (a separate class) but that never went anywhere.

---

One part that we might need to reconsider is block-like trailing lambdas. e.g.

foo.bar(a, b).bip<T>.callback {
  ///
}

I pushed for this in the original design because it looked better for "short" cases, but we've gotten a few bugs recently where "long" cases cause weird wrapping. e.g.

foo.bar(a, b).buuuuuuuuuuuuuuuuuuuuuuuuuuuuup<T>.callback<
  Foo,
  Bar
> {
  ///
}

A constraint on chain length or chain composition might be needed.