Dotty: Missing error messages

[felixmulder]

We have a multitude of errors that need to be ported to the new scheme - you'll find these under "Error Messages" below. But first, a tutorial:

HOWTO on creating new messages

Once you've identified an error you want to replace, you should decide what semantic information is needed in the message - i.e. the position, perhaps a tree or some types. For this example we're going to be implementing the error message for the following error:

case class Foo // you're not allowed to have a case class without a single parameter list!

So we create a file with this and compile it using ./bin/dotc test.scala. This gives us the error message:

case class Foo
           ^
           case class needs to have at least one parameter list

So now we search the repository for "case class needs to have" - I do this with git grep -n "case class needs to have".

And now we find that in src/dotty/tools/dotc/ast/Desugar.scala we have the following line:

ctx.error("case class needs to have at least one parameter list", cdef.pos)

Cool - now we know what to replace. So next step is to add a case class to src/dotty/tools/dotc/reporting/diagnostic/messages.scala. The case class that we add should extend Message:

abstract class Message(val errorId: Int) {
  /** The `msg` contains the diagnostic message e.g:
    *
    * > expected: String
    * > found:    Int
    *
    * This message wil be placed underneath the position given by the enclosing
    * `MessageContainer`
    */
  def msg: String

  /** The kind of the error message is something like "Syntax" or "Type
    * Mismatch"
    */
  def kind: String

  /** The explanation should provide a detailed description of why the error
    * occurred and use examples from the user's own code to illustrate how to
    * avoid these errors.
    */
  def explanation: String
  ...
}

Now we name our case class something useful and give it a unique errorId:

case class CaseClassMissingParamList() extends Message(4)

Since we can see that the call to ctx.error took the position of a cdef: TypeDef, we could perhaps use this in the constructor of our Message?

case class CaseClassMissingParamList(cdef: untpd.TypeDef) extends Message(4)

It might also be that we're going to be manipulating cdef tree, as such we should pass a Context to get all the implicit operations:

case class CaseClassMissingParamList(cdef: untpd.TypeDef)(implicit ctx: Context)
extends Message(4)

Alright! Now we can start adding things to our Messsage:

case class CaseClassMissingParamList(cdef: untpd.TypeDef)(implicit ctx: Context)
extends Message(4) {
  val kind = "Syntax" // this will be shown as a delimiter: "-- [E004] Syntax X -------"

  // we can use the "hl" interpolator to give syntax highlighting to our message:
  val msg =
    hl"""|A ${"case class"} must have at least one parameter list"""

  // not everything that is interpolated will be highlighted, for instance you can use
  // colors like: Red("must") - this will get the red color.
  val explanation =
    hl"""|${cdef.name} ${Red("must")} have at least one parameter list, if you would rather
         |have a singleton representation of ${cdef.name}, use a "${"case object"}".
         |Or, add an explicit `()' as a parameter list to ${cdef.name}.""".stripMargin
}

Stable Identifier

So far, we've simplified the above cases a smidge, the error messages themselves actually don't extend Message by passing an integer. They should actually create an entry into the java enum defined in ErrorMessageID.java. This will enable us to have stable identifiers which helps tooling between the different compilers.

Now you replace the call to ctx.error(String) with ctx.error(CaseClassMissingParamList(cdef)) - and you're done with the implementation!

Tests

To test your solution, add a test to ErrorMessagesTests.scala. An example for TypeMismatch is provided and documented.

Online documentation

We really want to have online documentation providing more detailed information on what went wrong. Currently the idea is to create markdown pages in the docs/docs folder. Any contribution here would be greatly appreciated!

Things to keep in mind

• You're creating a semantic object - this object will either be the output of the terminal or it could be used in an IDE, so make sure to include the relevant things. Typically: Types, Position, Tree
• Name your error something easy to understand!
• If you think your error doesn't need an explanation - that's fine - just put val explanation = ""
• If you find yourself not being able to manipulate trees - do you have the implicit context?
• "HALP!" - please post in our gitter channel: https://gitter.im/lampepfl/dotty or comment on this issue :smiley:
  

  Missing Messages

Please comment below saying which messages you'd like to tackle.

Available

• Applications.scala:95 (@aesteve)
• Checking.scala:49
• Checking.scala:526 (@benkobalog)
• Checking.scala:533
• Checking.scala:539
• Checking.scala:555
• Checking.scala:565
• Checking.scala:576
• Checking.scala:583
• Checking.scala:602
• Checking.scala:622
• Checking.scala:628
• Checking.scala:646
• Checking.scala:648
• Checking.scala:692
• Checking.scala:702
• Checking.scala:716
• Docstrings.scala:30
• Namer.scala:222
• Namer.scala:258
• Namer.scala:1081
• Namer.scala:1105
• Parsers.scala:2266
• Parsers.scala:2438
• Parsers.scala:2456
• PatternMatcher.scala:911
• RefChecks.scala:694
• RefChecks.scala:698
• RefChecks.scala:874
• SymDenotations.scala:1964
• TypeAssigner.scala:34
• TypeAssigner.scala:208
• TypeAssigner.scala:233
• TypeAssigner.scala:299
• TypeAssigner.scala:314
• TypeAssigner.scala:356
• TypeAssigner.scala:358
• TypeAssigner.scala:416
• Typer.scala:174
• Typer.scala:196
• Typer.scala:355
• Typer.scala:411
• Typer.scala:866
• Typer.scala:1172 (@Wojtechnology)
• Typer.scala:1519
• Typer.scala:1572 (on hold)
• Typer.scala:1731
• Typer.scala:1924
• Typer.scala:2015

Contributors

• @abeln
• @AndrewZurn
• @b-studios
• @gvolpe
• @insdami
• @jarrodj
• @johnregan
• @jyotman94
• @ljdelight
• @lorandszakacs
• @m-sp
• @mads-hartmann
• @maseev
• @mikla
• @msosnicki
• @rubenpieters
• @sebastianharko
• @ShaneDelmore
• @thiagoandrade6

[markusthoemmes]

This is an _awesome_ way of empowering people to contribute to the project. I think I'll use this to contribute. Thanks a lot for writing this up @felixmulder!

[markusthoemmes]

To avoid duplicated work: I'm taking on Parsers.scala:403 (shall we just edit the issue description and put names behind the specific messages to keep it transparent for everyone?)

[felixmulder]

@markusthoemmes: yes! Done :) Added a clarification to the top :)

[anicolaspp]

👍

[sebastianharko]

Indeed, this is an awesome way to get people to contribute to the project ... I'm tackling Parsers.scala:2064

[ShaneDelmore]

I'll take Desugar.scala and ErrorReporting.scala.

[sebastianharko]

@felixmulder: I created a PR for Parsers.scala: 2064 :-)

[ljdelight]

Hi, I'm new here. I'll take Parsers.scala:1737, Parsers.scala:1738, Parsers.scala:1739, Parsers.scala:1901

[sebastianharko]

Taking Parsers:1492, Parsers:1329, Parsers: 626 and TypeAssigner.scala:449

[AndrewZurn]

Was just sitting down with @ShaneDelmore and looking at these a little bit. Looks like quite a few Parsers and TypeAssigner messages left, if there are one or two 'easier' messages in there that I can take a look at, would someone mind assigning those to me?

Andrew

[jyotman]

Hi, Working on Parsers.scala:1180.

[felixmulder]

@AndrewZurn - how about Parsers.scala:695 to start off with? I'll assign you, but if you don't want it let me know :)

[sebastianharko]

@felixmulder can you mark Parsers:626 and Parsers:1492 as complete :-) ?

[felixmulder]

@sebastianharko - yes done! Thanks

[iamthiago]

Hi, also new over here, just trying to help this lovely project... I will work for now on Comments.scala:128, JavaParsers.scala:233 and Parsers.scala:319 ok?

[felixmulder]

@thiagoandrade6 - sounds great :)

[ALPHA-60]

I'd like to work on TypeAssigner.scala:447.

[AndrewZurn]

@felixmulder how about Parsers:1079 and Parsers:1082?

[m-sp]

working on Parsers.scala:1620

[maseev]

@felixmulder Is there any chance for a Scala newbie to contribute to this issue? Maybe there are some easier error messages? 😃

[felixmulder]

@maseev - I think it's a perfect opportunity, you'll get to dissect an error and learn the reason as to why it is an error in the first place. Some things are non-obvious to beginners, and I think if you understand the explanation, then somebody with the same skill-level can too.

