diff --git a/CHANGELOG.md b/CHANGELOG.md index 824fb890a0..e5b553527d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ #### :rocket: New Feature - Add a first-class `taggedTemplate<'param, 'output>` builtin type and the `TaggedTemplate` stdlib module (`TaggedTemplate.make`). Tagged-template tags are now tracked through the type system, so they emit real JS tagged-template syntax across module boundaries, when passed as first-class values, and when constructed at runtime by a factory (e.g. `postgres`). https://github.com/rescript-lang/rescript/pull/8461 +- Make mutation of private record mutable fields a configurable warning instead of a hard error. https://github.com/rescript-lang/rescript/pull/8366 #### :bug: Bug fix diff --git a/compiler/ext/warnings.ml b/compiler/ext/warnings.ml index 7acd96d03c..c768ae4537 100644 --- a/compiler/ext/warnings.ml +++ b/compiler/ext/warnings.ml @@ -74,6 +74,7 @@ type t = | Bs_toplevel_expression_unit of (string * top_level_unit_help) option (* 109 *) | Bs_todo of string option (* 110 *) + | Bs_private_record_mutation of string (* 111 *) (* If you remove a warning, leave a hole in the numbering. NEVER change the numbers of existing warnings. @@ -126,8 +127,9 @@ let number = function | Bs_integer_literal_overflow -> 107 | Bs_toplevel_expression_unit _ -> 109 | Bs_todo _ -> 110 + | Bs_private_record_mutation _ -> 111 -let last_warning_number = 110 +let last_warning_number = 111 let letter_all = let rec loop i = if i = 0 then [] else i :: loop (i - 1) in @@ -453,6 +455,8 @@ let message = function ^ "\n\n\ \ This code is not implemented yet and will crash at runtime. Make sure \ you implement this before running the code." + | Bs_private_record_mutation field_name -> + Printf.sprintf "Mutation of private record field %S." field_name let sub_locs = function | Deprecated (_, def, use, _) -> @@ -564,6 +568,7 @@ let descriptions = ); (109, "Toplevel expression has unit type"); (110, "Todo found"); + (111, "Mutation of private record field"); ] let help_warnings () = diff --git a/compiler/ext/warnings.mli b/compiler/ext/warnings.mli index 5ebdfa4b24..46cba811ad 100644 --- a/compiler/ext/warnings.mli +++ b/compiler/ext/warnings.mli @@ -67,6 +67,7 @@ type t = | Bs_toplevel_expression_unit of (string * top_level_unit_help) option (* 109 *) | Bs_todo of string option (* 110 *) + | Bs_private_record_mutation of string (* 111 *) val parse_options : bool -> string -> unit diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index efaea72e88..37bbf81b60 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -57,7 +57,6 @@ type error = string * Longident.t * (Path.t * Path.t) * (Path.t * Path.t) list | Undefined_method of type_expr * string * string list option | Private_type of type_expr - | Private_label of Longident.t * type_expr | Not_subtype of Ctype.type_pairs * Ctype.type_pairs * Ctype.subtype_context option | Too_many_arguments of bool * type_expr @@ -317,6 +316,12 @@ let extract_concrete_record env ty = | p0, p, {type_kind = Type_record (fields, repr)} -> (p0, p, fields, repr) | _ -> raise Not_found +let is_private_record_field env label = + match extract_concrete_typedecl env label.lbl_res with + | _, _, {type_kind = Type_record _; type_private = Private} -> true + | _ -> false + | exception Not_found -> false + let extract_concrete_variant env ty = match extract_concrete_typedecl env ty with | p0, p, {type_kind = Type_variant cstrs} -> (p0, p, cstrs) @@ -2953,6 +2958,9 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected) unify_exp ~context:None env record ty_record; if label.lbl_mut = Immutable then raise (Error (loc, env, Label_not_mutable lid.txt)); + if label.lbl_private = Private && is_private_record_field env label then + Location.prerr_warning lid.loc + (Warnings.Bs_private_record_mutation (Longident.last lid.txt)); Builtin_attributes.check_deprecated_mutable lid.loc label.lbl_attributes (Longident.last lid.txt); rue @@ -3637,9 +3645,8 @@ and type_label_exp ~call_context create env loc ty_expected end_def (); (* Generalize information merged from ty_expected *) generalize_structure ty_arg); - if label.lbl_private = Private then - if create then raise (Error (loc, env, Private_type ty_expected)) - else raise (Error (lid.loc, env, Private_label (lid.txt, ty_expected))); + if label.lbl_private = Private && create then + raise (Error (loc, env, Private_type ty_expected)); let arg = let snap = if vars = [] then None else Some (Btype.snapshot ()) in let field_name = Longident.last lid.txt in @@ -4828,9 +4835,6 @@ let report_error env loc ppf error = "In this type, the locally bound module name %s escapes its scope" id | Private_type ty -> fprintf ppf "Cannot create values of the private type %a" type_expr ty - | Private_label (lid, ty) -> - fprintf ppf "Cannot assign field %a of the private type %a" longident lid - type_expr ty | Not_a_variant_type lid -> fprintf ppf "The type %a@ is not a variant type" longident lid | Incoherent_label_order -> diff --git a/compiler/ml/typecore.mli b/compiler/ml/typecore.mli index cf87fe7361..cba37060eb 100644 --- a/compiler/ml/typecore.mli +++ b/compiler/ml/typecore.mli @@ -90,7 +90,6 @@ type error = string * Longident.t * (Path.t * Path.t) * (Path.t * Path.t) list | Undefined_method of type_expr * string * string list option | Private_type of type_expr - | Private_label of Longident.t * type_expr | Not_subtype of Ctype.type_pairs * Ctype.type_pairs * Ctype.subtype_context option | Too_many_arguments of bool * type_expr diff --git a/tests/build_tests/super_errors/expected/private_label.res.expected b/tests/build_tests/super_errors/expected/private_label.res.expected index 7db5e011a5..27f80acafc 100644 --- a/tests/build_tests/super_errors/expected/private_label.res.expected +++ b/tests/build_tests/super_errors/expected/private_label.res.expected @@ -1,10 +1,10 @@ - We've found a bug for you! + Warning number 111 /.../fixtures/private_label.res:10:12 8 │ 9 │ let r = M.make(1) - 10 │ let () = r.x = 2 + 10 │ let () = r.x = 2 11 │ - Cannot assign field x of the private type M.t \ No newline at end of file + Mutation of private record field "x". \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/private_record_construction.res.expected b/tests/build_tests/super_errors/expected/private_record_construction.res.expected new file mode 100644 index 0000000000..6592f6f905 --- /dev/null +++ b/tests/build_tests/super_errors/expected/private_record_construction.res.expected @@ -0,0 +1,10 @@ + + We've found a bug for you! + /.../fixtures/private_record_construction.res:9:30-39 + + 7 │ } + 8 │ + 9 │ let _item: PrivateRecord.t = {value: 1} + 10 │ + + Cannot create values of the private type PrivateRecord.t \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/private_record_immutable_field_mutation.res.expected b/tests/build_tests/super_errors/expected/private_record_immutable_field_mutation.res.expected new file mode 100644 index 0000000000..f1c61f660a --- /dev/null +++ b/tests/build_tests/super_errors/expected/private_record_immutable_field_mutation.res.expected @@ -0,0 +1,10 @@ + + We've found a bug for you! + /.../fixtures/private_record_immutable_field_mutation.res:10:1-21 + + 8 │ + 9 │ let item = PrivateRecord.make(1) + 10 │ item.name = "changed" + 11 │ + + The record field name is not mutable \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/private_record_mutation.res.expected b/tests/build_tests/super_errors/expected/private_record_mutation.res.expected new file mode 100644 index 0000000000..1571c05146 --- /dev/null +++ b/tests/build_tests/super_errors/expected/private_record_mutation.res.expected @@ -0,0 +1,10 @@ + + Warning number 111 + /.../fixtures/private_record_mutation.res:10:6-10 + + 8 │ + 9 │ let item = PrivateRecord.make(1) + 10 │ item.value = 2 + 11 │ + + Mutation of private record field "value". \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/private_record_mutation_warning_disabled.res.expected b/tests/build_tests/super_errors/expected/private_record_mutation_warning_disabled.res.expected new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/build_tests/super_errors/expected/private_record_update.res.expected b/tests/build_tests/super_errors/expected/private_record_update.res.expected new file mode 100644 index 0000000000..e94f2133bd --- /dev/null +++ b/tests/build_tests/super_errors/expected/private_record_update.res.expected @@ -0,0 +1,10 @@ + + We've found a bug for you! + /.../fixtures/private_record_update.res:10:16-34 + + 8 │ + 9 │ let item = PrivateRecord.make(1) + 10 │ let _updated = {...item, value: 2} + 11 │ + + Cannot create values of the private type PrivateRecord.t \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/private_record_construction.res b/tests/build_tests/super_errors/fixtures/private_record_construction.res new file mode 100644 index 0000000000..bd51cd9fdd --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/private_record_construction.res @@ -0,0 +1,9 @@ +module PrivateRecord: { + type t = private {mutable value: int} + let make: int => t +} = { + type t = {mutable value: int} + let make = value => {value: value} +} + +let _item: PrivateRecord.t = {value: 1} diff --git a/tests/build_tests/super_errors/fixtures/private_record_immutable_field_mutation.res b/tests/build_tests/super_errors/fixtures/private_record_immutable_field_mutation.res new file mode 100644 index 0000000000..2e0f7e326e --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/private_record_immutable_field_mutation.res @@ -0,0 +1,10 @@ +module PrivateRecord: { + type t = private {mutable value: int, name: string} + let make: int => t +} = { + type t = {mutable value: int, name: string} + let make = value => {value, name: "stable"} +} + +let item = PrivateRecord.make(1) +item.name = "changed" diff --git a/tests/build_tests/super_errors/fixtures/private_record_mutation.res b/tests/build_tests/super_errors/fixtures/private_record_mutation.res new file mode 100644 index 0000000000..c375d8e6b3 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/private_record_mutation.res @@ -0,0 +1,10 @@ +module PrivateRecord: { + type t = private {mutable value: int} + let make: int => t +} = { + type t = {mutable value: int} + let make = value => {value: value} +} + +let item = PrivateRecord.make(1) +item.value = 2 diff --git a/tests/build_tests/super_errors/fixtures/private_record_mutation_warning_disabled.res b/tests/build_tests/super_errors/fixtures/private_record_mutation_warning_disabled.res new file mode 100644 index 0000000000..d2bf48d0bd --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/private_record_mutation_warning_disabled.res @@ -0,0 +1,12 @@ +@@warning("-111") + +module PrivateRecord: { + type t = private {mutable value: int} + let make: int => t +} = { + type t = {mutable value: int} + let make = value => {value: value} +} + +let item = PrivateRecord.make(1) +item.value = 2 diff --git a/tests/build_tests/super_errors/fixtures/private_record_update.res b/tests/build_tests/super_errors/fixtures/private_record_update.res new file mode 100644 index 0000000000..41c0be841b --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/private_record_update.res @@ -0,0 +1,10 @@ +module PrivateRecord: { + type t = private {mutable value: int} + let make: int => t +} = { + type t = {mutable value: int} + let make = value => {value: value} +} + +let item = PrivateRecord.make(1) +let _updated = {...item, value: 2} diff --git a/tests/tests/src/privateRecordMutation.mjs b/tests/tests/src/privateRecordMutation.mjs new file mode 100644 index 0000000000..6de7f921e7 --- /dev/null +++ b/tests/tests/src/privateRecordMutation.mjs @@ -0,0 +1,24 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + + +function make(value) { + return { + value: value, + name: "stable" + }; +} + +function value(t) { + return t.value; +} + +function name(t) { + return t.name; +} + +export { + make, + value, + name, +} +/* No side effect */ diff --git a/tests/tests/src/privateRecordMutation.res b/tests/tests/src/privateRecordMutation.res new file mode 100644 index 0000000000..bf8e308c43 --- /dev/null +++ b/tests/tests/src/privateRecordMutation.res @@ -0,0 +1,8 @@ +type t = { + mutable value: int, + name: string, +} + +let make = value => {value, name: "stable"} +let value = t => t.value +let name = t => t.name diff --git a/tests/tests/src/privateRecordMutation.resi b/tests/tests/src/privateRecordMutation.resi new file mode 100644 index 0000000000..7e02808da1 --- /dev/null +++ b/tests/tests/src/privateRecordMutation.resi @@ -0,0 +1,8 @@ +type t = private { + mutable value: int, + name: string, +} + +let make: int => t +let value: t => int +let name: t => string diff --git a/tests/tests/src/privateRecordMutation_test.mjs b/tests/tests/src/privateRecordMutation_test.mjs new file mode 100644 index 0000000000..673d1f7e02 --- /dev/null +++ b/tests/tests/src/privateRecordMutation_test.mjs @@ -0,0 +1,17 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as Mocha from "mocha"; +import * as Test_utils from "./test_utils.mjs"; +import * as PrivateRecordMutation from "./privateRecordMutation.mjs"; + +Mocha.describe("PrivateRecordMutation_test", () => { + Mocha.test("mutates a mutable field exposed by a private record when warning is disabled", () => { + let item = PrivateRecordMutation.make(1); + item.value = 2; + Test_utils.eq("File \"privateRecordMutation_test.res\", line 10, characters 7-14", item.value, 2); + Test_utils.eq("File \"privateRecordMutation_test.res\", line 11, characters 7-14", PrivateRecordMutation.value(item), 2); + Test_utils.eq("File \"privateRecordMutation_test.res\", line 12, characters 7-14", item.name, "stable"); + }); +}); + +/* Not a pure module */ diff --git a/tests/tests/src/privateRecordMutation_test.res b/tests/tests/src/privateRecordMutation_test.res new file mode 100644 index 0000000000..053cd3f0a3 --- /dev/null +++ b/tests/tests/src/privateRecordMutation_test.res @@ -0,0 +1,14 @@ +@@warning("-111") + +open Mocha +open Test_utils + +describe(__MODULE__, () => { + test("mutates a mutable field exposed by a private record when warning is disabled", () => { + let item = PrivateRecordMutation.make(1) + item.value = 2 + eq(__LOC__, item.value, 2) + eq(__LOC__, PrivateRecordMutation.value(item), 2) + eq(__LOC__, item.name, "stable") + }) +})