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

Missing error messages #1589

Open
felixmulder opened this Issue Oct 13, 2016 · 107 comments

Comments

Projects
None yet
@felixmulder
Contributor

felixmulder commented Oct 13, 2016

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 😃

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

@markusthoemmes

This comment has been minimized.

Show comment
Hide comment
@markusthoemmes

markusthoemmes Oct 14, 2016

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 commented Oct 14, 2016

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

This comment has been minimized.

Show comment
Hide comment
@markusthoemmes

markusthoemmes Oct 15, 2016

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

markusthoemmes commented Oct 15, 2016

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

This comment has been minimized.

Show comment
Hide comment
@felixmulder

felixmulder Oct 15, 2016

Contributor

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

Contributor

felixmulder commented Oct 15, 2016

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

@anicolaspp

This comment has been minimized.

Show comment
Hide comment
@anicolaspp

anicolaspp commented Oct 15, 2016

👍

@sebastianharko

This comment has been minimized.

Show comment
Hide comment
@sebastianharko

sebastianharko Oct 15, 2016

Contributor

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

Contributor

sebastianharko commented Oct 15, 2016

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

@ShaneDelmore

This comment has been minimized.

Show comment
Hide comment
@ShaneDelmore

ShaneDelmore Oct 16, 2016

Contributor

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

Contributor

ShaneDelmore commented Oct 16, 2016

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

@sebastianharko

This comment has been minimized.

Show comment
Hide comment
@sebastianharko

sebastianharko Oct 16, 2016

Contributor

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

Contributor

sebastianharko commented Oct 16, 2016

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

@ljdelight

This comment has been minimized.

Show comment
Hide comment
@ljdelight

ljdelight Oct 16, 2016

Contributor

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

Contributor

ljdelight commented Oct 16, 2016

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

@sebastianharko

This comment has been minimized.

Show comment
Hide comment
@sebastianharko

sebastianharko Oct 16, 2016

Contributor

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

Contributor

sebastianharko commented Oct 16, 2016

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

@AndrewZurn

This comment has been minimized.

Show comment
Hide comment
@AndrewZurn

AndrewZurn Oct 17, 2016

Contributor

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

Contributor

AndrewZurn commented Oct 17, 2016

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

This comment has been minimized.

Show comment
Hide comment
@jyotman

jyotman Oct 18, 2016

Hi, Working on Parsers.scala:1180.

jyotman commented Oct 18, 2016

Hi, Working on Parsers.scala:1180.

@felixmulder

This comment has been minimized.

Show comment
Hide comment
@felixmulder

felixmulder Oct 18, 2016

Contributor

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

Contributor

felixmulder commented Oct 18, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@sebastianharko

sebastianharko Oct 22, 2016

Contributor

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

Contributor

sebastianharko commented Oct 22, 2016

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

@felixmulder

This comment has been minimized.

Show comment
Hide comment
@felixmulder

felixmulder Oct 23, 2016

Contributor

@sebastianharko - yes done! Thanks

Contributor

felixmulder commented Oct 23, 2016

@sebastianharko - yes done! Thanks

@iamthiago

This comment has been minimized.

Show comment
Hide comment
@iamthiago

iamthiago Oct 23, 2016

Contributor

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?

Contributor

iamthiago commented Oct 23, 2016

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

This comment has been minimized.

Show comment
Hide comment
@felixmulder

felixmulder Oct 23, 2016

Contributor

@thiagoandrade6 - sounds great :)

Contributor

felixmulder commented Oct 23, 2016

@thiagoandrade6 - sounds great :)

iamthiago pushed a commit to iamthiago/dotty that referenced this issue Oct 23, 2016