I would clone the repo and have a look at the ones available from Parsers.scala, they should be the most straight forward. Let us know if you need help on gitter.

That being said, you should probably have some general programming knowledge :)

[maseev]

@felixmulder Got it! I'll definitely have a look.

[Danny02]

working on CollectEntryPoints.scala:69

[iamthiago]

Working on Parsers.scala:307

[Blaisorblade]

I got started with Parsers.scala:307!

[Blaisorblade]

BTW: shouldn't the kind be an ADT rather than a string?

[felixmulder]

@Blaisorblade: in the end, that may be best - it was kept as a string so that it is easy to customize for contributors without having to think about "can I add this?" or "should I just use the CC that doesn't really fit my error?"

[iamthiago]

@Blaisorblade take a look on my PR, it's about the same issue. Your contribution is welcome :)

[maseev]

@felixmulder I'll take this one - Parsers.scala:602, if you don't mind. Looks pretty straightforward.

[smarter]

BTW: shouldn't the kind be an ADT rather than a string?

I was thinking that we could have traits SyntaxMessage,TypeMessage, etc that define val kind: String, then the semantic information is embedded in the class hierarchy

[Blaisorblade]

Right now it seems the kind is also indicated by picking the error-reporting method (syntaxError or others). Something similar happens for positions:

You're creating a semantic object - this object will either be the output of the terminal or it could be used in an IDE, so make sure to include the relevant things. Typically: Types, Position, Tree

I've just noticed this and thought back to IdentifierExpected and AuxConstructorNeedsNonImplicitParameter — they include none of those things. Same for #1645 and for my take on it in
https://github.com/lampepfl/dotty/compare/master...Blaisorblade:topic/1645-parser-error.

In fact, right now the positions are passed directly to syntaxError, so having them also seem kind of redundant. What about having def position: Position in Message instead?

[felixmulder]

@Blaisorblade - you're welcome to improve on these messages by passing in whatever you need in order to bring semantic meaning to them.

In fact, right now the positions are passed directly to syntaxError, so having them also seem kind of redundant. What about having def position: Position in Message instead?

A Message is contained within a MessageContainer which carries the position. The reporters consume MessageContainers. These containers have a level - Warning, Error, Info etc. How this happens:

syntaxError calls ctx.error which in turn is implemented in trait Reporting, this creates the MessageContainer appropriate to what has been reported. I.e. if a Message is reported as a warning, it will be turned into a Warning container. These containers are then consumed by ConsoleReporter which prints kind, followed by the level represented as a string.

[jozi-k]

I will take Parsers.scala:2110 and 2112.

[b-studios]

I started working on Parsers.scala:533 but noticed that the issue could also be addressed by refactoring the call to syntaxError away. Would that be a viable option for you?

[felixmulder]

@b-studios: if you want you can replace line 533 with something like

if (!thisOK && in.token != DOT) ctx.error("`.' expected", source atPost Position(in.offset))

But we want to keep the method syntaxError for a while so that it can be used as a convenience function.

[b-studios]

@felixmulder: Just to clarify, I suggested to factor the call to syntaxError away, not the method syntaxError itself. In particular, the same syntax error would eventually be raised by accept(DOT) in someDotSelectors.

[Blaisorblade]

@Blaisorblade - you're welcome to improve on these messages by passing in whatever you need in order to bring semantic meaning to them.

Thanks for the explanation—those errors might then be fine; in my branch, I now point out in Message ScalaDocs that the position is part of MessageContainer, not semantic information.

[felixmulder]

@b-studios: yes - exactly, I was just making sure you don't do unnecessary work :)

[maseev]

I'll take these ones - Parsers.scala:1425, Parsers.scala:1437.
BTW, @felixmulder we can mark Parsers.scala:602 as done, because the error message implementation has already been merged into upstream, right?

[aviralg]

@felixmulder I am working on MarkupParsers.scala:324 and 326

[rethab]

@felixmulder I was going to take a look at the following, however:

• Parsers:885 (view bounds are deprecated)
  
  
  
  • dotty suggests context bounds right now
  
  
  • nsc (with -deprecation and -Xfuture) suggests implicits (with a good example)
  
  
  • when they added the warnings to nsc, there were quite some discussions on what to recommend instead. what is dotty's stance on this?
  
  
