No outdent needed for else

[tgodzik]

Minimized code

    val kind = if true then
        1
        else 2

Output

This compiles.

Expectation

We expect an indentation after then, but in this case else should be an error. No outdent should be inserted before else.

[tgodzik]

Specifically I found the issue here: https://github.com/lampepfl/dotty/blob/88541a3877a5e63f254e93a3b5b46e5fb7bba458/scala3doc/src/dotty/dokka/tasty/ClassLikeSupport.scala#L371-L373

[tgodzik]

Should we add something like:
An <outdent> is also inserted if the next token following a statement sequence starting with an <indent> closes an indentation region, i.e. is one of then, else, do, catch, finally, yield, }, ), ] or case. ?

Suggested by @kpbochenek

[tgodzik]

Actually, that is already in the doc, I misunderstood.

[tgodzik]

Sorry for the back and forth, but I spend a large amount of time trying to figure this condition out:

An <outdent> is also inserted if the next token following a statement sequence starting with an <indent> closes an indentation region, i.e. is one of then, else, do, catch, finally, yield, }, ), ] or case.

and it seems pretty unclear. We are inserting indents and outdents at tokenizer level and it's not really possible to distinguish between:

val kind = if cond then 
     println()
     else println("B") 

and

val kind = if cond then 
     println()
     if anotherCond() then println("A")
     else println("B") 

else in both cases follows a statement sequence, but only in first case it ends the indentation. What we would need to do is remember if all if are closed and the same for every other such keyword. Is there any other way to do it?

It also seems really hard to figure out what will happen in both scenarios.

[odersky]

Here is first example:

val kind = if cond then 
     println()
     else println("B") 

println() is a statement in a statement sequence starting with an <indent>. The token following it is an else. Hence, an <outdent> is inserted.

On the other hand:

val kind = if cond then 
     println()
     if anotherCond() then println("A")
     else println("B") 

Here, the else follows the statement println("A") which is not part of a statement sequence starting with <indent>. Hence, no <outdent> is inserted.

[tgodzik]

@odersky This is still a bit unclear. For example we have:

val kind = if true then 
  println()
  def hello = 
    println("Hello")
  else println("B") 

Here else follows a statement that is not part of a statement sequence starting with , but it is still ok.
I don't think this condition is clear enough:

An <outdent> is also inserted if the next token following a statement sequence starting with an <indent> closes an indentation region, i.e. is one of then, else, do, catch, finally, yield, }, ), ] or case. 

we should add:

and there exists an unclosed pair to the said token (if-else, for-do etc.)

which will add new level of complication, which we can deal with, but I am not sure what does it add to the language. Why not enforce:

if cond then
  statements...
else
  statements...

This would be similar as it's done in other indented based languages. I imagine this can cause a whole lot of confusion for people and no one will want to write it:

if cond then
  statements
  else
    statements

[romanowski]

I think the biggest problem with:

if cond then
  statements
  else
    statements

is that if someone inserts additional if below first one:

if cond then
  if cond 2 then
  statements
  else
    statements

then else will applied to 2nd if may be really confusing (or cause problems that are really hard to debug).

[odersky]

Here else follows a statement that is not part of a statement sequence starting with , but it is still ok.

I did not understand that. else does follow a statement that is part of a statement sequence starting with <indent>.
The token sequence is:

if true then <indent>
  println()
  def hello = <indent>
    println("Hello") <outdent>
 else println("B") 

So else follows the statement starting with def hello which is part of the statement sequence following <then>.

@romanowski Here's your example as a test:

def statements = ???
def test(cond: Boolean) =
  if cond then
    if cond then
    statements
    else
      statements

Here's the test output with -Xprint:typer:

def test(cond: Boolean): Unit = 
      if cond then 
        if cond then statements 
        else statements 
      else ()

That looks correct to me, no?

[tgodzik]

Here else follows a statement that is not part of a statement sequence starting with , but it is still ok.

I did not understand that. else does follow a statement that is part of a statement sequence starting with <indent>.
The token sequence is:

if true then <indent>
  println()
  def hello = <indent>
    println("Hello") <outdent>
 else println("B") 

So else follows the statement starting with def hello which is part of the statement sequence following <then>.

I just meant it all at a tokenizer level, where we add indentation in Scalameta, that else follow a statement inside hello. So at that point we cannot say that else ends the previous indentation. From what I checked in Dotty, in some places the parser actually detects indentation, which we currently do not do. It would be probably good that I try to look over the dotty parser, but I feel like it might be near impossible for us to duplicate that.

@romanowski Here's your example as a test:

def statements = ???
def test(cond: Boolean) =
  if cond then
    if cond then
    statements
    else
      statements

Here's the test output with -Xprint:typer:

def test(cond: Boolean): Unit = 
      if cond then 
        if cond then statements 
        else statements 
      else ()

That looks correct to me, no?

This one does look correct, it's just the fact that adding an if inside a block changes the whole tree here. else stops belonging to the first if and belongs to the newly added one.

Overall, I feel that this is confusing for users, really hard to implement for us and I don't really understand why is it needed? I think it's very much related to #10671 in that we should be much more strict about indentation.

[tgodzik]

I think the biggest difference between Scalameta and Dotty parser is that the first one tokenizes the input and then parses it, while the latter does it simultaneously. This allows to add the observe methods, which add the needed tokens at the correct positions.

I am afraid that to support some of the cases we will need to rework the scalameta parser to be more similar to how Dotty parses the input. Though, I am not sure if that is possible.

[odersky]

I am afraid that to support some of the cases we will need to rework the scalameta parser to be more similar to how Dotty parses the input. Though, I am not sure if that is possible.

Would it be possible to use the Dotty parser instead? That's what I understand ammonite ended up doing.

[tgodzik]

I should be able to rework the parser. Switching to dotty parser would be much more effort currently.

Although, isn't this related to the other discussed issue about making the indentation stricter?

Other indentation based languages expect 'else' to be on the same indentation level as 'if' from what I've seen.

What is the purpose of making it possible to have it on the same indentation as the previous block? Honestly, it seems like a very specific use case and nothing I've seen people format their code like.

Wouldn't it be simpler to just disallow it? In the dotty parser it requires a bit of a hack in a form of the observeOutdent method.

[tgodzik]

Also, I've seen this used only once in the entire dotty codebase and I am pretty sure it's by mistake.

[odersky]

I think we can make indentation stricter over time. But I am nervous about doing it now, since we make an implicit promise that reasonable Scala 2 code will continue to compile. And it's hard to predict 100% what's reasonable. I remember having seen quite a few unmotivated right-indents in code. The simple fact that I was forced to add the outdent trick shows that. I would not have done that if existing code compiled already OK without it. We could go back, remove outdent and see what breaks.But I'd prefer not to "tighten the screws" so late in the 3.0 cycle, since there might be a backlash.

[tgodzik]

Ok I think I get where it's coming from. Are we are talking about a situation like:

if (cond) {
  statement()
} else {
  statement()
}

and that would be automatically rewritten by the compiler to:

if (cond)
  statement()
  else 
  statement()

?

If that is true I wonder if we shouldn't fix the rewrite rules to fix the indentation levels too? Or it should be something that scalafmt would actually do and not the automatic rewrite rules in the compiler?

But I am nervous about doing it now, since we make an implicit promise that reasonable Scala 2 code will continue to compile.

Scala 2 should continue to compile as it will include braces I think. So if someone has a right indented else it will not be a problem as long as they don't switch to braceless syntax. And if they do, they might still have issues with some other things, so I think they should be aware of how optional braces work if they indeed want to work with it. I don't think we will be able to forsee all the possible situations.

I do understand the hesitation here, but this might take a bit longer for us to support it. Though, I think I have a plan on how to do it now. I will just hold off on implementing it until the syntax is 100 % set.

[odersky]

No, this has nothing to do with indentation rewrites, they do the right thing.

if cond then
  statement()
else 
  statement()

It has to do with the fact that indentation is significant everywhere, even inside braces. So we have to accept all existing code, unless the code is clearly misleading. So, this is misleading:

if (x >= 0)
  println(x)
  1
else -1

whereas a shifted else looks weird, but is not as such misleading.

[tgodzik]

It has to do with the fact that indentation is significant everywhere, even inside braces. So we have to accept all existing code, unless the code is clearly misleading. So, this is misleading:

if (x >= 0)
  println(x)
  1
else -1

whereas a shifted else looks weird, but is not as such misleading.

Why is this misleading? I mean it's a pretty clear if-else.

if (x >= 0)
  println(x)
  1
  else -1

This is much more misleading.

What is an exact situation (code in Scala 2) that will not work if we remove the condition for else ? If the code works currently with braces and rewrites work correctly then I don't really see the danger.

[tgodzik]

Besides in Scala 2 we would need to write it:

if (x >= 0){
  println(x)
  1
}  else -1

To make it work.

[tgodzik]

Also it seems the docs mention:

It is an error if the indentation width of the token following an <outdent> does not match the indentation of some previous line in the enclosing indentation region. For instance, the following would be rejected.

if x < 0 then
    -x
  else   // error: `else` does not align correctly
     x

So this will not work:

  if x < 0 then
      -x
      -x
     else  
        x 

but this will:

  if x < 0 then
      -x
      -x
      else  
        x 

Why do we disallow that particular case, but not the other? This seems a bit inconsistent and is really hard to spot.

[tgodzik]

I still find it a bit inconsistent, but fixed in scalameta parser scalameta/scalameta#2244