diff --git a/CHANGELOG.md b/CHANGELOG.md index 940037e207..895cf7e0a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - Reanalyze: make optional args analysis liveness-aware, preventing false positives when functions are only called from dead code. https://github.com/rescript-lang/rescript/pull/8082 - Fix: do not warn for "editor" field in `rescript.json`. https://github.com/rescript-lang/rescript/pull/8084 +- Fix `@val` shadowing (rewrite using `globalThis`). https://github.com/rescript-lang/rescript/pull/8098 #### :memo: Documentation diff --git a/compiler/core/js_pass_external_shadow.ml b/compiler/core/js_pass_external_shadow.ml new file mode 100644 index 0000000000..8ca3ccb31c --- /dev/null +++ b/compiler/core/js_pass_external_shadow.ml @@ -0,0 +1,73 @@ +(* Copyright (C) 2025 - Authors of ReScript + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * In addition to the permissions granted to you by the LGPL, you may combine + * or link a "work that uses the Library" with a publicly distributed version + * of this file to produce a combined library or application, then distribute + * that combined work under the terms of your choosing, with no requirement + * to comply with the obligations normally placed on you by section 4 of the + * LGPL version 3 (or the corresponding section of a later version of the LGPL + * should you choose to use a later version). + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *) + +module E = Js_exp_make + +let global_this = E.js_global "globalThis" + +(* Only rewrite for lexical bindings (let-style). *) +let is_lexical_binding_kind (property : J.property) = + match property with + | Strict | StrictOpt | Alias -> true + | Variable -> false + +(* Skip JS globals/keywords and compiler-reserved JS ids. *) +let should_rewrite_binding (ident : Ident.t) = + (not (Ext_ident.is_js ident)) + && (not (Js_reserved_map.is_js_keyword ident.name)) + && not (Js_reserved_map.is_js_global ident.name) + +let rewrite_shadowed_global_in_expr ~(name : string) (expr : J.expression) : + J.expression = + let super = Js_record_map.super in + let self = + { + super with + expression = + (fun self expr -> + match expr.expression_desc with + | Var (Id id) when Ext_ident.is_js id && String.equal id.name name -> + E.dot global_this name + | _ -> super.expression self expr); + } + in + self.expression self expr + +let program (js : J.program) : J.program = + let super = Js_record_map.super in + let self = + { + super with + variable_declaration = + (fun self (vd : J.variable_declaration) -> + match vd with + | {ident; value = Some expr; property} + when is_lexical_binding_kind property + && should_rewrite_binding ident -> + let expr = rewrite_shadowed_global_in_expr ~name:ident.name expr in + {vd with value = Some expr} + | _ -> super.variable_declaration self vd); + } + in + self.program self js diff --git a/compiler/core/js_pass_external_shadow.mli b/compiler/core/js_pass_external_shadow.mli new file mode 100644 index 0000000000..0271e886dd --- /dev/null +++ b/compiler/core/js_pass_external_shadow.mli @@ -0,0 +1,26 @@ +(* Copyright (C) 2025 - Authors of ReScript + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * In addition to the permissions granted to you by the LGPL, you may combine + * or link a "work that uses the Library" with a publicly distributed version + * of this file to produce a combined library or application, then distribute + * that combined work under the terms of your choosing, with no requirement + * to comply with the obligations normally placed on you by section 4 of the + * LGPL version 3 (or the corresponding section of a later version of the LGPL + * should you choose to use a later version). + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *) + +(* Rewrites @val/@new shadowing to avoid reading a let before it is set. *) +val program : J.program -> J.program diff --git a/compiler/core/lam_compile_main.ml b/compiler/core/lam_compile_main.ml index bd4191b0e7..5871d643eb 100644 --- a/compiler/core/lam_compile_main.ml +++ b/compiler/core/lam_compile_main.ml @@ -242,7 +242,9 @@ in js |> _j "initial" |> Js_pass_flatten.program -|> _j "flattern" +|> _j "flatten" +|> Js_pass_external_shadow.program +|> _j "external_shadow" |> Js_pass_tailcall_inline.tailcall_inline |> _j "inline_and_shake" |> Js_pass_flatten_and_mark_dead.program diff --git a/tests/tests/src/ExternalShadow.mjs b/tests/tests/src/ExternalShadow.mjs new file mode 100644 index 0000000000..8ad2d4bd8b --- /dev/null +++ b/tests/tests/src/ExternalShadow.mjs @@ -0,0 +1,29 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + + +let X = {}; + +let process = globalThis.process; + +let proc = process; + +let New = {}; + +function url(prim) { + return new URL(prim); +} + +let Global = {}; + +let $$parseInt = parseInt; + +export { + X, + process, + proc, + New, + url, + Global, + $$parseInt, +} +/* process Not a pure module */ diff --git a/tests/tests/src/ExternalShadow.res b/tests/tests/src/ExternalShadow.res new file mode 100644 index 0000000000..aabe1cea27 --- /dev/null +++ b/tests/tests/src/ExternalShadow.res @@ -0,0 +1,23 @@ +/* Avoid @val shadowing: `let process = X.process` must not read itself. + The initializer should compile to `globalThis.process`, while other + uses of the external stay as plain `process`. */ +module X = { + @val external process: unknown = "process" +} + +let process = X.process /* expect `globalThis.process` */ +let proc = X.process /* expect plain `process` */ + +/* @new does not have the same shadowing issue because it uses `new URL(...)`. */ +module New = { + @new external url: string => unknown = "URL" +} + +let url = New.url /* expect `new URL(...)` in a wrapper function */ + +/* Reserved JS globals should not be rewritten to globalThis. */ +module Global = { + @val external parseInt: unknown = "parseInt" +} + +let parseInt = Global.parseInt /* expect plain `parseInt` */