• PostTyper:182 (super now allowed in methods annotated with @inline)
  -- scala does not warn about this
  -- why is this not allowed anyway?

[felixmulder]

@rethab:

Context bounds are syntactic sugar, e.g:

def foo[T: Ordering](t: T) = ???

// desugars into:
def foo[T](t: T)(implicit ord: Ordering[T]) = ???

So we're actually recommending the same thing I guess...

Regarding @inline: in scalac (or nsc) the inline is heuristics defined, in Dotty it's mandatory. I.e. whatever you define as inline in Dotty, will be inlined. This means that calling super in a method declared inline and compiling it with scalac will probably not inline it - but we have to. The call site for the inline method probably does not have the same super and therefore it would lead to weird behavior unless we add a forwarder to the correct super etc.

[maseev]

@felixmulder I think we can mark Parsers.scala:1437 as implemented 'cause PR was successfully merged.

[jarrodj]

@felixmulder, I'll start working on Parsers.scala:664

[abeln]

@felixmulder looking at TypeAssigner.scala:298

[adamtrousdale]

@felixmulder working on Parsers.scala:712 (which is now 733)

[b-studios]

@felixmulder Just to keep the list in sync, 533 now is merged and can be crossed out :)

[ennru]

Is there a good way to include a test case triggering the expected compilation error?

[felixmulder]

@ennru:

The easiest way currently is to create a check file in the repl tests dir, e.g: ./tests/repl/mytest.check

and then run dotty-compiler/testOnly dotc.tests -- *repl_all.

I'd love to have a custom Reporter that instead collected the errors in their case class form so that we could simply match on it in a unit test. In fact, here's an issue detailing the necessary steps for that: https://github.com/lampepfl/dotty/issues/1965

Update: functionality for this has been merged: #1966. I'll update the main post.

[ennru]

Can -explain be enabled per file during dotty-compiler/testOnly dotc.tests -- *repl_all ?

[felixmulder]

@ennru: it could be changed, but IMHO we should move away from testing error messages using the repl tests, instead - expose elements of the error message that you need to be able to test.

The idea is to provide semantic objects, so if a TypeMismatch message contains the expected type - it should expose it so that tooling can take that into account.

[abeln]

TypeAssigner.scala:298 was fixed in https://github.com/lampepfl/dotty/commit/d0621108bad55f9fc66c1c5ade9a0b7edb3117e7

[mads-hartmann]

I think the line numbers in the description might have drifted a bit. If no one is already working on I'll grab (Parsers.scala:2385) ☺️

only `case class` or `case object` allowed

[insdami]

2725 Is Parsers:2387 taken?

expected class or object definition

[felixmulder]

@insdami: looks like it's still there, go ahead! :)

[gvolpe]

I think the line numbers are outdated, especially the ones defined for files where multiple PRs are being merged. Every time a PR is merged the line number changes... Despite this, I can take Typer.scala:453 which I think refers to the error reported here? https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/typer/Typer.scala#L482

[gvolpe]

I also have identified this one, that I guess I know how to tackle it. https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/typer/Typer.scala#L770

[felixmulder]

@gvolpe - they are indeed a bit outdated, I'll update them on Monday!

I'm assigning you to the correct ones in the list :tada:

[msosnicki]

Hi, @felixmulder can I take the PostTyper.scala:182?

[johnregan]

Hi, @felixmulder can I take the Parsers.scala:1685 ?

[gvolpe]

Here's my take for Typer#482, https://github.com/lampepfl/dotty/pull/2779, sorry for the delay!

[mikla]

Hello, can I take Typer.scala:693?

[mikla]

Taking Typer.scala:775 (which is actually Typer.scala:801) "wrong number of parameters" error.

[lorandszakacs]

Can I take Parsers.scala:1709?

Which rather seems to be Parsers.scala:1703:
syntaxError("duplicate private/protected qualifier")

EDIT:
I shamelessly went ahead and did a PR:
https://github.com/lampepfl/dotty/pull/2897

[lorandszakacs]

Took Parsers.scala:2177 which was missing from this error index, or is out of sync.

Here's the PR:
https://github.com/lampepfl/dotty/pull/2919

