scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.73k stars 1.04k forks source link

new indentation violating the off-side rule should error #10671

Open eed3si9n opened 3 years ago

eed3si9n commented 3 years ago

Minimized code

compile:

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

  def test(name: String)(body: => Any) = ()

or more simply:

scala> def test(y: Int) =
     |   val x = 10
     |     x + y
     |
def test(y: Int): Int

Output

[warn] -- [E129] Potential Issue Warning: Test.scala:2:6
[warn] 2 |  test("hello")
[warn]   |  ^^^^^^^^^^^^^
[warn]   |A pure expression does nothing in statement position; you may be omitting necessary parentheses
[warn] one warning found

Expectation

Same as putting braces, no warnings should be printed.

[error]     test("hello")
[error] |    assert(1 == 1)
[error] |    ^^^^^^^^^^^^^^ 
[error] 🚩 Off-side rule violation. An indentation is started but the previous line does not end with a marker.
smarter commented 3 years ago

Output of -Xprint:typer:

package <empty> {
  class Test() extends Object() {
    {
      def $anonfun(body: => Any): Unit = this.test("hello")(body)
      closure($anonfun)
    }
    ():Unit
    def test(name: String)(body: => Any): Unit = ()
  }
}

So the warning is correct, test("hello") gets expanded to a lambda which is a pure expression. I guess you expected the assert to be a block argument because it's indented, but indentation syntax does not work that way: https://dotty.epfl.ch/docs/reference/other-new-features/indentation.html

eed3si9n commented 3 years ago

I guess you expected the assert to be a block argument because it's indented, but indentation syntax does not work that way

Could we please catch these in the parser like the follows:

   test("hello")
|    assert(1 == 1)
|    ^^^^^^^^^^^^^^ 
🚩 Off-side rule violation. An indentation is started but the previous line does not end with a marker.
smarter commented 3 years ago

I don't know enough about the parser to say. Somewhat related: https://github.com/lampepfl/dotty/issues/10372

eed3si9n commented 3 years ago

Should I open a new issue, or can I reuse this issue with a different title?

smarter commented 3 years ago

As you wish.

odersky commented 3 years ago

There are valid situations where one might want to indent some code, for instance to express a hierarchy of definitions (see Definitions.scala where this scheme is used). Or, what about this one:

def test(y: Int, z : Int) = 
  val x = 10
    x
  + y
  + z

That seems like it should be supported as well.

eed3si9n commented 3 years ago

There are valid situations where one might want to indent some code

The test example on Python would error as follows:

>>> def test(x, y):
...   x = 10
...     x
  File "<stdin>", line 3
    x
IndentationError: unexpected indent

If we're saying that indentation has semantics of { ... }, then my expectation is that the indentation must now be both necessary and sufficient with regard to starting a new block or a clause, and cannot have any other soft meanings Scala 2 may have admitted. Otherwise, it's misleading.

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

I ran into this while writing a unit test using MUnit. If Guillaume didn't show me -Xprint:typer I wouldn't have guessed what was happening.

To avoid breaking existing brace-style code, do you think it's possible to employ something like:

  1. By default, indentation is relaxed
  2. Once braces are omitted, the superfluous indentations are not allowed within the block/clause

?

// ok
def test(y: Int, z : Int) = {
  val x = 10
    x
  + y
  + z
}

// not ok
def test(y: Int, z : Int) =
  val x = 10
    x
  + y
  + z
eed3si9n commented 3 years ago

I have a draft PR for this https://github.com/lampepfl/dotty/pull/10691.