Skip to content

Commit 6fd5e2a

Browse files
authored
Fix @val shadowing (rewrite using globalThis) (#8098)
* Fix shadowing @Val bindings (rewrite using globalThis) * Add example with @new * CHANGELOG
1 parent 699d02b commit 6fd5e2a

File tree

6 files changed

+155
-1
lines changed

6 files changed

+155
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
- 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
2626
- Fix: do not warn for "editor" field in `rescript.json`. https://github.com/rescript-lang/rescript/pull/8084
27+
- Fix `@val` shadowing (rewrite using `globalThis`). https://github.com/rescript-lang/rescript/pull/8098
2728

2829
#### :memo: Documentation
2930

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
(* Copyright (C) 2025 - Authors of ReScript
2+
*
3+
* This program is free software: you can redistribute it and/or modify
4+
* it under the terms of the GNU Lesser General Public License as published by
5+
* the Free Software Foundation, either version 3 of the License, or
6+
* (at your option) any later version.
7+
*
8+
* In addition to the permissions granted to you by the LGPL, you may combine
9+
* or link a "work that uses the Library" with a publicly distributed version
10+
* of this file to produce a combined library or application, then distribute
11+
* that combined work under the terms of your choosing, with no requirement
12+
* to comply with the obligations normally placed on you by section 4 of the
13+
* LGPL version 3 (or the corresponding section of a later version of the LGPL
14+
* should you choose to use a later version).
15+
*
16+
* This program is distributed in the hope that it will be useful,
17+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
18+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
19+
* GNU Lesser General Public License for more details.
20+
*
21+
* You should have received a copy of the GNU Lesser General Public License
22+
* along with this program; if not, write to the Free Software
23+
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *)
24+
25+
module E = Js_exp_make
26+
27+
let global_this = E.js_global "globalThis"
28+
29+
(* Only rewrite for lexical bindings (let-style). *)
30+
let is_lexical_binding_kind (property : J.property) =
31+
match property with
32+
| Strict | StrictOpt | Alias -> true
33+
| Variable -> false
34+
35+
(* Skip JS globals/keywords and compiler-reserved JS ids. *)
36+
let should_rewrite_binding (ident : Ident.t) =
37+
(not (Ext_ident.is_js ident))
38+
&& (not (Js_reserved_map.is_js_keyword ident.name))
39+
&& not (Js_reserved_map.is_js_global ident.name)
40+
41+
let rewrite_shadowed_global_in_expr ~(name : string) (expr : J.expression) :
42+
J.expression =
43+
let super = Js_record_map.super in
44+
let self =
45+
{
46+
super with
47+
expression =
48+
(fun self expr ->
49+
match expr.expression_desc with
50+
| Var (Id id) when Ext_ident.is_js id && String.equal id.name name ->
51+
E.dot global_this name
52+
| _ -> super.expression self expr);
53+
}
54+
in
55+
self.expression self expr
56+
57+
let program (js : J.program) : J.program =
58+
let super = Js_record_map.super in
59+
let self =
60+
{
61+
super with
62+
variable_declaration =
63+
(fun self (vd : J.variable_declaration) ->
64+
match vd with
65+
| {ident; value = Some expr; property}
66+
when is_lexical_binding_kind property
67+
&& should_rewrite_binding ident ->
68+
let expr = rewrite_shadowed_global_in_expr ~name:ident.name expr in
69+
{vd with value = Some expr}
70+
| _ -> super.variable_declaration self vd);
71+
}
72+
in
73+
self.program self js
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
(* Copyright (C) 2025 - Authors of ReScript
2+
*
3+
* This program is free software: you can redistribute it and/or modify
4+
* it under the terms of the GNU Lesser General Public License as published by
5+
* the Free Software Foundation, either version 3 of the License, or
6+
* (at your option) any later version.
7+
*
8+
* In addition to the permissions granted to you by the LGPL, you may combine
9+
* or link a "work that uses the Library" with a publicly distributed version
10+
* of this file to produce a combined library or application, then distribute
11+
* that combined work under the terms of your choosing, with no requirement
12+
* to comply with the obligations normally placed on you by section 4 of the
13+
* LGPL version 3 (or the corresponding section of a later version of the LGPL
14+
* should you choose to use a later version).
15+
*
16+
* This program is distributed in the hope that it will be useful,
17+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
18+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
19+
* GNU Lesser General Public License for more details.
20+
*
21+
* You should have received a copy of the GNU Lesser General Public License
22+
* along with this program; if not, write to the Free Software
23+
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *)
24+
25+
(* Rewrites @val/@new shadowing to avoid reading a let before it is set. *)
26+
val program : J.program -> J.program

compiler/core/lam_compile_main.ml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,9 @@ in
242242
js
243243
|> _j "initial"
244244
|> Js_pass_flatten.program
245-
|> _j "flattern"
245+
|> _j "flatten"
246+
|> Js_pass_external_shadow.program
247+
|> _j "external_shadow"
246248
|> Js_pass_tailcall_inline.tailcall_inline
247249
|> _j "inline_and_shake"
248250
|> Js_pass_flatten_and_mark_dead.program

tests/tests/src/ExternalShadow.mjs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Generated by ReScript, PLEASE EDIT WITH CARE
2+
3+
4+
let X = {};
5+
6+
let process = globalThis.process;
7+
8+
let proc = process;
9+
10+
let New = {};
11+
12+
function url(prim) {
13+
return new URL(prim);
14+
}
15+
16+
let Global = {};
17+
18+
let $$parseInt = parseInt;
19+
20+
export {
21+
X,
22+
process,
23+
proc,
24+
New,
25+
url,
26+
Global,
27+
$$parseInt,
28+
}
29+
/* process Not a pure module */

tests/tests/src/ExternalShadow.res

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/* Avoid @val shadowing: `let process = X.process` must not read itself.
2+
The initializer should compile to `globalThis.process`, while other
3+
uses of the external stay as plain `process`. */
4+
module X = {
5+
@val external process: unknown = "process"
6+
}
7+
8+
let process = X.process /* expect `globalThis.process` */
9+
let proc = X.process /* expect plain `process` */
10+
11+
/* @new does not have the same shadowing issue because it uses `new URL(...)`. */
12+
module New = {
13+
@new external url: string => unknown = "URL"
14+
}
15+
16+
let url = New.url /* expect `new URL(...)` in a wrapper function */
17+
18+
/* Reserved JS globals should not be rewritten to globalThis. */
19+
module Global = {
20+
@val external parseInt: unknown = "parseInt"
21+
}
22+
23+
let parseInt = Global.parseInt /* expect plain `parseInt` */

0 commit comments

Comments
 (0)