Enforce indentation off-side rule

[eed3si9n]

Ref #10671

Currently

class A:
  val x = 1
    val y = 2

is admitted. This becomes confusing since this looseness ends up not
catching things like

class Test:
  test("hello")
    assert(1 == 1)

where assert(1 == 1) is actually not passed into test(...)(...).

Following the convention of indentation languages like Python, we should reject superfluous indentation, which can be defined as having two or more indentation widths within a region.

Test output

[info] Test run started
[info] Test dotty.tools.dotc.parsing.IndentTest.superfluousIndents2 started
-- Error: <code>:4:4 -----------------------------------------------------------
4 |    assert(1 == 1)
  |    ^^^^^^
  |    unexpected indent: found 4 spaces
[info] Test dotty.tools.dotc.parsing.IndentTest.parseBraces started
[info] Test dotty.tools.dotc.parsing.IndentTest.parseIndents started
[info] Test dotty.tools.dotc.parsing.IndentTest.superfluousIndents started
-- Error: <code>:4:4 -----------------------------------------------------------
4 |    val y = 2
  |    ^^^
  |    unexpected indent: found 4 spaces
[info] Test dotty.tools.dotc.parsing.IndentTest.innerClassIndents started

[eed3si9n]

Dogfooding / bootstrapping currently yields errors like

Error:  -- Error: /home/runner/work/dotty/dotty/library/src/scala/runtime/stdLibPatches/language.scala:36:0 
Error:  36 |  object noAutoTupling
Error:     |^^^^^^
Error:     |unexpected indent: found 0 spaces expected 2 spaces
Error:  one error found

but I'm not sure if it's an error in this code or in the existing scanner code since "found 0 spaces" seems wrong.

[odersky]

This would have to go through a thorough review, and I have already stated the problems with this approach in the issue. My current feeling is, it will certainly not fly at the present time since it breaks too much code.

The point to revisit would be if we make -Yindent-colons the default. But even then I would make the check semantics based, not syntax. I think there's real value in writing

val x = 1
   a
|| b
|| c

Some people want to format things this way.

[eed3si9n]

My current feeling is, it will certainly not fly at the present time since it breaks too much code.

Too much which code? My intent is that within a curly brace, we should continue to allow superfluous indentation. Here's a unit test I added for that:

  @Test
  def parseBraces: Unit =
    val code = s"""
      |class A {
      |  val x = 1
      |    val y = 2
      |}""".stripMargin
    assert(parseTextEither(code).isRight)

but not allow it when the user switch to indentation style:

  @Test
  def superfluousIndents2: Unit =
    val code = s"""
      |class Test:
      |  test("hello")
      |    assert(1 == 1)
      |""".stripMargin
    assert(parseTextEither(code).isLeft)

I think there's real value in writing

val x = 1
   a
|| b
|| c

I disagree on this point. I think, in many serious code base these variances are already formatted away by Scalafmt nowadays, and most people are happy to never think about it again. I see syntactically significant whitespace to be the next step in the continuum to make the shape of the code look the same regardless of who writes it, which is great for readability.

The point to revisit would be if we make -Yindent-colons the default. But even then I would make the check semantics based, not syntax.

class Test:
  test("hello")
    assert(1 == 1)

If indentation unambiguously meant introducing { ... } clause, then the above can be interpreted as test("hello"){assert(1 == 1)} without any colons. How would we change the parsing using the semantics of test(...) method etc?

[odersky]

I agree that sometimes code that's indented too much can be misleading. But it's also often useful to allow it. Here is a situation I came across yestderday. I had

  ...
  if someCondition then
    doSomething

and I wanted to comment out the if, like this:

  ...
  // if someCondition then
    doSomething

That was not meant to be permanent, just to try out things. Would that have worked with the proposed offside rule?

[sjrd]

Probably not, but then again it wouldn't have worked anyway if the thing just above the if was another indented block, like:

if cond1 then
  stat1
//if cond2 then
  stat2

In an indentation-based language, unfortunately the easiest way to avoid that problem is to do:

if cond1 then
  stat1
//if cond2 then
if true then
  stat2

[anatoliykmetyuk]

There was no activity on this one for a long while, so let's close it.

[eed3si9n]

On Scala 2 bug tracker there are probably issues (valid or otherwise) kept around for years. Is that not the case for Scala 3? Offside rule violation I still think is a bug, i.e. we should reject superfluous indentation.

I didn't notice that this was a PR on phone. #10671 is still open as bug.