Fewer braces and lambda: incorrect indentation region detection

[kitbellew]

Compiler version

• 3.3.3
• 3.3.4 LTS
• 3.5.2 Next

Minimized code

  def fn2(arg: String, arg2: String)(f: String => Unit): Unit =
    f(arg)

  // doesn't compile
  fn2(
      arg = "blue sleeps faster than tuesday",
      arg2 = "the quick brown fox jumped over the lazy dog"): env =>
      val x = env
      println(x)

  // does compile
  fn2(
      arg = "blue sleeps faster than tuesday",
      arg2 = "the quick brown fox jumped over the lazy dog"):
    env =>
      val x = env
      println(x)

  // does compile
  fn2(
      arg = "blue sleeps faster than tuesday",
      arg2 = "the quick brown fox jumped over the lazy dog"
  ): env =>
      val x = env
      println(x)

Output

scastie marks two errors:

• fn2 line with not a legal formal parameter for a function literal
• val x = env line with Not found: env

Expectation

According to the documentation, the first argument clause (with arg and arg2) does not start an indentation region, hence as long as the lambda body is indented relative to fn2 invocation, it should work.

• https://dotty.epfl.ch/docs/reference/other-new-features/indentation.html#optional-braces-3
• https://dotty.epfl.ch/docs/reference/other-new-features/indentation.html#indentation-and-braces-1

P.S. The problem was first reported in scalameta/scalafmt#4569.

[som-snytt]

The docs says

https://dotty.epfl.ch/docs/reference/other-new-features/indentation.html#indentation-and-braces-1

The assumed indentation width of a multiline region inside brackets or parentheses is:

• if the opening bracket or parenthesis is at the end of a line, the indentation width of token following it,

So indenting (relative to arg) works:

  fn2(
      arg = "blue sleeps faster than tuesday",
      arg2 = "the quick brown fox jumped over the lazy dog"): env =>
       val x = env // indented
       println(x)

On the linked ticket, it's assumed that this behavior is a bug because the following works:

  def fn3(arg: String, arg2: String)(f: => Unit): Unit = f

  fn3(
      arg = "blue sleeps faster than tuesday",
      arg2 = "the quick brown fox jumped over the lazy dog"):
      val x = "Hello"
      println(x)

The difference is that fn2 uses a lookahead scanner to detect that arrow is followed by <indent>; I think that correctly fails and that fn3 incorrectly succeeds, but I'll know better after a few more printlns.

[som-snytt]

Assigning myself just to let you know I'm looking at it right now.

It's actually simpler:

     *   2. Insert INDENT if
     *
     *      - indentation is significant, and
     *      - the last token can start an indentation region.
     *      - the indentation of the current token is strictly greater than the previous
     *        indentation width, or the two widths are the same and the current token is
     *        one of `:` or `match`.

I think the reference omits the special case for colon.

[kitbellew]

So indenting (relative to arg) works:

  fn2(
      arg = "blue sleeps faster than tuesday",
      arg2 = "the quick brown fox jumped over the lazy dog"): env =>
       val x = env // indented
       println(x)

But the (arg region ended right after lazy dog"), no? So : env => starts its own, relative to fn2.

And if (arg region ends somewhere else, then where, and why?

[som-snytt]

@kitbellew I'll answer more fully when my understanding has improved. It may just work under the "same width" exclusion, like a block following colon at EOL.

The philosophical words I was about to write: regions are a scaffolding for when to infer indents and outdents.

[som-snytt]

I added a WIP PR still in-progress... Edit: improved to make it less wrong.

@kitbellew You may wish to review the two test files, which reflect your intuition that the lambda body only needs to be indented from the "enclosing" indentation width.

The bug was that a fresh lookahead scanner recomputes its indentation width for whatever offset it starts at. That broke the condition that the arrow at EOL is followed by an indent. (Otherwise, function literals are agnostic about body syntax.)

My previous guess about allowing indent at equal widths was a red herring, and the code comment I quoted was wrong.

The alignment rule for match allows:

  println:
    def g = x match
    case 42 => // alignment ok
      "yay"
    g

which looks stranger (because it doesn't align with the selector expression) than the aligned syntax I've come to prefer:

x match
case 42 =>
case -1 =>

That is easy to scan because the match stops at "not a case".

I was looking at:

def g2(xs: List[Int]) =
  xs.map: (x: Int) =>
  x+1

but that makes no sense and quickly runs into difficulty:

  xs.map: x =>
  x + 1
  .filter: x =>
  x > 0

which, now that I've dismissed it, also holds a certain eccentric appeal.

[kitbellew]

@kitbellew You may wish to review the two test files, which reflect your intuition that the lambda body only needs to be indented from the "enclosing" indentation width.

Thank you for that, I wasn't expecting movement on this so soon. Checked the tests, added a comment.

but that makes no sense and quickly runs into difficulty:

  xs.map: x =>
  x + 1
  .filter: x =>
  x > 0

Related to last comment but separate from this issue, happy to submit another issue if you agree. When I was dealing with formatting rules for fewer-braces, ran into problems (we usually indent continuation select statements):

// doesn't compile but would be nice
// first body is indented "twice", or, rather, first outdent establishes an intermediate indentation level
xs.map: x =>
    x + 1
  .filter: x =>
    x > 0
xs.map:
    addOne
  .filter:
    isPositive

// does compile
xs
  .map: x =>
    x + 1
  .filter: x =>
    x > 0

// does compile but doesn't look good, at least, to some people
xs.map: x =>
    x + 1
.filter: x =>
    x > 0

the reason was that .filter was indented in between regions so

[som-snytt]

@kitbellew thanks, added those tests too, which just work. (Actually your example compiles before the fix. Maybe I'm missing what you mean.) (Maybe other compiler options are involved.)

The dot rule is that the dot must differ from the two regions by more than one. Maybe I'll add a neg test as well, although the PR only touches "whether to parse" and not actual parsing logic. There may be an existing test.

Added the 3.6.3 parse result as a test!

def `tested kit`(xs: List[Int]): Unit =
  {
    def addOne(i: Int): Int = i.+(1)
    def isPositive(i: Int): Boolean = i.>(0)
    xs.map[Int]((x: Int) => x.+(1)).filter((x: Int) => x.>(0))
    xs.map[Int]((i: Int) => addOne(i)).filter((i: Int) => isPositive(i))
    xs.map[Int]((x: Int) => x.+(1)).filter((x: Int) => x.>(0))
    {
      xs.map[Int]((x: Int) => x.+(1)).filter((x: Int) => x.>(0))
      ()
    }
  }

[som-snytt]

The dot rule will stop people from using l for lists.

def `test kit`(l: List[Int]): Unit =
  l.map: x =>
      x + 1
   .filter: x =>
      x > 0

[kitbellew]

@kitbellew thanks, added those tests too, which just work. (Actually your example compiles before the fix. Maybe I'm missing what you mean.) (Maybe other compiler options are involved.)

oh... i haven't checked this for a year, but let me try to find my old code which didn't work.