No-overload-matches error messages are unhelpful

[BrucePerens]

This is a common sort of error message I get when creating a model instance using the Avram ORM. It's extremely difficult to isolate what is actually wrong here. It would be much better if the compiler would tell, for each potential overload match, which arguments matched and which did not.

Error: no overload matches 'SavePerson.create!', company: String, company_id: Int64, location_id: Int64, full_name: String, title: (String | Nil), pronouns: Array(String), email: (String | Nil), work_phone: (String | Nil), role: Person::Role

Overloads are:
 - SavePerson.create!(params, id : Int64 | Avram::Nothing = Avram::Nothing.new, created_at : Time | Avram::Nothing = Avram::Nothing.new, updated_at : Time | Avram::Nothing = Avram::Nothing.new, company_id : Company::PrimaryKeyType | Avram::Nothing = Avram::Nothing.new, location_id : Location::PrimaryKeyType | Avram::Nothing | Nil = Avram::Nothing.new, pre_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, first_name : String | Avram::Nothing | Nil = Avram::Nothing.new, middle_name : String | Avram::Nothing | Nil = Avram::Nothing.new, last_name : String | Avram::Nothing | Nil = Avram::Nothing.new, full_name : String | Avram::Nothing = Avram::Nothing.new, post_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, pronouns : Array(String) | Avram::Nothing = Avram::Nothing.new, title : String | Avram::Nothing | Nil = Avram::Nothing.new, email : String | Avram::Nothing | Nil = Avram::Nothing.new, work_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, mobile_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, fax : String | Avram::Nothing | Nil = Avram::Nothing.new, role : Int32 | Avram::Nothing = Avram::Nothing.new)
 - SavePerson.create!(id : Int64 | Avram::Nothing = Avram::Nothing.new, created_at : Time | Avram::Nothing = Avram::Nothing.new, updated_at : Time | Avram::Nothing = Avram::Nothing.new, company_id : Company::PrimaryKeyType | Avram::Nothing = Avram::Nothing.new, location_id : Location::PrimaryKeyType | Avram::Nothing | Nil = Avram::Nothing.new, pre_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, first_name : String | Avram::Nothing | Nil = Avram::Nothing.new, middle_name : String | Avram::Nothing | Nil = Avram::Nothing.new, last_name : String | Avram::Nothing | Nil = Avram::Nothing.new, full_name : String | Avram::Nothing = Avram::Nothing.new, post_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, pronouns : Array(String) | Avram::Nothing = Avram::Nothing.new, title : String | Avram::Nothing | Nil = Avram::Nothing.new, email : String | Avram::Nothing | Nil = Avram::Nothing.new, work_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, mobile_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, fax : String | Avram::Nothing | Nil = Avram::Nothing.new, role : Int32 | Avram::Nothing = Avram::Nothing.new)
 - SavePerson.create!(params : Hash, **named_args)
 - Person::SaveOperation.create!(params, id : Int64 | Avram::Nothing = Avram::Nothing.new, created_at : Time | Avram::Nothing = Avram::Nothing.new, updated_at : Time | Avram::Nothing = Avram::Nothing.new, company_id : Company::PrimaryKeyType | Avram::Nothing = Avram::Nothing.new, location_id : Location::PrimaryKeyType | Avram::Nothing | Nil = Avram::Nothing.new, pre_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, first_name : String | Avram::Nothing | Nil = Avram::Nothing.new, middle_name : String | Avram::Nothing | Nil = Avram::Nothing.new, last_name : String | Avram::Nothing | Nil = Avram::Nothing.new, full_name : String | Avram::Nothing = Avram::Nothing.new, post_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, pronouns : Array(String) | Avram::Nothing = Avram::Nothing.new, title : String | Avram::Nothing | Nil = Avram::Nothing.new, email : String | Avram::Nothing | Nil = Avram::Nothing.new, work_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, mobile_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, fax : String | Avram::Nothing | Nil = Avram::Nothing.new, role : Int32 | Avram::Nothing = Avram::Nothing.new)
 - Person::SaveOperation.create!(id : Int64 | Avram::Nothing = Avram::Nothing.new, created_at : Time | Avram::Nothing = Avram::Nothing.new, updated_at : Time | Avram::Nothing = Avram::Nothing.new, company_id : Company::PrimaryKeyType | Avram::Nothing = Avram::Nothing.new, location_id : Location::PrimaryKeyType | Avram::Nothing | Nil = Avram::Nothing.new, pre_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, first_name : String | Avram::Nothing | Nil = Avram::Nothing.new, middle_name : String | Avram::Nothing | Nil = Avram::Nothing.new, last_name : String | Avram::Nothing | Nil = Avram::Nothing.new, full_name : String | Avram::Nothing = Avram::Nothing.new, post_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, pronouns : Array(String) | Avram::Nothing = Avram::Nothing.new, title : String | Avram::Nothing | Nil = Avram::Nothing.new, email : String | Avram::Nothing | Nil = Avram::Nothing.new, work_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, mobile_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, fax : String | Avram::Nothing | Nil = Avram::Nothing.new, role : Int32 | Avram::Nothing = Avram::Nothing.new)
 - Person::SaveOperation.create!(params : Hash, **named_args)

