Fix @scope shadowing (rewrite using globalThis) (#8100)

* Fix `@scope` shadowing by module name

* CHANGELOG

Author: cknitt

AGENTS.md

@@ -49,6 +49,7 @@ The Makefile’s targets build on each other in this order:
 - **Be careful with similar constructor names across different IRs** - Note that `Lam` (Lambda IR) and `Lambda` (typed lambda) have variants with similar constructor names like `Ltrywith`, but they represent different things in different compilation phases.
 
 - **Avoid warning suppressions** - Never use `[@@warning "..."]` to silence warnings. Instead, fix the underlying issue properly
+- **Skip trailing `; _` in record patterns** - The warning it targets is disabled in this codebase, so prefer `{field = x}` over `{field = x; _}`.
 
 - **Do not introduce new keywords unless absolutely necessary** - Try to find ways to implement features without reserving keywords, as seen with the "catch" implementation that avoids making it a keyword.
 

CHANGELOG.md

@@ -25,6 +25,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
+- Fix `@scope` shadowing (rewrite using `globalThis`). https://github.com/rescript-lang/rescript/pull/8100
 
 #### :memo: Documentation
 

compiler/core/js_pass_external_shadow.ml

@@ -24,6 +24,8 @@
 
 module E = Js_exp_make
 
+module StringSet = Set.Make (String)
+
 let global_this = E.js_global "globalThis"
 
 (* Only rewrite for lexical bindings (let-style). *)
@@ -55,18 +57,42 @@ let rewrite_shadowed_global_in_expr ~(name : string) (expr : J.expression) :
   self.expression self expr
 
 let program (js : J.program) : J.program =
+  let shadowed_globals =
+    Ext_list.fold_left js.block StringSet.empty (fun acc (st : J.statement) ->
+        match st.statement_desc with
+        | Variable {ident; property}
+          when is_lexical_binding_kind property && should_rewrite_binding ident
+          ->
+          StringSet.add ident.name acc
+        | _ -> acc)
+  in
   let super = Js_record_map.super in
   let self =
     {
       super with
+      expression =
+        (fun self expr ->
+          match expr.expression_desc with
+          | Static_index (obj, field, pos) ->
+            let obj = self.expression self obj in
+            let obj =
+              match obj.expression_desc with
+              | Var (Id id)
+                when Ext_ident.is_js id
+                     && StringSet.mem id.name shadowed_globals ->
+                E.dot global_this id.name
+              | _ -> obj
+            in
+            {expr with expression_desc = Static_index (obj, field, pos)}
+          | _ -> super.expression self expr);
       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 with value = Some expr}
           | _ -> super.variable_declaration self vd);
     }
   in

compiler/core/js_pass_external_shadow.mli

@@ -22,5 +22,5 @@
  * 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. *)
+(* Rewrite to using globalThis when shadowing would break @val/@scope globals. *)
 val program : J.program -> J.program

tests/tests/src/ExternalScopeShadow.mjs

@@ -0,0 +1,16 @@
+// Generated by ReScript, PLEASE EDIT WITH CARE
+
+
+let URL = {};
+
+globalThis.URL.revokeObjectURL("some url");
+
+globalThis.URL.revokeObjectURL("some url");
+
+let MyURL;
+
+export {
+  URL,
+  MyURL,
+}
+/*  Not a pure module */

tests/tests/src/ExternalScopeShadow.res

@@ -0,0 +1,11 @@
+/* @scope should not be shadowed by a module with the same name. */
+module URL = {
+  @val @scope("URL")
+  external revokeObjectURL: string => unit = "revokeObjectURL"
+}
+
+URL.revokeObjectURL("some url") /* expect `globalThis.URL.revokeObjectURL(...)` */
+
+module MyURL = URL
+
+MyURL.revokeObjectURL("some url") /* expect `globalThis.URL.revokeObjectURL(...)` */