literal icon indicating copy to clipboard operation
literal copied to clipboard

Add more context to error messages

Open joeldrapper opened this issue 2 years ago • 7 comments

We can usually add some additional context such as the name of the attribute. Additionally, collection types could implement an alternative interface that allows them to highlight specifically which member failed and why.

joeldrapper avatar Jul 06 '23 10:07 joeldrapper

I’m struggling to find a nice way to present all the information. When we have a type failure, we’ll have three things:

  1. The type it was expected to be — this is usually inspectable, it’ll look like _Array(Integer) or something like that.
  2. The context (new) — this is something we can provide when asking Literal to check a type. It could be something like Class#method:property, but if that property is enumerated as part of the check (e.g. for an array or hash), it might have further context like Class#method:property[key][0]
  3. The value that was given that didn’t match the expected type. This is sometimes inspectable but often not really very inspectable. It could be like #<Foo:0x0000000100fe3660>......... When presenting the value, it might make sense to try to get its class (if it’s an object) and mention that first, e.g.
Received a String: `"actual string"`.
  1. The final bit of information is where the error occurred. We should be able to highlight the exact line number where the method was called and given the wrong type of argument.

joeldrapper avatar Jun 24 '24 13:06 joeldrapper

Just wanted to say that (2) would be a great addition.

Given that our codebase is made up of almost entirely Literal::Attributes and Literal::Datas, a specific type violation can be hard to track down.

Presentation-wise, I think RSpec output does a decent job of presenting an expected vs actual value, and I see this as somewhat analogous.

Looking forward to 1.0! We're still pinned to 455f225574dbcb39b7e325a9f75c928a79ec812b and are eager to upgrade/simplify.

hangsu avatar Jun 24 '24 17:06 hangsu

Looking forward to 1.0! We're still pinned to 455f225574dbcb39b7e325a9f75c928a79ec812b and are eager to upgrade/simplify.

We’re really close now.

joeldrapper avatar Jun 24 '24 18:06 joeldrapper

Here’s where I’m at so far.

CleanShot 2024-06-25 at 19 02 47@2x

joeldrapper avatar Jun 25 '24 18:06 joeldrapper

Maybe:

Person#initialize
  Type mismatch for prop `name`
    Expected: `String`
    Actual: `Integer(1)`

Here's Sorbet's type error: https://sorbet.org/docs/error-reference#7002

Edit: prop instead of argument

hangsu avatar Jun 25 '24 18:06 hangsu

Made some progress on this in #98, but we can go further. Specifically, I would like to make collection types recursively add further context, so you know which specific key a check failed on.

joeldrapper avatar Jul 05 '24 10:07 joeldrapper

hey @joeldrapper. Is there anything I can do to help? Otherwise, do you know if that's the last thing that will change until literal 1.0? In other words is it valid to use main right now?

Vagab avatar Aug 15 '24 18:08 Vagab

I don’t expect to ship any significant changes to main. I think this is the last bit.

joeldrapper avatar Aug 27 '24 18:08 joeldrapper

ok thank you! Is there anything I can do to help with this issue btw?

Vagab avatar Aug 27 '24 18:08 Vagab