Dotty: Scala Wart: Presence of comments affects program logic

[lihaoyi]

Opening this issue, as suggested by Martin, to provide a place to discuss the individual warts brought up in the blog post Warts of the Scala Programming Language and the possibility of mitigating/fixing them in Dotty (and perhaps later in Scala 2.x). These are based on Scala 2.x behavior, which I understand Dotty follows closely, apologies in advance if it has already been fixed

---

Did you know the presence or absence of comments can affect the logic of your
program?

@ object Foo{
    def bar(x: Any) = println("Foo#bar(x) " + x)
    def bar = println("Foo#bar")
  }
defined class Foo

@ val baz = 1
baz: Int = 1

@ {
  Foo bar
  baz
  }
Foo#bar(x) 1

@ {
  Foo bar

  baz
  }
Foo#bar

@ {
  Foo bar
  // Wooo!
  baz
  }
Foo#bar(x) 1

@ {
  Foo bar
  // Wooo!

  baz
  }
Foo#bar


@ {
  Foo bar
  // Wooo!
  // Hoo!
  baz
  }
Foo#bar(x) 1

As you can see, this code behaves differently if we have a line between
the Foo bar and the baz, unless that line has a line comment on it!
When there's no newlines or the newline is filled by a comment,
Foo.bar(x: Any) gets called, and when there's a newline not filled by a
comment then the other overload Foo.bar gets called.

There are other places in the language syntax where this is the case:

@ {
  class X(x: Int)(y: Int)

  new X(1)(2)

  class Y(x: Int)
         (y: Int)

  new Y(1)(2)
  }
defined class X
res103_1: X = $sess.cmd103$X@6a58eda9
defined class Y
res103_3: Y = $sess.cmd103$Y@136ca935

@ {
  class Z(x: Int)

         (y: Int)
  }
cmd105.sc:3: not found: value y
       val res105_1 = (y: Int)
                       ^
Compilation Failed

@ {
  class W(x: Int)
  // Woohoo
         (y: Int)
  }
defined class W

A full listing of the places where this "comment can change behavior of
newlines" can be found in the
OneNLMax
rule of the ScalaParse grammar. I don't have an immediate answer for what the
correct solution is here, but I'm 99.9% sure we should make it so comments
like this don't affect the semantics of a Scala program!

PostScript:

As mentioned by Sebastian on reddit, this comes down to a question of "what do you consider comments to be", since in these cases the Scala grammar is looking for a blank line with no printable characters. Do we consider them to be "whitespace", which case a line with comments counts as "no printable characters", or do we consider them "things that do nothing", in which case there's still "something" with printable characters on the line preventing the "blank line" rule from kicking in. My personal opinion is that "treating them as whitespace" feels more natural, but I understand that's somewhat subjective

[odersky]

The tricky bit is this: Say you write

class X(y: Y)  // explanation what Y means
       (z: Z)

Now you want to add to the comment:

class X(y: Y)  // explanation what Y means
               // with some extra info
       (z: Z)

I think it would be very annoying if the program stopped working at that point.

[smarter]

I think in that situation the style that scalafmt uses is something like:

class X(y: Y)( // explanation what Y means
  // with some extra info
  z: Z
)

[odersky]

So I would classify this one as "not a wart", because there's a good reason why things are done the way they are currently done.

[odersky]

@smarter Sure, that's a workaround, but I find it esthetically not as nice. The Scala syntax was designed so that we should be able to write the opening parenthesis on a new line, as long as there is not a whitespace line before the parenthesis. Should we now forbid it? Why?

[lihaoyi]

Wouldn't this be an already-standard way of associating a long comment with each parameter?

/**
 * @param y Explanation what Y means
 *           with some extra info
 */
class X(y: Y)
       (z: Z)

Perhaps not 100% equivalent, but pretty close. It's also already "standard" enough all IDEs and tools like Scaladoc already know how to handle it, and people already know what it means.

Also, feel free to close any of these issues as "not a wart" if there's an explanation and consensus around it. The closed issue can continue to be a focal point if anyone else wants to look at the explanation and/or continue discussion

[NthPortal]

@lihaoyi The difference with that is, it's a scaladoc comment, so it ends up in the API documentation. You may have just wanted it in the source code (not quite the example here, but it seems plausible).

[odersky]

I don't think there's drive to change this, currently. Reopen if that should change.

[LPTK]

Just wanted to add a use case for this behavior, that I find very useful.
I often format mixin compositions with many traits as follows:

object A extends Class
  with Trait0
  with Trait1
  with Trait2
  ...

Then it's very easy to comment and uncomment one of the trait as needed:

object A extends Class
  with Trait0
  //with Trait1 // TODO fix #123 and re-enable this
  with Trait2
  ...

Or to document each mixin:

object A extends Class
  // used for foo:
  with Trait0
  // used for bar:
  with Trait1
  // used for baz:
  with Trait2
  ...

I've also left a message on the forum a while ago, with a proposal that could solve both this and the problem with postfix operators; it is copy-pasted below:

---

I think there’s a better solution to postfix operators, which would be to introduce _some_ limited measure of indentation sensitivity to Scala’s syntax.

val x = foo bar
baz

means:

val x = foo bar;
baz

and

val x = foo bar
  baz

means:

val x = foo bar baz

I think this will avoid confusing newcomers, since it’s a natural convention that most people already follow – that a non-indented line does not normally continue the expression above.

The same principle could be used to get rid of the fact that removing a comment in some places can break Scala code, which many people seem to find inexcusable.

foo.bar
{ ... }

means (new behavior, less surprising IMHO):

foo.bar;
{ ... }

and

foo.bar
  { ... }

means:

foo.bar { ... }

which is the same as:

foo.bar
// comment
  { ... }

same as:

foo.bar

  { ... }

[smarter]

@LPTK I like this proposal, but I think its viability depends on whether we can write a reliable rewriting rule to not break existing code.

[LPTK]

The simplest way to do it would be to materialize semicolons in the rewritten file everywhere they would have been "implicitly inserted" using the old rules. But that would kind of pollute the file.
So for each such insertion, we would first apply both the old and new parsers on the tree's characters and check that it parses to the same thing (otherwise insert the semicolon). No idea how easy/feasible that is in practice, though.