Thiago Pereira Thiago Pereira
Add error message - Comments.scala:128
This commit adds the semantic object for the ```definition not found``` error.
It is part of the (lampepfl#1589

iamthiago added a commit to iamthiago/dotty that referenced this issue Oct 23, 2016

Add error message - Comments.scala:128
This commit adds the semantic object for the ```definition not found``` error.
It is part of the (lampepfl#1589

iamthiago added a commit to iamthiago/dotty that referenced this issue Oct 24, 2016

Add error message - Comments.scala:128
This commit adds the semantic object for the ```definition not found``` error.
It is part of the (lampepfl#1589

iamthiago added a commit to iamthiago/dotty that referenced this issue Oct 24, 2016

Add error message - Comments.scala:128
This commit adds the semantic object for the ```definition not found``` error.
It is part of the (lampepfl#1589

iamthiago added a commit to iamthiago/dotty that referenced this issue Oct 25, 2016

Add error message - Comments.scala:128
This commit adds the semantic object for the ```definition not found``` error.
It is part of the (lampepfl#1589

iamthiago added a commit to iamthiago/dotty that referenced this issue Oct 25, 2016

@ALPHA-60

This comment has been minimized.

Show comment
Hide comment
@ALPHA-60

ALPHA-60 Oct 25, 2016

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

ALPHA-60 commented Oct 25, 2016

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

iamthiago added a commit to iamthiago/dotty that referenced this issue Oct 25, 2016

Add error message - Comments.scala:128
This commit adds the semantic object for the ```definition not found``` error.
It is part of the (lampepfl#1589
@AndrewZurn

This comment has been minimized.

Show comment
Hide comment
@AndrewZurn

AndrewZurn Oct 25, 2016

Contributor

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

Contributor

AndrewZurn commented Oct 25, 2016

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

felixmulder added a commit that referenced this issue Oct 25, 2016

Merge pull request #1623 from ShaneDelmore/1589_Missing_error_messages
#1589: Improve error message for WrongNumberOfArgs
@m-sp

This comment has been minimized.

Show comment
Hide comment
@m-sp

m-sp Oct 26, 2016

Contributor

working on Parsers.scala:1620

Contributor

m-sp commented Oct 26, 2016

working on Parsers.scala:1620

iamthiago added a commit to iamthiago/dotty that referenced this issue Oct 26, 2016

iamthiago added a commit to iamthiago/dotty that referenced this issue Oct 26, 2016

iamthiago added a commit to iamthiago/dotty that referenced this issue Oct 26, 2016

iamthiago added a commit to iamthiago/dotty that referenced this issue Oct 26, 2016

Add error message IdentifierExpected
This commit adds the semantic object fir the ```identifier expected``` error.
It is part of the lampepfl#1589

nerush added a commit to nerush/dotty that referenced this issue Nov 28, 2017

nerush added a commit to nerush/dotty that referenced this issue Nov 28, 2017

vitorsvieira added a commit to vitorsvieira/dotty that referenced this issue Nov 28, 2017

vitorsvieira added a commit to vitorsvieira/dotty that referenced this issue Nov 28, 2017

vitorsvieira added a commit to vitorsvieira/dotty that referenced this issue Nov 28, 2017

vitorsvieira added a commit to vitorsvieira/dotty that referenced this issue Nov 28, 2017

vitorsvieira added a commit to vitorsvieira/dotty that referenced this issue Nov 28, 2017

@vitorsvieira

This comment has been minimized.

Show comment
Hide comment
@vitorsvieira

vitorsvieira Nov 28, 2017

Contributor

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
Contributor

vitorsvieira commented Nov 28, 2017

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

This comment has been minimized.

Show comment
Hide comment
@gosubpl

gosubpl Nov 29, 2017

Contributor

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

Contributor

gosubpl commented Nov 29, 2017

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

@Wojtechnology

This comment has been minimized.

Show comment
Hide comment
@Wojtechnology

Wojtechnology Jan 3, 2018

Contributor

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

Contributor

Wojtechnology commented Jan 3, 2018

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

@benkobalog

This comment has been minimized.

Show comment
Hide comment
@benkobalog

benkobalog Jan 4, 2018

Contributor

Working on Checking.scala:526

Contributor

benkobalog commented Jan 4, 2018

Working on Checking.scala:526

aesteve added a commit to aesteve/dotty that referenced this issue Jan 8, 2018

@aesteve

This comment has been minimized.

Show comment
Hide comment

aesteve commented Jan 8, 2018

Working on Applications.scala:95

PR here

@Wojtechnology

This comment has been minimized.

Show comment
Hide comment
@Wojtechnology

Wojtechnology Jan 9, 2018

Contributor

Took Typer.scala:1519

Contributor

Wojtechnology commented Jan 9, 2018

Took Typer.scala:1519

aesteve added a commit to aesteve/dotty that referenced this issue Jan 9, 2018

aesteve added a commit to aesteve/dotty that referenced this issue Jan 9, 2018

@aesteve

This comment has been minimized.

Show comment
Hide comment
@aesteve

aesteve commented Jan 9, 2018

Working on Checking.scala:555

@nicolasstucki

This comment has been minimized.

Show comment
Hide comment
@nicolasstucki

nicolasstucki Jan 9, 2018

Contributor

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

Contributor

nicolasstucki commented Jan 9, 2018

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

This comment has been minimized.

Show comment
Hide comment
@felixmulder

felixmulder Jan 9, 2018

Contributor

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

Contributor

felixmulder commented Jan 9, 2018

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

@nicolasstucki

This comment has been minimized.

Show comment
Hide comment
@nicolasstucki

nicolasstucki Jan 9, 2018

Contributor

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

Contributor

nicolasstucki commented Jan 9, 2018

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

@felixmulder

This comment has been minimized.

Show comment
Hide comment
@felixmulder

felixmulder Jan 9, 2018

Contributor

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

Contributor

felixmulder commented Jan 9, 2018

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

benkobalog added a commit to benkobalog/dotty that referenced this issue Jan 24, 2018

benkobalog added a commit to benkobalog/dotty that referenced this issue Jan 24, 2018

benkobalog added a commit to benkobalog/dotty that referenced this issue Jan 28, 2018

allanrenucci added a commit to benkobalog/dotty that referenced this issue Jan 29, 2018

allanrenucci added a commit that referenced this issue Jan 29, 2018

Merge pull request #3908 from benkobalog/ErrorMessage-SymbolIsNotAValue
Added error message for Symbol is not a value #1589

jendrikw pushed a commit to jendrikw/dotty that referenced this issue Mar 2, 2018

@jendrikw

This comment has been minimized.

Show comment
Hide comment
@jendrikw

jendrikw Mar 3, 2018

Contributor

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.

Contributor

jendrikw commented Mar 3, 2018

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.

allanrenucci added a commit that referenced this issue Mar 19, 2018

Merge pull request #4064 from jendrikw/error-double-definition
Add error message about double definition (#1589)
@mt40

This comment has been minimized.

Show comment
Hide comment
@mt40

mt40 May 18, 2018

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.

mt40 commented May 18, 2018

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

This comment has been minimized.

Show comment
Hide comment
@hermesespinola

hermesespinola Jul 2, 2018

Contributor

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

Contributor

hermesespinola commented Jul 2, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment