Improve function apply error messages #7496

zth · 2025-05-21T13:09:18Z

This reworks the function apply error messages some to make them clearer and easier to understand.

Feedback very welcome.

zth · 2025-05-21T13:13:09Z

tests/build_tests/super_errors/expected/method_arity_mismatch.res.expected

-  This function has type (int, int) => unit
-  It is applied with [1;31m1[0m argument but it requires [1;33m2[0m.
+  This function call is incorrect.
+  The function has type:
+  (int, int) => unit
+
+  It is called with [1;31m1[0m argument but requires [1;33m2[0m.


For some reason the new error mechanism does not pick up this specific test. It's an object method call, looks like this:

let f = obj => { obj["hi"](1, 2) obj["hi"](1) }

Couldn't figure out why it wasn't picked up, should've been caught with the too few unlabelled arguments check. Maybe @cristianoc has ideas?

Either way, it's not worse than before, so we can definitely ship this and figure out how to improve it later.

This was resolved right?

No, not yet. But it's not important now, can be investigated as a follow up.

pkg-pr-new · 2025-05-21T13:25:40Z

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7496

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7496

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7496

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7496

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7496

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7496

commit: ff18cec

tsnobip

great work @zth, this is much better, I'd just differentiate more the wording in Apply_wrong_label between the cases where the argument is unlabelled and where it's an incorrect label.

tsnobip · 2025-05-21T13:20:33Z

tests/build_tests/super_errors/expected/arity_mismatch.res.expected

+  This function call is incorrect:
+  - It takes [1;33m1[0m unlabelled argument, but is called with [1;31mjust 0[0m


wow this is much better!

Yes, except for the "It", which now seems to refer to the function call instead of the function.

Suggested change

- It takes �[1;33m1�[0m unlabelled argument, but is called with �[1;31mjust 0�[0m

- The function takes �[1;33m1�[0m unlabelled argument, but is called with �[1;31mjust 0�[0m

"The function" is repeated below though.

I'll change it to The function.

tsnobip · 2025-05-21T13:24:49Z

tests/build_tests/super_errors/expected/fun_return_poly1.res.expected

the double negation makes it a bit hard to follow I think, what about:

Suggested change

This function does not take an unlabelled argument at this position.

It has type: ('a, ~def: int=?) => 'b

The argument at this position should be labelled.

This function has type: ('a, ~def: int=?) => 'b

tsnobip · 2025-05-21T13:25:43Z

tests/build_tests/super_errors/expected/missing_label.res.expected

tsnobip · 2025-05-21T13:26:16Z

tests/build_tests/super_errors/expected/moreArguments1.res.expected

+  - Missing arguments that must be provided: ~b
+
+  The function has type:
+  (~a: int, ~b: int) => int


tsnobip · 2025-05-21T13:26:52Z

tests/build_tests/super_errors/expected/moreArguments4.res.expected

+  - Missing arguments that must be provided: ~d, ~c, ~b
+
+  The function has type:
+  (int, ~b: int, ~c: 'a, ~d: 'b) => int


this is really much better!

tsnobip · 2025-05-21T13:34:12Z

compiler/ml/typecore.ml

    let print_label ppf = function
-      | Noloc.Nolabel -> fprintf ppf "without label"
-      | l -> fprintf ppf "with label %s" (prefixed_label_name l)
+      | Noloc.Nolabel -> fprintf ppf "an unlabelled argument at this position"
+      | l -> fprintf ppf "the argument @{<info>%s@}" (prefixed_label_name l)
    in
-    fprintf ppf
-      "@[<v>@[<2>The function applied to this argument has type@ %a@]@.This \
-       argument cannot be applied %a@]"
-      type_expr ty print_label l
+    fprintf ppf "@[<v>@[<2>This function does not take %a.@]@,It has type: %a@]"
+      print_label l type_expr ty


I would not use the same text for labelled or unlabelled arguments here, see my comment below.

cknitt · 2025-05-21T14:36:23Z

tests/build_tests/super_errors/expected/arity_mismatch.res.expected

+  This function call is incorrect:
+  - It takes [1;33m1[0m unlabelled argument, but is called with [1;31mjust 0[0m


Yes, except for the "It", which now seems to refer to the function call instead of the function.

Suggested change

- It takes �[1;33m1�[0m unlabelled argument, but is called with �[1;31mjust 0�[0m

- The function takes �[1;33m1�[0m unlabelled argument, but is called with �[1;31mjust 0�[0m

"The function" is repeated below though.

cknitt · 2025-05-21T14:40:22Z

tests/build_tests/super_errors/expected/method_arity_mismatch.res.expected

+  This function call is incorrect.
+  The function has type:


Here the function type is shown at the top, and the explanation of what is wrong is shown below.

In the example further above it is the other way around.

I actually prefer the style here, can we standardize on it?

I.e., always start with

This function call is incorrect.
The function has type:
...

and then the explanation below?

Or maybe even on one line:

This function call is incorrect. The function has type:
...

I adjusted. Please have another look.

zth · 2025-05-21T15:50:28Z

@tsnobip @cknitt look again please.

tsnobip

Looks good to me now!

cknitt · 2025-05-22T05:13:50Z

tests/build_tests/super_errors/expected/fun_return_poly1.res.expected

+  The argument at this position should be labelled.
+  This function has type: ('a, ~def: int=?) => 'b


We still have different styles here. Other messages have

function type

error message

This one has

error message

function type

Well, technically I'd say we have this structure:

"Header"

Function type

Error message

For this particular error message I think it makes more sense to have it like this. You're going to see the highlighted position first, then the message that the arg should be labelled, and then the function type. I think that makes sense.

Ok, fine with me, too.

rework function apply error messages

3312980

zth requested review from cknitt, tsnobip, cristianoc and fhammerschmidt May 21, 2025 13:09

update test output

de87848

zth commented May 21, 2025

View reviewed changes

tsnobip reviewed May 21, 2025

View reviewed changes

cknitt reviewed May 21, 2025

View reviewed changes

adjust messages

ff18cec

zth requested review from cknitt and tsnobip May 21, 2025 15:50

tsnobip approved these changes May 21, 2025

View reviewed changes

cknitt reviewed May 22, 2025

View reviewed changes

cknitt approved these changes May 22, 2025

View reviewed changes

zth merged commit 3cbc730 into master May 22, 2025
16 checks passed

zth deleted the function-apply-errors branch May 22, 2025 08:33

		This function call is incorrect:
		- It takes [1;33m1[0m unlabelled argument, but is called with [1;31mjust 0[0m

	- It takes �[1;33m1�[0m unlabelled argument, but is called with �[1;31mjust 0�[0m
	- The function takes �[1;33m1�[0m unlabelled argument, but is called with �[1;31mjust 0�[0m

		The argument at this position should be labelled.
		This function has type: ('a, ~def: int=?) => 'b

Improve function apply error messages #7496

Improve function apply error messages #7496

Uh oh!

Conversation

zth commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cristianoc May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkg-pr-new bot commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tsnobip left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zth commented May 21, 2025

Uh oh!

tsnobip left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zth commented May 21, 2025 •

edited

Loading

cristianoc May 22, 2025 •

edited

Loading

pkg-pr-new bot commented May 21, 2025 •

edited

Loading