Skip to content

Commit c799ba1

Browse files
authored
Improve error message when concatenating strings (#6416)
* improve error message when concatenating strings * changelog
1 parent ccea327 commit c799ba1

File tree

4 files changed

+52
-18
lines changed

4 files changed

+52
-18
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
- Display the compile time for `rescript build` command. https://github.com/rescript-lang/rescript-compiler/pull/6404
3535
- Improve help message for `build` and `clean` commands. https://github.com/rescript-lang/rescript-compiler/pull/6404
3636
- Pass through the `-verbose` flag to builds in watch mode. https://github.com/rescript-lang/rescript-compiler/pull/6404
37+
- Improve error message when defining duplicate labels in a record. https://github.com/rescript-lang/rescript-compiler/pull/6415
38+
- Improve error message when trying to concatenate strings using the wrong operator. https://github.com/rescript-lang/rescript-compiler/pull/6416
3739

3840
# 11.0.0-rc.3
3941

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
2+
We've found a bug for you!
3+
/.../fixtures/math_operator_string.res:1:9-15
4+
5+
1 │ let x = "hello" + "what"
6+
2 │
7+
8+
This has type: string
9+
But it's being used with the + operator, which works on: int
10+
11+
Are you looking to concatenate strings? Use the operator ++, which concatenates strings.
12+
13+
Possible solutions:
14+
- Change the + operator to ++ to concatenate strings instead.
15+
16+
You can convert string to int with Belt.Int.fromString.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
let x = "hello" + "what"

jscomp/ml/error_message_utils.ml

+33-18
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ let errorExpectedTypeText ppf typeClashContext =
5555
| _ -> fprintf ppf "But it's expected to have type:"
5656

5757
let printExtraTypeClashHelp ppf trace typeClashContext =
58-
match typeClashContext with
59-
| Some (MathOperator {forFloat; operator; isConstant}) -> (
58+
match (typeClashContext, trace) with
59+
| Some (MathOperator {forFloat; operator; isConstant}), _ -> (
6060
let operatorForOtherType =
6161
match operator with
6262
| "+" -> "+."
@@ -77,16 +77,31 @@ let printExtraTypeClashHelp ppf trace typeClashContext =
7777
| _ -> "compute"
7878
in
7979
(* TODO check int vs float explicitly before showing this *)
80-
fprintf ppf
81-
"\n\n\
82-
\ Floats and ints have their own mathematical operators. This means you \
83-
cannot %s a float and an int without converting between the two.\n\n\
84-
\ Possible solutions:\n\
85-
\ - Ensure all values in this calculation has the type @{<info>%s@}. \
86-
You can convert between floats and ints via @{<info>Belt.Float.toInt@} \
87-
and @{<info>Belt.Int.fromFloat@}."
88-
operatorText
89-
(if forFloat then "float" else "int");
80+
(match (operator, trace) with
81+
| ( "+",
82+
[
83+
({Types.desc = Tconstr (p1, _, _)}, _);
84+
({desc = Tconstr (p2, _, _)}, _);
85+
] )
86+
when Path.same Predef.path_string p1 || Path.same Predef.path_string p2 ->
87+
fprintf ppf
88+
"\n\n\
89+
\ Are you looking to concatenate strings? Use the operator \
90+
@{<info>++@}, which concatenates strings.\n\n\
91+
\ Possible solutions:\n\
92+
\ - Change the @{<info>+@} operator to @{<info>++@} to concatenate \
93+
strings instead."
94+
| _ ->
95+
fprintf ppf
96+
"\n\n\
97+
\ Floats and ints have their own mathematical operators. This means \
98+
you cannot %s a float and an int without converting between the two.\n\n\
99+
\ Possible solutions:\n\
100+
\ - Ensure all values in this calculation has the type @{<info>%s@}. \
101+
You can convert between floats and ints via \
102+
@{<info>Belt.Float.toInt@} and @{<info>Belt.Int.fromFloat@}."
103+
operatorText
104+
(if forFloat then "float" else "int"));
90105
match (isConstant, trace) with
91106
| Some constant, _ ->
92107
if forFloat then
@@ -115,30 +130,30 @@ let printExtraTypeClashHelp ppf trace typeClashContext =
115130
(if forFloat then "int" else "float")
116131
| _ -> ())
117132
| _ -> ())
118-
| Some Switch ->
133+
| Some Switch, _ ->
119134
fprintf ppf
120135
"\n\n\
121136
\ All branches in a @{<info>switch@} must return the same type. To fix \
122137
this, change your branch to return the expected type."
123-
| Some IfCondition ->
138+
| Some IfCondition, _ ->
124139
fprintf ppf
125140
"\n\n\
126141
\ To fix this, change the highlighted code so it evaluates to a \
127142
@{<info>bool@}."
128-
| Some IfReturn ->
143+
| Some IfReturn, _ ->
129144
fprintf ppf
130145
"\n\n\
131146
\ @{<info>if@} expressions must return the same type in all branches \
132147
(@{<info>if@}, @{<info>else if@}, @{<info>else@})."
133-
| Some MaybeUnwrapOption ->
148+
| Some MaybeUnwrapOption, _ ->
134149
fprintf ppf
135150
"\n\n\
136151
\ Possible solutions:\n\
137152
\ - Unwrap the option to its underlying value using \
138153
`yourValue->Belt.Option.getWithDefault(someDefaultValue)`"
139-
| Some ComparisonOperator ->
154+
| Some ComparisonOperator, _ ->
140155
fprintf ppf "\n\n You can only compare things of the same type."
141-
| Some ArrayValue ->
156+
| Some ArrayValue, _ ->
142157
fprintf ppf
143158
"\n\n\
144159
\ Arrays can only contain items of the same type.\n\n\

0 commit comments

Comments
 (0)