Skip to content

Add error message - Comments.scala:128 #1621

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

Merged
merged 1 commit into from
Oct 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/dotty/tools/dotc/core/Comments.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import util.Positions._
import util.CommentParsing._
import util.Property.Key
import parsing.Parsers.Parser
import reporting.diagnostic.messages.ProperDefinitionNotFound

object Comments {
val ContextDoc = new Key[ContextDocstrings]
Expand Down Expand Up @@ -125,7 +126,7 @@ object Comments {
val newName = (tree.name.show + "$" + codePos + "$doc").toTermName
untpd.DefDef(newName, tree.tparams, tree.vparamss, tree.tpt, tree.rhs)
case _ =>
ctx.error("proper definition was not found in `@usecase`", codePos)
ctx.error(ProperDefinitionNotFound(), codePos)
Copy link
Contributor

@felixmulder felixmulder Oct 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

tree
}
}
Expand Down
39 changes: 38 additions & 1 deletion src/dotty/tools/dotc/reporting/diagnostic/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -543,5 +543,42 @@ object messages {
|
|""".stripMargin
}


case class ProperDefinitionNotFound()(implicit ctx: Context) extends Message(20) {
val kind = "Definition Not Found"
val msg = hl"""|Proper definition was not found in ${"@usecase"}"""

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

val explanation = {
hl"""|${"@usecase"} are only supported for ${"def"}s. They exist because with Scala's
|advanced type-system, we sometimes end up with seemingly scary signatures.
|
|Let's see an example using the `map`function:
|
|${"List(1, 2, 3).map(2 * _) // res: List(2, 4, 6)"}
|
|It's very straight forward to understand and use, but has such a scary signature:
|
|$noUsecase
|
|In order to mitigate this and ease the usage of such functions we have the ${"@usecase"}
|annotation for docstrings. Which can be used like this:
|
|$usecase
|
|Now when creating the docs, the method signature is substituted by the
|${"@usecase"} with a reader-friendly version. The compiler makes sure that it is valid.
|
|Because of this, you must use ${"def"} when defining ${"@usecase"}.""".stripMargin
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done again :) Thank you!

}

}