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.

Sign up for GitHub

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

Fix issue with error messages for uncurried functions where expected … #5973

Merged
merged 1 commit into from
Feb 4, 2023
Merged

Fix issue with error messages for uncurried functions where expected … #5973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ These are only breaking changes for unformatted code.
- Fix issue where error messages related to non-existent props were displayed without location information https://github.com/rescript-lang/rescript-compiler/pull/5960
- Fix type inference issue with uncurried functions applied to a single unit argument. The issue was introduced in https://github.com/rescript-lang/rescript-compiler/pull/5907 when adding support to `foo()` when `foo` has only optional arguments. https://github.com/rescript-lang/rescript-compiler/pull/5970
- Fix issue where uncurried functions were incorrectly converting the type of a prop given as a default value to curried https://github.com/rescript-lang/rescript-compiler/pull/5971
- Fix issue with error messages for uncurried functions where expected and given type were swapped https://github.com/rescript-lang/rescript-compiler/pull/5973

#### :nail_care: Polish

Expand Down
10 changes: 10 additions & 0 deletions jscomp/build_tests/super_errors/expected/curried_expected.res.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

We've found a bug for you!
/.../fixtures/curried_expected.res:3:24-38

1 │ let expectCurried = f => f(1) + 2
2 │
3 │ let z1 = expectCurried((. x, y) => x+y)
4 │

This function is an uncurried function where a curried function is expected
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message used to say the exact opposite before this PR.

3 changes: 3 additions & 0 deletions jscomp/build_tests/super_errors/fixtures/curried_expected.res
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
let expectCurried = f => f(1) + 2

let z1 = expectCurried((. x, y) => x+y)
2 changes: 1 addition & 1 deletion jscomp/ml/typecore.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2107,7 +2107,7 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected =
let state = Warnings.backup () in
let arity = Ast_uncurried.attributes_to_arity sexp.pexp_attributes in
let uncurried_typ = Ast_uncurried.make_uncurried_type ~env ~arity (newvar()) in
unify_exp_types loc env ty_expected uncurried_typ;
unify_exp_types loc env uncurried_typ ty_expected;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expected type goes on the rhs, just like other instances in this file.

(* Disable Unerasable_optional_argument for uncurried functions *)
let unerasable_optional_argument = Warnings.number Unerasable_optional_argument in
Warnings.parse_options false ("-" ^ string_of_int unerasable_optional_argument);
Expand Down
9 changes: 7 additions & 2 deletions jscomp/super_errors/super_typecore.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ end

let reportArityMismatch ~arityA ~arityB ppf =
fprintf ppf "This function expected @{<info>%s@} %s, but got @{<error>%s@}"
arityA
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was swapped, not swapping it back so it's consistent with the other error messages (A is the given one, and B is the expected).

(if arityA = "1" then "argument" else "arguments")
arityB
(if arityB = "1" then "argument" else "arguments")
arityA

(* Pasted from typecore.ml. Needed for some cases in report_error below *)
(* Records *)
Expand Down Expand Up @@ -169,6 +169,11 @@ let report_error env ppf = function
(_, {desc = Tconstr (Pident {name = "function$"},_,_)}) :: _
) ->
fprintf ppf "This function is a curried function where an uncurried function is expected"
| Expr_type_clash (
(_, {desc = Tconstr (Pident {name = "function$"}, [{desc=Tvar _}; _],_)}) ::
(_, {desc = Tarrow _}) :: _
) ->
fprintf ppf "This function is an uncurried function where a curried function is expected"
Copy link
Collaborator Author

@cristianoc cristianoc Feb 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This avoids leaking $function<'a, ...> and talks about "an uncurried function" generically.

| Expr_type_clash (
(_, {desc = Tconstr (Pident {name = "function$"},[_; tA],_)}) ::
(_, {desc = Tconstr (Pident {name = "function$"},[_; tB],_)}) :: _
Expand Down