Allow lines starting with . to fall outside previous indentation widths

[odersky]

If a line following some indented code starts with a '.' and its indentation width is different from the indentation widths of the two neighboring regions by more than a single space, the line is accepted even if it does not match a previous indentation width. This tweak of the indentation rules is introduced so that code like the following can be written.

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

[odersky]

See also scalameta/scalafmt#3489

[kitbellew]

out of curiosity, why more than a single space?

this is not something scalafmt would necessarily do (unless indent was set to 1, of course), but I can imagine someone doing this manually:

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

[odersky]

We have a design rule for indentation that adding or removing a single space should not usually change program behavior, as long as your basic indentation width is at least 2 spaces.

[kitbellew]

perhaps @tailrec?

[odersky]

Technically this is tail recursive. However we have the policy in the compiler that we will add @tailrec only for methods that could in practice recurse deeply if they would not be tail recursive. E.g. a method recursing over statement sequences, since a statement sequence can have lots of elements. Here, this is not really the case. There would be very few changes from spaces to tabs in leading whitespace, if any at all.

This policy is to avoid clutter and pointless busywork.

[kitbellew]

would it make sense to add a test with

xs.match
    case _ => Seq(0, 1)
  .filter: x =>
   // the rest as before

[kitbellew]

@tgodzik not sure if this reached your inbox :)

[tgodzik]

@tgodzik not sure if this reached your inbox :)

Ach sorry, I had a large backlog and was just getting back to it. Looks like scalameta already supports this, we are a bit more lenient than the dotty parser, but I think that's for the best as it allows us to fix simple mistakes.

[tgodzik]

I worry that this might be confusing for users, since the rules for this will be quite hard to find and a difference of one whitespace might cause hard to understand errors.

I would argue that indentation for .toString could be anything between the indentation of Block (including) and the indentation of 2 + 2 (excluding).

The alternative for this:

Block{
      2 + 2
     }.toString

works and dropping braces will break it.

Why does .toString need to match the indentation of anything before? It's more a continuation of the previous statement and not a standalone statement itself. We are already pretty lenient for a similar syntax with multiple parameter clauses.

[odersky]

The one-space rule is there to not open the flood-gates that small changes can have a silent change in meaning.

[tgodzik]

We have a design rule for indentation that adding or removing a single space should not usually change program behavior, as long as your basic indentation width is at least 2 spaces.

Doesn't the PR break the behaviour from the other side? With the current logic if I add a space then I could reach the threshold and the program stops compiling. Let's say I have:

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

if I add a space before .toString it breaks.

[kitbellew]

@odersky @tgodzik is there anything I could do to help resolve this proposal?

If the design rule allowing one space wiggle room is currently already applied to significant indentation, then I'd go with that.

However, if that same rule is not yet applied (otherwise why would the isClose method need to be implemented), then I would go with Tomasz's suggestion. Also, his challenge (that wiggle cuts both ways) seems valid.

[odersky]

On Thu, Mar 16, 2023 at 5:57 PM Tomasz Godzik ***@***.***> wrote: We have a design rule for indentation that adding or removing a single space should not usually change program behavior, as long as your basic indentation width is at least 2 spaces. Doesn't the PR break the behaviour from the other side? With the current logic if I add a space then I could reach the threshold and the program stops compiling.

Yes, but that's OK. We want to prevent silent change of meaning, issuing compile errors is expected. - Martin

…

Let's say I have: def bar(): String = Block: 2 + 2 .toString if I add a space before .toString it breaks. — Reply to this email directly, view it on GitHub <#17056 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGCKVSATZFZP4MFS276B5TW4NBBFANCNFSM6AAAAAAVRPSO54> . You are receiving this because you authored the thread.Message ID: ***@***.***>

-- Martin Odersky Professor, Programming Methods Group (LAMP) Faculty IC, EPFL Station 14, Lausanne, Switzerland

[kitbellew]

@tgodzik We have a design rule for indentation that adding or removing a single space should not usually change program behavior, as long as your basic indentation width is at least 2 spaces. Doesn't the PR break the behaviour from the other side? With the current logic if I add a space then I could reach the threshold and the program stops compiling.

@odersky: Yes, but that's OK. We want to prevent silent change of meaning, issuing compile errors is expected. - Martin

Won't this lead to silent change of meaning, without causing a compile error, with a single space:

from

foo:
  bar
 .baz

to

foo:
  bar
  .baz

where

class A(val value: Int) {
  def baz: A = new A(value + 1)
}
def foo(obj: A) = new A(obj.value * 2)

[odersky]

We assume the original program has at least two space indent.

[kitbellew]

We assume the original program has at least two space indent.

sorry, i wasn't clear. the original program did have two-space indent but also the first .baz was indented one space by mistake (and you propose to allow one-space mistakes). then someone makes an additional one-space accidental shift, and now the code has changed.

i am curious about the one-space leeway: is it practiced in any other existing optional-braces scenario? or is this a more forward-looking change?

[odersky]

@tgodzik So what do we do. I don't have the time to context switch to this again. Since Scala-meta already allows this, I propose we go ahead with the merge?

[tgodzik]

@tgodzik So what do we do. I don't have the time to context switch to this again. Since Scala-meta already allows this, I propose we go ahead with the merge?

I am a bit worried about it being too fragile, but I don't want to block it if you feel this is the way to go. Maybe we could get another opinion on this? @Kordyjan @nicolasstucki ?

[odersky]

I am a bit worried about it being too fragile, but I don't want to block it if you feel this is the way to go.

But you say scalameta already supports this?

[kitbellew]

I am a bit worried about it being too fragile, but I don't want to block it if you feel this is the way to go.

But you say scalameta already supports this?

I think scalameta inserts outdent (and closes the block tree) when the level of indentation drops below the current significant indent, and doesn't check that it matches anything (assuming the compiler would do that). the part that reads the dot doesn't care about indentation at all, it just attaches it to the earlier parsed tree. @tgodzik for double-check...

[prolativ]

Do I get this right that the suggested change would also allow writing code like

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

when one actually meant

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

?

[odersky]

yes

[tgodzik]

I think scalameta inserts outdent (and closes the block tree) when the level of indentation drops below the current significant indent, and doesn't check that it matches anything (assuming the compiler would do that). the part that reads the dot doesn't care about indentation at all, it just attaches it to the earlier parsed tree. @tgodzik for double-check...

I can confirm that's the case, we don't check indentations levels, we only care about the places where it can end, so the indentation need to be lesser than the current region. As long as the indentation doesn't drop below the region we treat it as the same region.

[odersky]

@Kobenko @nicolasstucki what is your opinion? If there are no strong opinions against, I propose to merge.

[Kobenko]

@Kobenko @nicolasstucki what is your opinion? If there are no strong opinions against, I propose to merge.

LGTM. If @susliko @ioaoue have a different opinion, please let us know