https://github.com/luckyframework/avram

[BrucePerens]

I think it would be helpful if the error report looked like this:
Error: no overload matches 'SavePerson.create!', company: String, company_id: Int64, location_id: Int64, full_name: String, title: (String | Nil), pronouns: Array(String), email: (String | Nil), work_phone: (String | Nil), role: Person::Role

Overloads are:

• SavePerson.create!(params, id : Int64 | Avram::Nothing = Avram::Nothing.new, created_at : Time | Avram::Nothing = Avram::Nothing.new, updated_at : Time | Avram::Nothing = Avram::Nothing.new, company_id : Company::PrimaryKeyType | Avram::Nothing = Avram::Nothing.new, location_id : Location::PrimaryKeyType | Avram::Nothing | Nil = Avram::Nothing.new, pre_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, first_name : String | Avram::Nothing | Nil = Avram::Nothing.new, middle_name : String | Avram::Nothing | Nil = Avram::Nothing.new, last_name : String | Avram::Nothing | Nil = Avram::Nothing.new, full_name : String | Avram::Nothing = Avram::Nothing.new, post_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, pronouns : Array(String) | Avram::Nothing = Avram::Nothing.new, title : String | Avram::Nothing | Nil = Avram::Nothing.new, email : String | Avram::Nothing | Nil = Avram::Nothing.new, work_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, mobile_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, fax : String | Avram::Nothing | Nil = Avram::Nothing.new, role : Int32 | Avram::Nothing = Avram::Nothing.new)
• Matched: company_id, location_id, ...*
• Did not match: full_name
• Required, but not provided: pronouns

[docelic]

Maybe another (additional?) option could be to colorize the matched or mismatched args, and to sort the list so that the closest matches can be seen directly, without having to scroll through the less relevant ones.

[BrucePerens]

As it happens, I'm red-green colorblind, and this is true for around 8% of European-descended males. I can still see Crystal colorization for the most part, but differentiating red and green text is often difficult. In the interest of such people, and the more visually handicapped, it would be nice if color would be used to highlight, but would never be the sole means of receiving information.

[straight-shoota]

This is a duplicate of #8179 but I'm keeping this newer issue open becaus it's more substantial already.

[straight-shoota]

In a terminal, maybe we could use other text highlighting than color. For examole, all unmatched arguments and parameters (thise are the ones you care about) could be underlined.

[willhbr]

w.r.t red/green output in terminals - obviously colour shouldn't be the only way of showing information, but in case anyone wasn't aware your terminal emulator almost certainly lets you go in and re-map what colour "red" and "green" display as (even to completely different colours!). So you may be able to make things a little bit easier to differentiate (when viewing git diffs, etc).

[zw963]

When column type/name was not correct in avram, the output compilation error is hard to read. e.g.

The output content like this:

UpdateUser.new(record : T, id : Int64 | Avram::Nothing = Avram::Nothing.new, created_at : Time | Avram::Nothing = Avram::Nothing.new,updated_at : Time | Avram::Nothing = Avram::Nothing.new,open_id : String | Avram::Nothing = Avram::Nothing.new,account : String | Avram::Nothing = Avram::Nothing.new)

If we can output instead of like this (add newline after comma), readability more easily.

- UpdateUser.new(
record : T,
id : Int64 | Avram::Nothing = Avram::Nothing.new,
created_at : Time | Avram::Nothing = Avram::Nothing.new,
updated_at : Time | Avram::Nothing = Avram::Nothing.new,
open_id : String | Avram::Nothing = Avram::Nothing.new,
account : String | Avram::Nothing = Avram::Nothing.new,
)

[BrucePerens]

@zw963 formatting would be helpful, but I think it's essential that any error report tell you what's wrong directly, not obliquely, if that is at all possible. So, my problem with it saying "I can't match to this long list of arguments" is that it is oblique, and requires the programmer to perform the final step of matching a failing method call to a method prototype. This is laborious in the case of Avram, because the argument list is every field of an arbitrarily long record.

So, I reiterate my candidate for a direct, not oblique, error report.

Matched: company_id, location_id, ...*
Did not match: full_name
Required, but not provided: pronouns

This should not require any additional information, like a color key, to interpret.

There is also the issue of the steep Crystal learning curve. The learning curve isn't about the Crystal language. It's about how to connect an error message to the location of the actual error through the many layers of code that are reported.

Thanks

Bruce

[asterite]

I think the "Matched", "Did not match", "Required but not provided" might be a bit too verbose. Maybe it's just enough to show one error per overload, because if no overload matched then there's always an error in each of them.

For the initial example, I'm thinking about an output like this:

company: String, company_id: Int64, location_id: Int64, full_name: String, title: (String | Nil), pronouns: Array(String), email: (String | Nil), work_phone: (String | Nil), role: Person::Role

