Improved error messages in Desugar.scala

[ShaneDelmore]

No description provided.

[ShaneDelmore]

Is this reasonable advice?

[felixmulder]

As this is how you'd solve the problem, I think it is totally reasonable advice :)

When you say "Extend the object with the desired type", are you implying trait T; object O extends T and placing the self type in T?

If so, perhaps phrase it as "Let the object extend a trait containing the self type"

[ShaneDelmore]

Should I try to create the class definition here using args from the AST? Or is that excessive/difficult?

[ShaneDelmore]

I would have liked to add the args here but did not see how to grab them. Can I use scala.meta for this?

[felixmulder]

Currently, using scala.meta for this is not possible. It might be in the future!

Hmm, I can't verify this right now but something like:

1. check if cdef.rhs is a Template
2. if so get the constr (a DefDef)

Check what .show reveals on the constructor. Should be something like: def <init>(param1: X, param2: Y)

See bellow instead :)

[felixmulder]

Nice! I like the usage of the programmers own code here - makes for something that's easy to grok.

[felixmulder]

Perhaps just give a quick reason as to why - i.e. "There may not be any method, member or object in scope with the same name as the implicit class and a case class automatically gets a companion object with the same name."

[ShaneDelmore]

That's great, I did not know the reasoning behind this until you explained it.

[smarter]

Maybe you could link to this page which lists all the restrictions of implicit classes: http://docs.scala-lang.org/overviews/core/implicit-classes.html

[ShaneDelmore]

Great idea, I will add this.

[felixmulder]

As this is how you'd solve the problem, I think it is totally reasonable advice :)

When you say "Extend the object with the desired type", are you implying trait T; object O extends T and placing the self type in T?

If so, perhaps phrase it as "Let the object extend a trait containing the self type"

[felixmulder]

@ShaneDelmore - wait, scratch that. You're in the classDef method, and on Desugar.scala:253 you already have basically all you'll need at least for the implicit class one.

[smarter]

I wouldn't recommend (or even mention) package objects, they have tricky semantics and are not usually what you want to do.

[ShaneDelmore]

Is there a highlight available for URLs? The highlighting on this treats http: as plain text and //restOfTheUrl as a comment.

Also, do highlighted sections compose?

[felixmulder]

All interpolated strings in hl"..." get syntax highlighting. If you want to interpolate implicitClassRestrictions in you highlighted block I'd wrap it in a NoColor(...) Highlight. Already "colored" strings do not get interpolated by the interpolator.

See: https://github.com/lampepfl/dotty/blob/master/src/dotty/tools/dotc/printing/Highlighting.scala#L55

[ShaneDelmore]

This is duplicated from Desugar.scala. Is it preferred to duplicate it here to keep the coupling looser or should I instead add more parameters to the case class and pass along explicitely what I need, name, impl, constr0?

[felixmulder]

Good question, I think there's a risk that more people will need to print the class def and as such perhaps we should factor this out into a method call that takes the cdef - a small bit of duplication here is fine in order for it to be easy to use for new contributors IMHO.

[ShaneDelmore]

Now that the explanation body has been simplified there isn't really much here worth extracting.

[ShaneDelmore]

This feels ugly but I couldn't think of a better way to do it. Should I extract this or keep it contained and out of the way since it won't be needed once we can use scala.meta?

[felixmulder]

I would actually prefer this to have an else clause that is something like {\n ...\n }.

My motivation for this is if we have an implicit class with a lot of members, they will all get printed and bloat the message. For copy-pastability (is that even a word?), I think it's fine if we leave out the complete method body.

[ShaneDelmore]

Sounds good. I will try to make the suggested updates this evening.

[ShaneDelmore]

Example output:

-- [E010] Syntax Error: test.scala -------------------------------------------------------------------------------------
2 |implicit class Foo(n: Int)
  |^^^^^^^^^^^^^^^^^^^^^^^^^^
  |An implicit class may not be top-level

Explanation
===========
There may not be any method, member or object in scope with the same name as the
implicit class and a case class automatically gets a companion object with the same name
created by the compiler which would cause a naming conflict if it were allowed.

For a full list of restrictions on implicit classes visit
http://docs.scala-lang.org/overviews/core/implicit-classes.html

