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.

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

chore(compiler): Add fromNumber(<literal>) warning for short types #2024

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 33 additions & 13 deletions compiler/src/typed/typed_well_formedness.re
Expand Up @@ -276,7 +276,7 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = {
Grain_parsing.Location.prerr_warning(exp_loc, warning);
};
}
// Check: Warn if using Int32.fromNumber(<literal>)
// Check: Warn if using XXXX.fromNumber(<literal>)
| TExpApp(
{
exp_desc:
Expand All @@ -289,42 +289,62 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = {
_,
[
(
Unlabeled,
_,
{
exp_desc:
TExpConstant(
Const_number(
(Const_number_int(_) | Const_number_float(_)) as n,
(
Const_number_int(_) | Const_number_float(_) |
Const_number_rational(_) |
Const_number_bigint(_)
) as n,
spotandjake marked this conversation as resolved.
Show resolved Hide resolved
),
),
},
),
],
)
when
modname == "Int32"
modname == "Int8"
spotandjake marked this conversation as resolved.
Show resolved Hide resolved
|| modname == "Int16"
|| modname == "Int32"
|| modname == "Int64"
|| modname == "Uint8"
|| modname == "Uint16"
|| modname == "Uint32"
|| modname == "Uint64"
|| modname == "Float32"
|| modname == "Float64" =>
|| modname == "Float64"
|| modname == "Rational"
|| modname == "BigInt" =>
// NOTE: Due to type-checking, we shouldn't need to worry about ending up with a FloatXX value and a Const_number_float
let n_str =
switch (n) {
| Const_number_int(nint) => Int64.to_string(nint)
| Const_number_float(nfloat) => Float.to_string(nfloat)
| _ => failwith("Impossible")
| Const_number_rational({rational_num_rep, rational_den_rep}) =>
Printf.sprintf("%s/%s", rational_num_rep, rational_den_rep)
| Const_number_bigint({bigint_rep}) => bigint_rep
spotandjake marked this conversation as resolved.
Show resolved Hide resolved
};
let warning =
let mod_type =
switch (modname) {
| "Int32" => Grain_utils.Warnings.FromNumberLiteralI32(n_str)
| "Int64" => Grain_utils.Warnings.FromNumberLiteralI64(n_str)
| "Uint32" => Grain_utils.Warnings.FromNumberLiteralU32(n_str)
| "Uint64" => Grain_utils.Warnings.FromNumberLiteralU64(n_str)
| "Float32" => Grain_utils.Warnings.FromNumberLiteralF32(n_str)
| "Float64" => Grain_utils.Warnings.FromNumberLiteralF64(n_str)
| "Int8" => Grain_utils.Warnings.Int8
| "Int16" => Grain_utils.Warnings.Int16
| "Int32" => Grain_utils.Warnings.Int32
| "Int64" => Grain_utils.Warnings.Int64
| "Uint8" => Grain_utils.Warnings.Uint8
| "Uint16" => Grain_utils.Warnings.Uint16
| "Uint32" => Grain_utils.Warnings.Uint32
| "Uint64" => Grain_utils.Warnings.Uint64
| "Float32" => Grain_utils.Warnings.Float32
| "Float64" => Grain_utils.Warnings.Float64
| "Rational" => Grain_utils.Warnings.Rational
| "BigInt" => Grain_utils.Warnings.BigInt
| _ => failwith("Impossible")
};
let warning =
Grain_utils.Warnings.FromNumberLiteral(mod_type, modname, n_str);
if (Grain_utils.Warnings.is_active(warning)) {
Grain_parsing.Location.prerr_warning(exp_loc, warning);
};
Expand Down
93 changes: 44 additions & 49 deletions compiler/src/utils/warnings.re
Expand Up @@ -6,6 +6,20 @@ type loc = {
loc_ghost: bool,
};

type number_type =
| Int8
| Int16
| Int32
| Int64
| Uint8
| Uint16
| Uint32
| Uint64
| Float32
| Float64
| Rational
| BigInt;

type t =
| LetRecNonFunction(string)
| AmbiguousName(list(string), list(string), bool)
Expand All @@ -24,15 +38,10 @@ type t =
| ShadowConstructor(string)
| NoCmiFile(string, option(string))
| FuncWasmUnsafe(string, string, string)
| FromNumberLiteralI32(string)
| FromNumberLiteralI64(string)
| FromNumberLiteralU32(string)
| FromNumberLiteralU64(string)
| FromNumberLiteralF32(string)
| FromNumberLiteralF64(string)
| FromNumberLiteral(number_type, string, string)
| UselessRecordSpread;

let last_warning_number = 24;
let last_warning_number = 19;

let number =
fun
Expand All @@ -53,12 +62,7 @@ let number =
| NonClosedRecordPattern(_) => 15
| UnusedExtension => 16
| FuncWasmUnsafe(_, _, _) => 17
| FromNumberLiteralI32(_) => 18
| FromNumberLiteralI64(_) => 19
| FromNumberLiteralU32(_) => 20
| FromNumberLiteralU64(_) => 21
| FromNumberLiteralF32(_) => 22
| FromNumberLiteralF64(_) => 23
| FromNumberLiteral(_, _, _) => 18
| UselessRecordSpread => last_warning_number;

let message =
Expand Down Expand Up @@ -127,36 +131,32 @@ let message =
++ " from the `"
++ m
++ "` module the instead."
| FromNumberLiteralI32(n) =>
Printf.sprintf(
"it looks like you are calling Int32.fromNumber() with a constant number. Try using the literal syntax (e.g. `%sl`) instead.",
n,
)
| FromNumberLiteralI64(n) =>
Printf.sprintf(
"it looks like you are calling Int64.fromNumber() with a constant number. Try using the literal syntax (e.g. `%sL`) instead.",
n,
)
| FromNumberLiteralU32(n) =>
Printf.sprintf(
"it looks like you are calling Uint32.fromNumber() with a constant number. Try using the literal syntax (e.g. `%sul`) instead.",
n,
)
| FromNumberLiteralU64(n) =>
Printf.sprintf(
"it looks like you are calling Uint64.fromNumber() with a constant number. Try using the literal syntax (e.g. `%suL`) instead.",
n,
)
| FromNumberLiteralF32(n) =>
Printf.sprintf(
"it looks like you are calling Float32.fromNumber() with a constant number. Try using the literal syntax (e.g. `%sf`) instead.",
String.contains(n, '.') ? n : n ++ ".",
)
| FromNumberLiteralF64(n) =>
Printf.sprintf(
"it looks like you are calling Float64.fromNumber() with a constant number. Try using the literal syntax (e.g. `%sd`) instead.",
String.contains(n, '.') ? n : n ++ ".",
)
| FromNumberLiteral(mod_type, mod_name, n) => {
let literal =
switch (mod_type) {
| Int8 => Printf.sprintf("%ss", n)
| Int16 => Printf.sprintf("%sS", n)
| Int32 => Printf.sprintf("%sl", n)
| Int64 => Printf.sprintf("%sL", n)
| Uint8 => Printf.sprintf("%sus", n)
| Uint16 => Printf.sprintf("%suS", n)
| Uint32 => Printf.sprintf("%sul", n)
| Uint64 => Printf.sprintf("%suL", n)
| Float32 =>
Printf.sprintf("%sf", String.contains(n, '.') ? n : n ++ ".")

| Float64 =>
Printf.sprintf("%sd", String.contains(n, '.') ? n : n ++ ".")
| Rational =>
Printf.sprintf("%sr", String.contains(n, '/') ? n : n ++ "/1")
| BigInt => Printf.sprintf("%st", n)
};
Printf.sprintf(
"it looks like you are calling %s.fromNumber() with a constant number. Try using the literal syntax (e.g. `%s`) instead.",
mod_name,
literal,
);
}
| UselessRecordSpread => "this record spread is useless as all of the record's fields are overridden.";

let sub_locs =
Expand Down Expand Up @@ -200,12 +200,7 @@ let defaults = [
ShadowConstructor(""),
NoCmiFile("", None),
FuncWasmUnsafe("", "", ""),
FromNumberLiteralI32(""),
FromNumberLiteralI64(""),
FromNumberLiteralU32(""),
FromNumberLiteralU64(""),
FromNumberLiteralF32(""),
FromNumberLiteralF64(""),
FromNumberLiteral(Int8, "", ""),
UselessRecordSpread,
];

Expand Down
21 changes: 15 additions & 6 deletions compiler/src/utils/warnings.rei
Expand Up @@ -20,6 +20,20 @@ type loc = {
loc_ghost: bool,
};

type number_type =
| Int8
| Int16
| Int32
| Int64
| Uint8
| Uint16
| Uint32
| Uint64
| Float32
| Float64
| Rational
| BigInt;

type t =
| LetRecNonFunction(string)
| AmbiguousName(list(string), list(string), bool)
Expand All @@ -38,12 +52,7 @@ type t =
| ShadowConstructor(string)
| NoCmiFile(string, option(string))
| FuncWasmUnsafe(string, string, string)
| FromNumberLiteralI32(string)
| FromNumberLiteralI64(string)
| FromNumberLiteralU32(string)
| FromNumberLiteralU64(string)
| FromNumberLiteralF32(string)
| FromNumberLiteralF64(string)
| FromNumberLiteral(number_type, string, string)
| UselessRecordSpread;

let is_active: t => bool;
Expand Down
110 changes: 103 additions & 7 deletions compiler/test/suites/numbers.re
Expand Up @@ -199,34 +199,130 @@ describe("numbers", ({test, testSkip}) => {
);

// well-formedness warnings
test("float32_fromNumber_warn1", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF32("5"))).toMatch(
test("short_fromNumber_warn1", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteral(Uint8, "Uint8", "2"))).
toMatch(
"2us",
)
});
test("short_fromNumber_warn2", ({expect}) => {
expect.string(
Warnings.message(FromNumberLiteral(Uint16, "Uint16", "2")),
).
toMatch(
"2uS",
)
});
test("short_fromNumber_warn3", ({expect}) => {
expect.string(
Warnings.message(FromNumberLiteral(Uint32, "Uint32", "2")),
).
toMatch(
"2ul",
)
});
test("short_fromNumber_warn4", ({expect}) => {
expect.string(
Warnings.message(FromNumberLiteral(Uint64, "Uint64", "2")),
).
toMatch(
"2uL",
)
});
test("short_fromNumber_warn5", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteral(Int8, "Int8", "2"))).
toMatch(
"2s",
)
});
test("short_fromNumber_warn6", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteral(Int16, "Int16", "2"))).
toMatch(
"2S",
)
});
test("short_fromNumber_warn7", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteral(Int32, "Int32", "2"))).
toMatch(
"2l",
)
});
test("short_fromNumber_warn8", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteral(Int64, "Int64", "2"))).
toMatch(
"2L",
)
});
test("float32_fromNumber_warn9", ({expect}) => {
expect.string(
Warnings.message(FromNumberLiteral(Float32, "Float32", "5")),
).
toMatch(
"5.f",
)
});
test("float32_fromNumber_warn2", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF32("5."))).toMatch(
expect.string(
Warnings.message(FromNumberLiteral(Float32, "Float32", "5.")),
).
toMatch(
"5.f",
)
});
test("float32_fromNumber_warn3", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF32("5.5"))).toMatch(
expect.string(
Warnings.message(FromNumberLiteral(Float32, "Float32", "5.5")),
).
toMatch(
"5.5f",
)
});
test("float64_fromNumber_warn1", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF64("5"))).toMatch(
expect.string(
Warnings.message(FromNumberLiteral(Float64, "Float64", "5")),
).
toMatch(
"5.d",
)
});
test("float64_fromNumber_warn2", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF64("5."))).toMatch(
expect.string(
Warnings.message(FromNumberLiteral(Float64, "Float64", "5.")),
).
toMatch(
"5.d",
)
});
test("float64_fromNumber_warn3", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF64("5.5"))).toMatch(
expect.string(
Warnings.message(FromNumberLiteral(Float64, "Float64", "5.5")),
).
toMatch(
"5.5d",
)
});
test("rational_fromNumber_warn1", ({expect}) => {
expect.string(
Warnings.message(FromNumberLiteral(Rational, "Rational", "2")),
).
toMatch(
"2/1r",
)
});
test("rational_fromNumber_warn2", ({expect}) => {
expect.string(
Warnings.message(FromNumberLiteral(Rational, "Rational", "2/4")),
).
toMatch(
"2/4r",
)
});
test("bigint_fromNumber_warn1", ({expect}) => {
expect.string(
Warnings.message(FromNumberLiteral(BigInt, "Bigint", "2")),
).
toMatch(
"2t",
)
});
});