Skip to content

Improve error message when concatenating strings #6416

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

Merged
merged 2 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
- Display the compile time for `rescript build` command. https://github.com/rescript-lang/rescript-compiler/pull/6404
- Improve help message for `build` and `clean` commands. https://github.com/rescript-lang/rescript-compiler/pull/6404
- Pass through the `-verbose` flag to builds in watch mode. https://github.com/rescript-lang/rescript-compiler/pull/6404
- Improve error message when defining duplicate labels in a record. https://github.com/rescript-lang/rescript-compiler/pull/6415
- Improve error message when trying to concatenate strings using the wrong operator. https://github.com/rescript-lang/rescript-compiler/pull/6416

# 11.0.0-rc.3

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@

We've found a bug for you!
/.../fixtures/math_operator_string.res:1:9-15

1 │ let x = "hello" + "what"
2 │

This has type: string
But it's being used with the + operator, which works on: int

Are you looking to concatenate strings? Use the operator ++, which concatenates strings.

Possible solutions:
- Change the + operator to ++ to concatenate strings instead.

You can convert string to int with Belt.Int.fromString.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let x = "hello" + "what"
51 changes: 33 additions & 18 deletions jscomp/ml/error_message_utils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ let errorExpectedTypeText ppf typeClashContext =
| _ -> fprintf ppf "But it's expected to have type:"

let printExtraTypeClashHelp ppf trace typeClashContext =
match typeClashContext with
| Some (MathOperator {forFloat; operator; isConstant}) -> (
match (typeClashContext, trace) with
| Some (MathOperator {forFloat; operator; isConstant}), _ -> (
let operatorForOtherType =
match operator with
| "+" -> "+."
Expand All @@ -77,16 +77,31 @@ let printExtraTypeClashHelp ppf trace typeClashContext =
| _ -> "compute"
in
(* TODO check int vs float explicitly before showing this *)
fprintf ppf
"\n\n\
\ Floats and ints have their own mathematical operators. This means you \
cannot %s a float and an int without converting between the two.\n\n\
\ Possible solutions:\n\
\ - Ensure all values in this calculation has the type @{<info>%s@}. \
You can convert between floats and ints via @{<info>Belt.Float.toInt@} \
and @{<info>Belt.Int.fromFloat@}."
operatorText
(if forFloat then "float" else "int");
(match (operator, trace) with
| ( "+",
[
({Types.desc = Tconstr (p1, _, _)}, _);
({desc = Tconstr (p2, _, _)}, _);
] )
when Path.same Predef.path_string p1 || Path.same Predef.path_string p2 ->
fprintf ppf
"\n\n\
\ Are you looking to concatenate strings? Use the operator \
@{<info>++@}, which concatenates strings.\n\n\
\ Possible solutions:\n\
\ - Change the @{<info>+@} operator to @{<info>++@} to concatenate \
strings instead."
| _ ->
fprintf ppf
"\n\n\
\ Floats and ints have their own mathematical operators. This means \
you cannot %s a float and an int without converting between the two.\n\n\
\ Possible solutions:\n\
\ - Ensure all values in this calculation has the type @{<info>%s@}. \
You can convert between floats and ints via \
@{<info>Belt.Float.toInt@} and @{<info>Belt.Int.fromFloat@}."
operatorText
(if forFloat then "float" else "int"));
match (isConstant, trace) with
| Some constant, _ ->
if forFloat then
Expand Down Expand Up @@ -115,30 +130,30 @@ let printExtraTypeClashHelp ppf trace typeClashContext =
(if forFloat then "int" else "float")
| _ -> ())
| _ -> ())
| Some Switch ->
| Some Switch, _ ->
fprintf ppf
"\n\n\
\ All branches in a @{<info>switch@} must return the same type. To fix \
this, change your branch to return the expected type."
| Some IfCondition ->
| Some IfCondition, _ ->
fprintf ppf
"\n\n\
\ To fix this, change the highlighted code so it evaluates to a \
@{<info>bool@}."
| Some IfReturn ->
| Some IfReturn, _ ->
fprintf ppf
"\n\n\
\ @{<info>if@} expressions must return the same type in all branches \
(@{<info>if@}, @{<info>else if@}, @{<info>else@})."
| Some MaybeUnwrapOption ->
| Some MaybeUnwrapOption, _ ->
fprintf ppf
"\n\n\
\ Possible solutions:\n\
\ - Unwrap the option to its underlying value using \
`yourValue->Belt.Option.getWithDefault(someDefaultValue)`"
| Some ComparisonOperator ->
| Some ComparisonOperator, _ ->
fprintf ppf "\n\n You can only compare things of the same type."
| Some ArrayValue ->
| Some ArrayValue, _ ->
fprintf ppf
"\n\n\
\ Arrays can only contain items of the same type.\n\n\
Expand Down