Add error message - Comments.scala:128

[iamthiago]

This commit adds the semantic object for the proper definition not found error.
It is part of the #1589

[iamthiago]

Ok, this is my first PR over here, so go easy with me :)
I have some doubts related to the explanation of this error. I particularly believe we should have something there, but I have no idea. I would appreciate any help.

[felixmulder]

I think a lot of these changes can be left out, basically the only addition here should be something like:

// you should be able to omit most of the path and get something like:
import reporting.diagnostic.messages.ProperDefinitionNotFound
// or even just:
import reporting.diagnostic.messages._

whichever floats your goat :)

[iamthiago]

Ahh cool, actually I think intellij tried to optimise it for me, but you know, does not always help :)

[felixmulder]

To get a good sense of what's going on here for the error message's explanation there are a couple of things you (or the user) need(s) to know:

1. What are @usecase?
2. Why do we care that we're getting an untpd.DefDef here?

[felixmulder]

Usecases are a workaround for ugly signatures that the user of a library does not need to know about. For instance:

trait List[+A] {
  def map[B, That](f: A => B)(implicit bf: CanBuildFrom[List[A], B, That] : That = ???
}

now the users of the list are going "dafuq 😕". Basically the implicit builder will effectively build a collection of the expected type - but the user just assumes that's what happens when you map - and doesn't need to know that.

So if we do this:

/** Map from List[A] => List[B]
  *
  * @usecase def map[B](f: A => B): List[B]
  */
def map[B, That](f: A => B)(implicit bf: CanBuildFrom[List[A], B, That]): That = ???

when building the documentation, the doc tool (which is a sort of compiler) will switch out the definition of map for the one defined in the @usecase. As such, the user reading the docs, won't see the ugly definition and be blissfully unaware :)

Because of this, the error occurs when the user has not written a def but something else (like val map: List[B] or w/e).

So perhaps the explanation can contain some of this information?

[iamthiago]

Great explanation, thanks for your time to do that, I will be working on it and should push a fix later :)

[iamthiago]

@felixmulder I have updated the code based on your explanation, take a look and let me know if you agree. Also, there is another PR with approved state with the same number(18), perhaps a merge should occur.

[felixmulder]

You're right, update yours to 19? I'll have a look at your changes first thing in the morning :)

[iamthiago]

yeah, I did a rebase with master and now it's 19!
Sorry the delay, but besides timezone, I still have to work 8 hours a day:( but we will get used to it and I will help as much as I can :)

[felixmulder]

I feel that this explanation still needs some improvement - I feel that it is not clear where you should put the @usecase "thingy". With the current explanation somebody might mistake it for an annotation - or am I in the wrong here?

As I mentioned before - @usecase is added to the docstring of the method. As such perhaps something roughly like:

val explanation = {
  val noUsecase =
    "def map[B, That](f: A => B)(implicit bf: CanBuildFrom[List[A], B, That]): That"

  val usecase =
    """|/** Map from List[A] => List[B]
       |  *
       |  * @usecase def map[B](f: A => B): List[B]
       |  */
       |def map[B, That](f: A => B)(implicit bf: CanBuildFrom[List[A], B, That]): That""".stripMargin

  hl"""|Usecases are only supported for ${"def"}s. They exist because with Scala's
       |advanced type-system, we sometimes end up with seemingly scary signatures.
       |The usage of these methods, however, needs not be - for instance the `map`
       |function
       |
       |${"List(1, 2, 3).map(2 * _) // res: List(2, 4, 6)"}
       |
       |is easy to understand and use - but has a rather bulky signature:
       |
       |$noUsecase
       |
       |to mitigate this and ease the usage of such functions we have the ${"@usecase"}
       |annotation for docstrings. Which can be used like this:
       |
       |$usecase
       |
       |When creating the docs, the signature of the method is substituted by the
       |usecase and the compiler makes sure that it is valid. Because of this, you're
       |only allowed to use ${"def"}s when defining usecases.""".stripMargin
}

could be the basis for your explanation :)

[iamthiago]

Well done again :) Thank you!

[felixmulder]

Don't worry about the time @thiagoandrade6 - there's no rush :)

[iamthiago]

@felixmulder Take a look in the code whenever you can. Basically I got your message but I have rewritten some parts of that. I believe anyone who got such error, will now easily get rid of that with this very good explanation :)

[felixmulder]

@thiagoandrade6 - thanks for your patience, I'll merge as soon as the CI passes :)

[iamthiago]

@felixmulder thanks for make my life easier, helping me to understand more about how the compile works. I have other several questions, but I will read more the code, docs, talks, whatever exist and asking you on demand. For now let's move on to the next PR :)

[felixmulder]

No worries - for questions not regarding a specific PR, I'll refer you to our gitter-channel :) happy coding!