Overloads are:
 - SavePerson.create!(params, id : Int64 | Avram::Nothing = Avram::Nothing.new, created_at : Time | Avram::Nothing = Avram::Nothing.new, updated_at : Time | Avram::Nothing = Avram::Nothing.new, company_id : Company::PrimaryKeyType | Avram::Nothing = Avram::Nothing.new, location_id : Location::PrimaryKeyType | Avram::Nothing | Nil = Avram::Nothing.new, pre_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, first_name : String | Avram::Nothing | Nil = Avram::Nothing.new, middle_name : String | Avram::Nothing | Nil = Avram::Nothing.new, last_name : String | Avram::Nothing | Nil = Avram::Nothing.new, full_name : String | Avram::Nothing = Avram::Nothing.new, post_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, pronouns : Array(String) | Avram::Nothing = Avram::Nothing.new, title : String | Avram::Nothing | Nil = Avram::Nothing.new, email : String | Avram::Nothing | Nil = Avram::Nothing.new, work_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, mobile_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, fax : String | Avram::Nothing | Nil = Avram::Nothing.new, role : Int32 | Avram::Nothing = Avram::Nothing.new)
   # Argument 'params' not given
 - SavePerson.create!(id : Int64 | Avram::Nothing = Avram::Nothing.new, created_at : Time | Avram::Nothing = Avram::Nothing.new, updated_at : Time | Avram::Nothing = Avram::Nothing.new, company_id : Company::PrimaryKeyType | Avram::Nothing = Avram::Nothing.new, location_id : Location::PrimaryKeyType | Avram::Nothing | Nil = Avram::Nothing.new, pre_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, first_name : String | Avram::Nothing | Nil = Avram::Nothing.new, middle_name : String | Avram::Nothing | Nil = Avram::Nothing.new, last_name : String | Avram::Nothing | Nil = Avram::Nothing.new, full_name : String | Avram::Nothing = Avram::Nothing.new, post_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, pronouns : Array(String) | Avram::Nothing = Avram::Nothing.new, title : String | Avram::Nothing | Nil = Avram::Nothing.new, email : String | Avram::Nothing | Nil = Avram::Nothing.new, work_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, mobile_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, fax : String | Avram::Nothing | Nil = Avram::Nothing.new, role : Int32 | Avram::Nothing = Avram::Nothing.new)
   # Given argument 'company' doesn't exist
 - SavePerson.create!(params : Hash, **named_args)
   # Argument 'params' not given
 - Person::SaveOperation.create!(params, id : Int64 | Avram::Nothing = Avram::Nothing.new, created_at : Time | Avram::Nothing = Avram::Nothing.new, updated_at : Time | Avram::Nothing = Avram::Nothing.new, company_id : Company::PrimaryKeyType | Avram::Nothing = Avram::Nothing.new, location_id : Location::PrimaryKeyType | Avram::Nothing | Nil = Avram::Nothing.new, pre_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, first_name : String | Avram::Nothing | Nil = Avram::Nothing.new, middle_name : String | Avram::Nothing | Nil = Avram::Nothing.new, last_name : String | Avram::Nothing | Nil = Avram::Nothing.new, full_name : String | Avram::Nothing = Avram::Nothing.new, post_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, pronouns : Array(String) | Avram::Nothing = Avram::Nothing.new, title : String | Avram::Nothing | Nil = Avram::Nothing.new, email : String | Avram::Nothing | Nil = Avram::Nothing.new, work_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, mobile_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, fax : String | Avram::Nothing | Nil = Avram::Nothing.new, role : Int32 | Avram::Nothing = Avram::Nothing.new)
   # Argument 'params' not given
 - Person::SaveOperation.create!(id : Int64 | Avram::Nothing = Avram::Nothing.new, created_at : Time | Avram::Nothing = Avram::Nothing.new, updated_at : Time | Avram::Nothing = Avram::Nothing.new, company_id : Company::PrimaryKeyType | Avram::Nothing = Avram::Nothing.new, location_id : Location::PrimaryKeyType | Avram::Nothing | Nil = Avram::Nothing.new, pre_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, first_name : String | Avram::Nothing | Nil = Avram::Nothing.new, middle_name : String | Avram::Nothing | Nil = Avram::Nothing.new, last_name : String | Avram::Nothing | Nil = Avram::Nothing.new, full_name : String | Avram::Nothing = Avram::Nothing.new, post_nominal : String | Avram::Nothing | Nil = Avram::Nothing.new, pronouns : Array(String) | Avram::Nothing = Avram::Nothing.new, title : String | Avram::Nothing | Nil = Avram::Nothing.new, email : String | Avram::Nothing | Nil = Avram::Nothing.new, work_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, mobile_phone : String | Avram::Nothing | Nil = Avram::Nothing.new, fax : String | Avram::Nothing | Nil = Avram::Nothing.new, role : Int32 | Avram::Nothing = Avram::Nothing.new)
   # Given argument 'company' doesn't exist
 - Person::SaveOperation.create!(params : Hash, **named_args)
   # Given argument 'company' doesn't exist

The messages might not be exactly those, but that's the general idea.

What do you think?

Other such messages, for each overload, might be:

• Not enough arguments
• Too many arguments
• Expected company_id to be X but Y was given

Then if an error happens in all of the overloads... say for example all overloads have a name : String argument and you pass name: 1, then the main error message could be "Expected name to be String but Int32 was given". All the overloads will be listed, but no message will show up for each overload, because it's the same message for all of them.

[zw963]

If the compile can capture the programmer intention, that is okay.

the issue is, afraid compiler give info which guide the programmer to the wrong indirection.

anyway, from my point of view, current Overloads output is okay, it just need pretty print.

[asterite]

@zw963 could you provide an example of how would that look with pretty print?

[BrucePerens]

@asterite why not put the "Argument X doesn't exist" and other such messages above the method prototype instead of below it?
There is often only one method prototype, and this puts the important information first. There is an awful lot of information to pick through, and it may be that for most cases the programmer will not have to look at it at all.