Thanks!

[lorandszakacs]

I did a simple semantic search in Intellij and got following list of errors that need updating.
Valid for commit 0ec7e56ffc55c8127127a8a193e6824935d82afc:

Parsers.scala:2150
Parsers.scala:2257
Parsers.scala:2429
Parsers.scala:2447
Parsers.scala:2491
Scala2Unpickler.scala:681
SymDenotations.scala:1924
TypeAssigner.scala:35
TypeAssigner.scala:196
TypeAssigner.scala:223
TypeAssigner.scala:287
TypeAssigner.scala:300
TypeAssigner.scala:302
TypeAssigner.scala:339
TypeAssigner.scala:341
TypeAssigner.scala:357
TypeAssigner.scala:359
TypeAssigner.scala:399
TypeAssigner.scala:433
TypeAssigner.scala:548
Typer.scala:393
Typer.scala:862
Typer.scala:985
Typer.scala:990
Typer.scala:992
Typer.scala:1107
Typer.scala:1427
Typer.scala:1521
Typer.scala:1555
Typer.scala:1574
Typer.scala:1898 (def missingArgs)

[TheReturningVoid]

I'll take Typer:992 (right now, it's Typer:991) if no-one else is on it.

[TheReturningVoid]

And finished with that one; it's in #3094.

[hepin1989]

Hi,I know Dotty ships great detailed error message, could we make these error message more international ?
Eg, make them a template and could have native language support.
like

case class Foo
           ^
           case class needs to have at least one parameter list

could be

case class Foo
           ^
           case class 至少需要1个参数列表

[smarter]

@hepin1989 This is an interesting idea, I suggest you open a separate issue with more technical details on how this could be done.

[CucumisSativus]

I'll take Scala2Unpickler.scala:681

[hermesespinola]

Hey there! I'll take Parsers.scala:2150

[maseev]

I'll take this one - Typer.scala:1898 (def missingArgs). Looks pretty straightforward to me.

[maseev]

@felixmulder it looks like these ones:

• Parsers.scala:2150
• Typer.scala:985
• Typer.scala:990

• Typer.scala:992

  are already implemented.

[maseev]

I'll take this one - Typer.scala:1427 (ctx.error("only static objects can extend scala.Phantom", cdef.pos))

[maseev]

And this one - Typer.scala:1555 (ctx.errorOrMigrationWarning(i"not a function: ${res.tpe}; cannot be followed by _", tree.pos))

[hermesespinola]

I'll take Typer.scala:1427 - ctx.error("only static objects can extend scala.Phantom", cdef.pos)

[maseev]

@hermesespinola It looks like I already implemented this part in this PR. Unfortunately, if I understand correctly, phantom types will be removed from Dotty, so the PR will probably be closed.

[hermesespinola]

Oh, didn't realize, even though it was two comments before. Just saw the comment from Odersky. Thanks for the info.

[karolchmist]

I'll try Parsers.scala:2500

[maseev]

It looks like these ones:

• TypeAssigner.scala:374
• TypeAssigner.scala:376
• Parsers.scala:2500

are already implemented.

[maseev]

And these:

• Checking.scala:498
• Checking.scala:501

are related to Phantom types which are going to be removed, according to this message.
Should they also be removed from the list?

[thebignet]

Working on RefChecks.scala:609

[felixmulder]

Thanks for helping out with updating the missing messages. I updated the list according to your reports @maseev 👍

[maseev]

I'll take this one - Checking.scala:551

[maseev]

I think I'll take this one - Namer.scala:880 as well

[maseev]

Working on Namer.scala:872

[markarasev]

Hello, I'm taking Applications.scala:95

[gosubpl]

I'd like to take Applications.scala:972.

[gosubpl]

What about Applications.scala:951-953:

            errorType(
              ex"Pattern type $unapplyArgType is neither a subtype nor a supertype of selector type $selType",
              tree.pos)

Is that also something that can be fixed in this ticket?

[smarter]

@gosubpl Yes!

[gosubpl]

Thanks for the update @smarter . Taking Applications.scala:952.

[maseev]

There are a bunch of error messages that have been implemented lately:

• Checking.scala:551
• Namer.scala:880
• Namer.scala:872
• Applications.scala:972
• RefChecks.scala:609

