Skip to content

Show real type names in error messages #4355

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

Open
Blaisorblade opened this issue Apr 20, 2018 · 6 comments
Open

Show real type names in error messages #4355

Blaisorblade opened this issue Apr 20, 2018 · 6 comments
Labels
backlog No work planned on this by the core team for the time being. itype:enhancement prio:low

Comments

@Blaisorblade
Copy link
Contributor

In #4323 I was concerned about writing incorrect names for types in error messages.

Writing Expr' in the error seems a (separate?) bug:

15 |     case (IExpr(i1), IExpr(i2)) => IExpr(i1 + i2)
   |                                    ^^^^^^^^^^^^^^
   |                                    found:    Expr.IExpr
   |                                    required: Expr'[T]
   |                                    
   |                                    where:    Expr  is a object
   |                                              Expr' is a trait

because it's false that "Expr' is a trait". A bit better would be:

   |                                    where:    Expr  refers to object Expr
   |                                              Expr' refers to trait Expr

But even better would be to have a clearer syntax for these "footnotes", something where the annotation is more clearly separate from the Scala syntax. For instance, if we use @ (which is reserved and can't be used in types, I think):

   |                                    where:    Expr@1  refers to object Expr
   |                                              Expr@2 refers to trait Expr
@LPTK
Copy link
Contributor

LPTK commented Apr 20, 2018

I was really confused because of this at first, when reading Dotty error messages. I absolutely concur that error messages should not make up facts such as "Expr' is a trait", a fortiori when Expr' is a valid name in other functional languages and even in some forks of Scalac.

Also, the current where clause is very uninformative – "Expr is an object"... okay, which object exactly?

Instead, why not change the where clause to actual Scala syntax, which would make its meaning much more intuitive? For example, say we have a mismatch between types involving my.module.Expr and other.module.Expr; it could look like:

   |                                    found:    (Expr, Expr0)
   |                                    required: (Expr0, Expr)
   |                                    
   |                                    as interpreted under:
   |                                              import my.module.Expr
   |                                              import other.module.{Expr => Expr0}

Or, which I would personally find better, keep a shortened prefix (only in case of ambiguity obviously):

   |                                    found:    (mm.Expr, om.Expr)
   |                                    required: (om.Expr, mm.Expr)
   |                                    
   |                                    as interpreted under:
   |                                              import my.{module => mm}
   |                                              import other.{module => om}

@smarter
Copy link
Member

smarter commented Apr 20, 2018

I'm not a fan of either solution Expr@1 is almost valid Scala syntax too, and having to read error message "as interpreted under a given set of renaming" is likely to endlessly confuse beginners. My preference would be to use unicode subscripts: Expr1, Expr2, we could also differentiate them using colors.

@LPTK
Copy link
Contributor

LPTK commented Apr 20, 2018

having to read error message "as interpreted under a given set of renaming" is likely to endlessly confuse beginners

Isn't that exactly what is being done now, but with less clarity?

How will users know which types Expr1 and Expr2 are, if you're not going to list the set of renamings?

@smarter
Copy link
Member

smarter commented Apr 20, 2018

We can also have a longer description such as "Expr1 is an object in package foo.bla.baz". I just dislike using import renaming to express this. Note that import renaming is not enough anyway because some types do not have a stable path, e.g.:

class Foo {
  def meth = {
    class Expr
    def expr(e: Expr) = {}

    def bla = {
      class Expr
      expr(new Expr)
    }
  }
}

Here's the error Dotty outputs currently:

8 |      expr(new Expr)
  |               ^^^^
  |               found:    Expr
  |               required: Expr'
  |               
  |               where:    Expr  is a class in method bla
  |                         Expr' is a class in method meth

@Blaisorblade
Copy link
Contributor Author

"Expr1 is an object in package foo.bla.baz". I just dislike using import renaming to express this.

Import renaming also seems unconvincing, but again, we shouldn't write that Expr1 isn't an object, because that object is called Expr. I'd say that Expr1 isn't an object.

Expr@1 is almost valid Scala syntax too, and having to read error message "as interpreted under a given set of renaming" is likely to endlessly confuse beginners. My preference would be to use unicode subscripts: Expr1, Expr2, we could also differentiate them using colors.

I agree with Unicode subscripts are better if we can rely on Unicode, since they're not allowed in Scala.

Which we probably often can. Otherwise, Expr@1 was the best idea I had within ASCII after skimming https://stackoverflow.com/a/7657692, but I'm not sure it's much better.

@odersky odersky added the backlog No work planned on this by the core team for the time being. label Apr 5, 2022
@ckipp01
Copy link
Member

ckipp01 commented May 11, 2023

Reading through this, it's a bit hard to actually pinpoint what we're trying to fix compared to where we're at now. In the linked issue some of the code samples that were marked failing now work, and the output is a bit different as well. Is there a minimal code sample that can be provided here to show what exactly it is we're trying to fix/enhance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog No work planned on this by the core team for the time being. itype:enhancement prio:low
Projects
None yet
Development

No branches or pull requests

6 participants