To resolve the conflict declare Foo inside of an object then import the class
from the object at the use site if needed, for example:

object Implicits {
  implicit class Foo(val n: Int)
}

import Implicits.Foo

-- [E010] Syntax Error: test.scala -------------------------------------------------------------------------------------
4 |implicit class Foo(bar: Int, baz: String) {
  |^
  |An implicit class may not be top-level
5 |  def baz = bar.toString
6 |
7 |  def qux = baz.take(5)
8 |}

Explanation
===========
There may not be any method, member or object in scope with the same name as the
implicit class and a case class automatically gets a companion object with the same name
created by the compiler which would cause a naming conflict if it were allowed.

For a full list of restrictions on implicit classes visit
http://docs.scala-lang.org/overviews/core/implicit-classes.html

To resolve the conflict declare Foo inside of an object then import the class
from the object at the use site if needed, for example:

object Implicits {
  implicit class Foo(val bar: Int, val baz: String) {
    def baz = bar.toString

    def qux = baz.take(5)
  }
}

import Implicits.Foo

[ShaneDelmore]

Updated error on def with body to prevent overly long error explanation:

-- [E010] Syntax Error: test.scala -------------------------------------------------------------------------------------
4 |implicit class Foo(bar: Int, baz: String) {
  |^
  |An implicit class may not be top-level
5 |  def baz = bar.toString
6 |
7 |  def qux = baz.take(5)
8 |}

Explanation
===========
There may not be any method, member or object in scope with the same name as the
implicit class and a case class automatically gets a companion object with the same name
created by the compiler which would cause a naming conflict if it were allowed.

For a full list of restrictions on implicit classes visit
  http://docs.scala-lang.org/overviews/core/implicit-classes.html

To resolve the conflict declare Foo inside of an object then import the class
from the object at the use site if needed, for example:

object Implicits {
  implicit class Foo(val bar: Int, val baz: String){
 ...
 }
}

import Implicits.Foo

[smarter]

I think it would be nice to make it more explicit that the import Implicits.Foo is a at the use site, for example by writing:

object Implicits {
  implicit class Foo(val bar: Int, val baz: String){
 ...
 }
}

...

// At the use site:
import Implicits.Foo

[ShaneDelmore]

-- [E011] Syntax Error: test.scala -------------------------------------------------------------------------------------
14 |  implicit case class Foo(bar: Int, baz: String)
   |  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |  A case class may not be defined as implicit

Explanation
===========
implicit classes may not be case classes. Instead use a plain class:
  example: implicit class Foo...

For a full list of restrictions on implicit classes visit
http://docs.scala-lang.org/overviews/core/implicit-classes.html

[ShaneDelmore]

-- [E012] Syntax Error: test.scala -------------------------------------------------------------------------------------
13 |  self: Foo =>
   |  ^^^^^^^^^
   |  objects must not have a self type

Explanation
===========
objects must not have a self type:

Consider these alternative solutions:
  - Create a trait or a class instead of an object
  - Let the object extend a trait containing the self type:
      example: object Test extends Foo

[ShaneDelmore]

-- [E013] Syntax Error: test.scala -------------------------------------------------------------------------------------
24 |  val largeTuple = (1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23)
   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |                   A tuple cannot have more than 22 members

Explanation
===========
This restriction will be removed in the future.
Currently it is possible to use nested tuples when more than 22 are needed, for example:

  ((1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22)(23))

[felixmulder]

Super impressive work, keep up the good work :)

[ShaneDelmore]

-- [E010] Syntax Error: test.scala -------------------------------------------------------------------------------------
4 |implicit class Foo(bar: Int, baz: String) {
  |^
  |An implicit class may not be top-level
5 |  def baz = bar.toString
6 |
7 |  def qux = baz.take(5)
8 |}

Explanation
===========
There may not be any method, member or object in scope with the same name as the
implicit class and a case class automatically gets a companion object with the same name
created by the compiler which would cause a naming conflict if it were allowed.

For a full list of restrictions on implicit classes visit
  http://docs.scala-lang.org/overviews/core/implicit-classes.html

To resolve the conflict declare Foo inside of an object then import the class
from the object at the use site if needed, for example:

object Implicits {
  implicit class Foo(val bar: Int, val baz: String){
 ...
 }
}

// At the use site:
import Implicits.Foo