Inconsistent fewerBraces indentation

[soronpo]

Configuration (required)

Please paste the contents of your .scalafmt.conf file here:

version = 3.7.2
runner.dialect = scala3

Command-line parameters (required)

When I run scalafmt via CLI like this: scalafmt a.scala

Steps

Given code like this:

class Top extends EDDesign:
  process(clk, rst):
    val c = Bits(16) const all(0)
    if (rst)
      y :== c
    else if (clk.rising)
      y :== x
  val myblock = process(all):
    val my_var = Bits(16) <> VAR
    my_var := x
    y :== my_var
  process.forever:
    z :== x
    y :== z
end Top

Problem

Scalafmt formats code like this:

class Top extends EDDesign:
  process(clk, rst):
      val c = Bits(16) const all(0)
      if (rst)
        y :== c
      else if (clk.rising)
        y :== x
  val myblock = process(all):
      val my_var = Bits(16) <> VAR
      my_var := x
      y :== my_var
  process.forever:
    z :== x
    y :== z
end Top

Expectation

I would like the formatted output to look like this (same as the input, in this case):

class Top extends EDDesign:
  process(clk, rst):
    val c = Bits(16) const all(0)
    if (rst)
      y :== c
    else if (clk.rising)
      y :== x
  val myblock = process(all):
    val my_var = Bits(16) <> VAR
    my_var := x
    y :== my_var
  process.forever:
    z :== x
    y :== z
end Top

Workaround

None

Notes

I think that it's worth considering adding an indent.fewerBracesSite option that will control the indentation for fewer braces.

[kitbellew]

@soronpo since you saw the other thread, you should understand the reason, and the consistency behind it.

since we must indent chained methods (.select), including after foo: and after foo.bar:, what do you propose to do? apply the extra indent only if there's a chained select, and shift everything right when it is added? that would make a lot of people unhappy...

[kitbellew]

Scalafmt formats code like this:

...
  val myblock = process(all):
      val my_var = Bits(16) <> VAR
      my_var := x
      y :== my_var
  process.forever:
    z :== x
    y :== z
end Top

just noticed this. is this really what you see? different indentation for foo: and for foo.bar:? that was what we tried to fix in the most recent release (in the linked thread).

[soronpo]

is this really what you see

Yes, indeed this is what I see. So first we have this inconsistency, and I actually do not understand why there is a need for the double indent at all.

In the original issue, there is this example:

def test(): String =
  Block:
    2 + 2
  .toString

After formatting, this code should NOT have changed at all.

[kitbellew]

is this really what you see

Yes, indeed this is what I see. So first we have this inconsistency, and I actually do not understand why there is a need for the double indent at all.

In the original issue, there is this example:

def test(): String =
  Block:
    2 + 2
  .toString

After formatting, this code should NOT have changed at all.

unfortunately, this example makes .toString at the same indentation level as Block:, and that's incorrect: chained select is always indented relative to the beginning of the expression.

[soronpo]

unfortunately, this example makes .toString at the same indentation level as Block:, and that's incorrect: chained select is always indented relative to the beginning of the expression.

You say that from a formatter perspective or compiler perspective? From compiler perspective what currently scalafmt generates is simply incorrect and fails to compile.
See:
https://scastie.scala-lang.org/b2DsQm0OR8ejbnCQxhfxUg

From formatter perspective, I just don't agree with "always indented relative to the beginning of the expression", and I don't think it's a valid assumption under a fewerBraces clause.

[kitbellew]

unfortunately, this example makes .toString at the same indentation level as Block:, and that's incorrect: chained select is always indented relative to the beginning of the expression.

You say that from a formatter perspective or compiler perspective? From compiler perspective what currently scalafmt generates is simply incorrect and fails to compile. See: https://scastie.scala-lang.org/b2DsQm0OR8ejbnCQxhfxUg

don't you think that means there's a bug in the compiler or the documentation? the rule says (https://docs.scala-lang.org/scala3/reference/other-new-features/indentation.html) that it inserts outdent if the indent is strictly less than the previous indented region.

.toString is strictly less than 2+2 so there's outdent there; but it is not strictly less than Block, so there's no outdent -- and since it's not an optional-braces situation (relative to Block), why does it need to match any indentation levels?

@tgodzik how do we clear that up with the core language experts?

From formatter perspective, I just don't agree with "always indented relative to the beginning of the expression", and I don't think it's a valid assumption under a fewerBraces clause.

Which is why the formatter has always been called "opinionated".

[kitbellew]

You say that from a formatter perspective or compiler perspective? From compiler perspective what currently scalafmt generates is simply incorrect and fails to compile. See: https://scastie.scala-lang.org/b2DsQm0OR8ejbnCQxhfxUg

Shouldn't this also fail (your example, but with braces around the def body):

https://scastie.scala-lang.org/1EXFwyXdQX6zoqnEzkpJwQ

[soronpo]

Shouldn't this also fail (your example, but with braces around the def body):

https://scastie.scala-lang.org/1EXFwyXdQX6zoqnEzkpJwQ

That's a good question. I think we need Odersky to participate in this discussion. I'll try to e-mail him.

[odersky]

It turns out this one passes because the enclosing region is enclosed in braces so it does not count as an indentation region.
If we leave out the braces we get an error.

On the other hand, code like this looks reasonable, and it's a shame to reject it:

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

See scala/scala3#17056 for a possible fix.