@zw963 wished to break at commas, his example is here: #11106 (comment)
I like that, because long argument lists are otherwise even more difficult to parse. But it's not sufficient by itself. And it does mean that some error reports will be tens of lines long. But I am happy to trade more lines for better readability.

Finallly, I wish there was some way to hide Avram::Nothing = Avram::Nothing.new or replace it with something simpler that indicates that the argument is optional.

[asterite]

There is often one method prototype, yet the example you provided has like five overloads. So I think we should consider that case if you want your specific example to improve.

[BrucePerens]

Yes, Avram is the worst case we have so far. But the example I gave repeats every prototype twice as both SavePerson.create! and Person::SaveOperation.create!. And I could not tell you why. The first version is what I called, perhaps the second is a macro expansion. As far as I can tell there are three overloads of the method called.

[zw963]

@zw963 could you provide an example of how would that look with pretty print?

Okay, a more robust way is, when union type is more longer, not easy to read, print all overload union type's argument on separate line.

e.g. following error message is really hard to read, and find the column out. (see screenshot)

We can add a newline after the comma, like this.

Overloads are: 
- UpdateUser.new(record : T,
	 id : Int64 | Avram::Nothing = Avram::Nothing.new,
	 created_at : Time | Avram::Nothing = Avram::Nothing.new,
	 updated_at : Time | Avram::Nothing = Avram::Nothing.new,
	 open_id : String | Avram::Nothing = Avram::Nothing.new,
	 account : String | Avram::Nothing = Avram::Nothing.new,
         password : String | Avram::Nothing = Avram::Nothing.new,
         password_digest : String | Avram::Nothing = Avram::Nothing.new,
         licenses_allowed_creation_times : Int32 | Avram::Nothing = Avram::Nothing.new,
  )
- UpdateUser.new(record : T,
	 id : Int64 | Avram::Nothing = Avram::Nothing.new,
	 created_at : Time | Avram::Nothing = Avram::Nothing.new,
	 updated_at : Time | Avram::Nothing = Avram::Nothing.new,
	 open_id : String | Avram::Nothing = Avram::Nothing.new,
	 account : String | Avram::Nothing = Avram::Nothing.new,
         password : String | Avram::Nothing = Avram::Nothing.new,
         password_digest : String | Avram::Nothing = Avram::Nothing.new,
         licenses_allowed_creation_times : Int32 | Avram::Nothing = Avram::Nothing.new,
  )  

As so, we can find the columns as describe in error message (in this case, is password, password_digest, licenses_allowed_creation_times), and thought on why this error happen.

We don't make decisions what wrong here, we just pretty print all overloads here, let use to figure it out. (more easiler)

[zw963]

@BrucePerens , i prefer to only add newline instead give exact error message like "Matched", "Did not match", "Required but not provided", because i consider that not so useful.

1. the union type is the foot stone or Crystal, with type_inference + Multiple dispatch, it make Crystal a very special existance, make it unque. anyway, the user should figure it out what those overloads means. it not like any of others language, give you a specific error message is enough, if that is true, if we even don't need print those overloads out, only "Matched", "Did not match", "Required but not provided"? obviously, this is not acceptable.

2. if use "Matched", "Did not match", "Required but not provided" error instead, for the newcomer, there probably exists more than one error message coexist because newbie write wrong code, always, that not really help for figure it out.

[zw963]

Finallly, I wish there was some way to hide Avram::Nothing = Avram::Nothing.new or replace it with something simpler that indicates that the argument is optional.

I consider this is not a good idea, that exactly avram want us to know.

[BrucePerens]

Well, Billy, you are obviously an achieved computer scientist. And you comprehend these complexities and even relish them. I would ask you to have some sympathy for the newbie. For them, the error reports of crystal are mainly incomprehensible. And that has driven a tremendous gap between Ruby and Crystal that has thrown a barrier before Crystal adoption. Although the two languages appear very similar, the difference becomes evident when one confronts a compiler error report. Ones like this are simply incomprehensible to the newbie and require a significant learning curve. Indeed, I have some achievement in this world and still find many of them challenging. So I really strongly request that we always have the newbie in our minds when we code error handling.