Skip to content
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

Confusing incorrect arity hint when labelled arguments are provided without labels #3884

Open
joshi-monster opened this issue Nov 24, 2024 · 7 comments
Labels
bug Something isn't working help wanted Contributions encouraged priority:medium

Comments

@joshi-monster
Copy link
Contributor

fn wibble(a a: Int, b b: Float, c c: String) {
  todo
}

pub fn wobble() {
  wibble(1, 2.0)
}

produces

error: Incorrect arity
  ┌─ /home/arkan/Projects/oss/gleam/probe/src/probe.gleam:6:3
  │
6 │   wibble(1, 2)
  │   ^^^^^^^^^^^^ Expected 3 arguments, got 2

This call accepts these additional labelled arguments:

  - a
  - b
  - c

I think the hint ("this call accepts these additional labelled arguments: ...") is confusing here, because I already provide values for a and b, but using positional arguments instead. While it is technically correct that I could provide any of the arguments as a labelled argument, changing the error from an incorrect arity to a type mismatch in the process, I don't think it's very useful to tell me that I could change the meaning of the arguments I already pass.

I chose different types to make it more obvious, but in my opinion all arguments that I already provide should not be included in the hint, even if the types were compatible. This is probably even more subjective. ~ 💜

@joshi-monster joshi-monster added the bug Something isn't working label Nov 24, 2024
@GearsDatapacks
Copy link
Member

What should the hint say in this case? Just suggest the c label?

@joshi-monster
Copy link
Contributor Author

I personally would want it to only suggest c since I don't ever mix positioned and labelled arguments in a way that would change their order, (e.g. only passing the second argument of a ternary function as a labelled argument), but I'm not sure if that's the best/indented experience for everyone.

@lpil
Copy link
Member

lpil commented Nov 25, 2024

We could remove the word "additional"?

@lpil lpil added help wanted Contributions encouraged priority:medium labels Nov 25, 2024
@joshi-monster
Copy link
Contributor Author

joshi-monster commented Nov 25, 2024

That might be not clear as well, since arguments that are passed as labelled arguments are removed from the list right now.

Would it maybe be possible to split the list like so?


This call is missing these arguments:

  - c c: String

Additionally, it accepts the following labelled arguments:

  - a: Int
  - b: Float

It could list positional missing arguments as well in the first list, without a label. I added type annotations in the example because I think that would also be cool, but that's probably another issue :)

@lpil
Copy link
Member

lpil commented Nov 25, 2024

Can you tell which ones have been supplied when it's incomplete like that?

@joshi-monster
Copy link
Contributor Author

Do you not think that we can assume that positional arguments that type-check are valid? If they don't typecheck, you get a type mismatch error for the first one as well right now (there seem to be some inconsistencies here between positional and labelled arguments too), and if they have a label, they would still be printed anyways in the second list.

Maybe this is the assumption I make because I work this way, but is not generalisable to everyone?

@lpil
Copy link
Member

lpil commented Nov 25, 2024

If only positional arguments are supplied it doesn't sound too bad, but it gets less clear if there's a mix. I may be worrying about nothing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Contributions encouraged priority:medium
Projects
None yet
Development

No branches or pull requests

3 participants