Check for excess generic arguments

[josh-degraw]

Closes #149

[kaleidawave]

Awesome, thanks for the addition! Tests are great and passing.

The final part is adding a position to the new error. You should be able to create a range for the excess type parameter diagnostic using Span::union in something like:

let first_excess_type_parameter = &call_site_type_arguments[expected_parameters_length];
let last_excess_type_parameter = call_site_type_arguments.last();
let error_position = first_excess_type_parameter.1.union(&last_excess_type_parameter.1);

Then in diagnostics you can change the TypeCheckError => Diagnostic to build a Diagnostic::Position

After that change it will be good to merge! Another two to the test count!

[josh-degraw]

Awesome, thanks for the addition! Tests are great and passing.

The final part is adding a position to the new error. You should be able to create a range for the excess type parameter diagnostic using Span::union in something like:

let first_excess_type_parameter = &call_site_type_arguments[expected_parameters_length];
let last_excess_type_parameter = call_site_type_arguments.last();
let error_position = first_excess_type_parameter.1.union(&last_excess_type_parameter.1);

Then in diagnostics you can change the TypeCheckError => Diagnostic to build a Diagnostic::Position

After that change it will be good to merge! Another two to the test count!

Thank you for the guidance! I was wracking my brain trying to figure out how to get the position info lmao.

[josh-degraw]

Do you think it would make sense to have a specific error type for when the function isn't generic in the first place? Excess type argument makes sense if you provide more than you should, but I feel like something like Cannot pass a type argument to a non-generic function might make more sense? Seems straightforward enough to account for that since I'm already returning this error in two spots in this function for each situation.

[kaleidawave]

something like Cannot pass a type argument to a non-generic function might make more sense

I wondered about that. I checked and TSC doesn't actually have a specific error for that. I think that would be a good idea if it is not too hard to add!

Also in the other diagnostic could the count be printed (it is being calculated but not put in the diagnostic). So "n excess type arguments".

After that good to merge!

[josh-degraw]

Would you prefer to match closer to what tsc does? e.g.

Expected {x} type arguments, but got {y}.

[kaleidawave]

not sure 😅 I like "n excess ..." as it is cleaner. But maybe the expected count would be helpful? Up to you :)

[josh-degraw]

not sure 😅 I like "n excess ..." as it is cleaner. But maybe the expected count would be helpful? Up to you :)

Haha it's your project, it's your call :) I personally feel like knowing how many types the function could accept would be helpful so I don't need to do that math in my head. I'm also inclined to say that using expected x, got y can be a good choice because it feels equally as readable whether the function accepts any parameters or not, and can also require just one error type if that's preferred.

BUT I do it could be useful to have distinct errors for each and the non-generic function case could still benefit from having a specific message (though the message could also technically be inferred by the count fields too, to limit the match clauses required since it is technically the same problem) 🤔

[josh-degraw]

Okay I updated it to do both and conditionally use a more specific error message when the function is not generic. If you're not a fan, I'm happy to pull that back out and simplify it either direction.

[kaleidawave]

rustfmt didn't like diagnostics.rs for some reason? Normally something to do with a macro like format. Have cleaned it up manually.

Have made a quick tweak to avoid (s). Thanks for making the change to print the count. Another two tests 🎉!