[vitorsvieira]

Folks,

Following messages were implemented:

• PatternMatcher.scala:911
• RefChecks.scala:694
• RefChecks.scala:698

Implemented but discarded as the code block won't be continued in dotty:

• RefChecks.scala:874

[gosubpl]

Also taking Applications.scala:834 and Applications.scala:942

[Wojtechnology]

Taking Typer.scala:1109 which is now Typer.scala:1172.

[benkobalog]

Working on Checking.scala:526

[aesteve]

Working on Applications.scala:95

PR here

[Wojtechnology]

Took Typer.scala:1519

[aesteve]

Working on Checking.scala:555

[nicolasstucki]

Do not make error messages for Phantoms.
Some major changes to the phantoms are coming in #3410. Making error messages for phantoms is useless until we merge that PR as they might completely change

[felixmulder]

@nicolasstucki - are any of the listed available errors phantoms? If so, please mark them as on hold for #3410

[nicolasstucki]

I would like to encourage newcomers to look at #3759 rather than this one.

[felixmulder]

I think people volunteering their free time are going to do what they find to be most fun 😃

[jendrikw]

The line numbers are out of date. To make it easier for others to find the referenced line even if the file changes, it would be a good idea to copy the lines here so others can ctrl-f to find the line.

[mt40]

I could be wrong here but the error in Docstrings.scala:30 seems to be prevented at Comments.scala:127, which already has a new error message implementation.

[hermesespinola]

I'll do Typer.scala:411. I guess it's the one about stable identifiers: "stable identifier required, but ${tree.show} found".

[skvithalani]

I am taking up the normal case $cls in ${cls.owner} cannot extend an enum from Checking.scala.

[sleepiejohn]

Does the backtick ` have any meaning for the interpolator ? As seen at:

 val explanation =
    hl"""|${cdef.name} ${Red("must")} have at least one parameter list, if you would rather
         |have a singleton representation of ${cdef.name}, use a "${"case object"}".
         |Or, add an explicit `()' as a parameter list to ${cdef.name}.""".stripMargin

Does it treat the fragment within as code?

[smarter]

On 20 October 2018 23:20:36 CEST, "João Pedro de Carvalho" [e-mail] wrote:

Does the backtick ` have any meaning for the interpolator ? As seen at:

val explanation =
hl"""|${cdef.name} ${Red("must")} have at least one parameter list, if
you would rather
|have a singleton representation of ${cdef.name}, use a "${"case
object"}".
|Or, add an explicit `()' as a parameter list to
${cdef.name}.""".stripMargin

Does it treat the fragment within as code?

--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
https://github.com/lampepfl/dotty/issues/1589#issuecomment-431618962

No, the backtick doesn't do anything here

[sleepiejohn]

@smarter would be interesting remove the backticks from the messages and exaplanation and replacing where it should be formated as code by the interpolation literal, eg: `()` would become ${"()"} so the interpolator takes care of it? Because I also saw usage of backticks wraping interpolations like:
`${"case e: Exception =>"}`.

At least for me they only add clutter to the output

[smarter]

Sure, that seems like a good idea.

On lundi 29 octobre 2018 19:55:56 CET João Pedro de Carvalho wrote:

@smarter would be interesting remove the backticks from the messages and
exaplanation and replacing where it should be formated as code by the
interpolation literal, eg: `()` would become ${"()"} so the
interpolator takes care of it? Because I also saw usage of backticks
wraping interpolations like: `${"case e: Exception =>"}`.

At least for me they only add clutter to the output

[z123]

I would like to take on PatternMatcher.scala:911

[evgeniiloikov]

Typer.scala:866 should be done here #7385

[harpresing]

Working on RefChecks.scala:119

[x3ro]

Namer.scala:258 is tackled by #7457

[hope-doe]

Hi!
Desugar.scala:457 done there #7729

[hope-doe]

Hi!
Parser:1426, 1428 done there #7848
(and #7729 is fixed also)

[jvdp]

Starting from the top I did Checking.scala:~49~ 53 and I've made a PR for it.

[anatoliykmetyuk]

The architecture for errors described in this issue proved to be heavy and impractical. The migration process frequently interferes with in-flight PRs and the benefits are marginal. We should find a more light-weight way to represent errors.