Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
eed3si9n opened this issue Dec 6, 2020 · 8 comments
Open

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

eed3si9n opened this issue Dec 6, 2020 · 8 comments

Comments

@eed3si9n
Copy link
Member

eed3si9n commented Dec 6, 2020

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
Copy link
Member

smarter commented Dec 7, 2020

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

@smarter smarter closed this as completed Dec 7, 2020
@eed3si9n
Copy link
Member Author

eed3si9n commented Dec 7, 2020

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
Copy link
Member

smarter commented Dec 7, 2020

I don't know enough about the parser to say. Somewhat related: #10372

@eed3si9n
Copy link
Member Author

eed3si9n commented Dec 7, 2020

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

@smarter
Copy link
Member

smarter commented Dec 7, 2020

As you wish.

@eed3si9n eed3si9n reopened this Dec 7, 2020
@eed3si9n eed3si9n changed the title False positive E129 "A pure expression does nothing" using optional braces new indentation violating the off-side rule should error Dec 7, 2020
@odersky
Copy link
Contributor

odersky commented Dec 7, 2020

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
Copy link
Member Author

eed3si9n commented Dec 7, 2020

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
Copy link
Member Author

eed3si9n commented Dec 7, 2020

I have a draft PR for this #10691.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants