gleam-lang / gleam

⭐️ A friendly language for building type-safe, scalable systems!
https://gleam.run
Apache License 2.0
17.42k stars 723 forks source link

Wrong Assignment Error Message on Bad Names #3204

Closed Arian94 closed 2 months ago

Arian94 commented 3 months ago

When defining a new Variable starting with a capital letter, it throws a wrong error message as in the picture. If not fixed yet, I have a possible solution by defining a new TypeError and implement its message in to_diagnostics(). Please, inform me if my solution is proper.

Screenshot 2024-05-27 143019

lpil commented 3 months ago

Oh dear! That is rubbish! Thank you

Arian94 commented 3 months ago

Was my solution implementable? You could assign it to me if you want to.

On Mon, May 27, 2024, 17:00 Louis Pilfold @.***> wrote:

Oh dear! That is rubbish! Thank you

— Reply to this email directly, view it on GitHub https://github.com/gleam-lang/gleam/issues/3204#issuecomment-2133382443, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQGAO3MQAW2HF46KQ2ZVH63ZEMRMXAVCNFSM6AAAAABIK7KVRSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZTGM4DENBUGM . You are receiving this because you authored the thread.Message ID: @.***>

lpil commented 3 months ago

Ah! Now that I'm looking at this again I realise what is happening here. An uppercase letter means this is a pattern consisting of a single constructor. Let's make the error say something like this:

This pattern is matching a constructor named One, but there is no constructor in scope with that name.

Arian94 commented 3 months ago

If the error changes to what you mentioned, I believe there are some other cases where the error will be wrong. The UnknownVariable enum is used in multiple places and changing its error message would cause other places to change unintentionally.

lpil commented 3 months ago

Could you give an example of when that would be?

Arian94 commented 3 months ago
import gleam/io

pub fn main() {
  let one = 23
  let two = 4

  io.debug(one == three)
}

The error would be:

The name `three` is not in scope here.
lpil commented 3 months ago

Yes, that's right. That seems unrelated to this issue though?

Arian94 commented 3 months ago

Yes. That is why I declared another enum in my PR at #3207. Have you looked at it? May I know your thoughts on it?

lpil commented 3 months ago

It doesn't implement the behaviour I've outlined above, could you update it to do that please. Thank you

Arian94 commented 3 months ago

I'm a bit unsure.

You mean it is only needed to update the Error::UnknownVariable error message to what you gave? Or define a new Enum like I did and write the error message you mentioned?

lpil commented 3 months ago

I'm not saying anything specific about the implementation, I've not looked into how this would be implemented. We do need an error like the one above and only in the specific situation the first post of this issue shows.

Arian94 commented 3 months ago

3207 should work accordingly.