[gjs/wip/ptomato/mozjs52: 37/37] WIP - js: Allow access to modules' lexical scope

[Philip Chimento <pchimento src gnome org>]

commit 5f12298cc97bd82fcbde22087499ae9dc9df326a
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Jun 24 22:28:32 2017 -0700

    WIP - js: Allow access to modules' lexical scope
    
    For compatibility with pre-ES6 code, we allow imported modules to access
    members of the lexical scope (i.e. variables defined with 'const' or
    'let') as if they were properties, because that is how SpiderMonkey
    previously implemented 'let' and 'const'.
    
    When such properties are accessed, however, we log a warning that tells
    people to fix their code. Hopefully such uses will become rare and we can
    remove this compatibility workaround.
    
    FIXME: This still leaks properties into the global object if the module
    defines them on 'this'.

 gjs/module.cpp                     |   68 +++++++++++++++++++++++++++++++++++-
 installed-tests/js/testImporter.js |   17 +++++++++
 2 files changed, 84 insertions(+), 1 deletions(-)
---
diff --git a/gjs/module.cpp b/gjs/module.cpp
index d09ab04..102c0e8 100644
--- a/gjs/module.cpp
+++ b/gjs/module.cpp
@@ -34,6 +34,7 @@ class GjsModule {
 
     enum Slot {
         NAME,
+        LEXICAL_ENV,
         NSLOTS
     };
 
@@ -49,6 +50,14 @@ class GjsModule {
         return retval;
     }
 
+    static inline JSObject *
+    lexical_env(JSObject *module)
+    {
+        JS::Value v_lexical = JS_GetReservedSlot(module, Slot::LEXICAL_ENV);
+        g_assert(v_lexical.isObjectOrNull());
+        return v_lexical.toObjectOrNull();
+    }
+
     /* Creates a JS module object. Use instead of the class's constructor */
     static JSObject *
     create(JSContext   *cx,
@@ -59,6 +68,7 @@ class GjsModule {
         if (!JS_IdToValue(cx, name, &v_name))
             return nullptr;
         JS_SetReservedSlot(module, Slot::NAME, v_name);
+        JS_SetReservedSlot(module, Slot::LEXICAL_ENV, JS::NullValue());
         return module;
     }
 
@@ -95,7 +105,8 @@ class GjsModule {
                .setSourceIsLazy(true);
 
         JS::RootedScript compiled_script(cx);
-        if (!JS::Compile(cx, options, script, script_len, &compiled_script))
+        if (!JS::CompileForNonSyntacticScope(cx, options, script, script_len,
+                                             &compiled_script))
             return false;
 
         JS::AutoObjectVector scope_chain(cx);
@@ -104,6 +115,14 @@ class GjsModule {
         if (!JS_ExecuteScript(cx, scope_chain, compiled_script, &ignored_retval))
             return false;
 
+        JS::RootedObject global(cx, JS::CurrentGlobalOrNull(cx));
+        JS::RootedObject lexical(cx);
+        if (!js::ExecuteInGlobalAndReturnScope(cx, global, compiled_script,
+                                               &lexical))
+            return false;
+
+        JS_SetReservedSlot(module, Slot::LEXICAL_ENV, JS::ObjectValue(*lexical));
+
         gjs_schedule_gc_if_needed(cx);
 
         GjsAutoJSChar n = module_name(cx, module);
@@ -142,9 +161,55 @@ class GjsModule {
 
     /* JSClass operations */
 
+    static bool
+    resolve(JSContext       *cx,
+            JS::HandleObject module,
+            JS::HandleId     id,
+            bool            *resolved)
+    {
+        JS::RootedObject lexical(cx, lexical_env(module));
+        if (!lexical) {
+            *resolved = false;
+            return true;  /* nothing imported yet */
+        }
+
+        if (!JS_HasPropertyById(cx, lexical, id, resolved))
+            return false;
+        if (*resolved) {
+            GjsAutoJSChar name(cx);
+            if (!gjs_get_string_id(cx, id, &name))
+                return false;
+
+            GjsAutoJSChar mod_name = module_name(cx, module);
+            g_warning("Some code accessed the property '%s' on the module "
+                      "'%s'. That property was defined with 'let' or 'const' "
+                      "inside the module. This was previously supported, but "
+                      "is not correct according to the ES6 standard. Any "
+                      "symbols to be exported from a module must be defined "
+                      "with 'var'. The property access will work as normal "
+                      "for compatibility for the time being, but please fix "
+                      "your code anyway.", name.get(), mod_name.get());
+
+            JS::Rooted<JS::PropertyDescriptor> desc(cx);
+            return JS_GetPropertyDescriptorById(cx, lexical, id, &desc) &&
+                JS_DefinePropertyById(cx, module, id, desc);
+        }
+        return true;
+    }
+
+    static constexpr JSClassOps class_ops = {
+        nullptr,  /* addProperty */
+        nullptr,  /* deleteProperty */
+        nullptr,  /* getProperty */
+        nullptr,  /* setProperty */
+        nullptr,  /* enumerate */
+        &GjsModule::resolve,
+    };
+
     static constexpr JSClass klass = {
         "GjsModule",
         JSCLASS_HAS_RESERVED_SLOTS(Slot::NSLOTS),
+        &GjsModule::class_ops,
     };
 
 public:
@@ -191,3 +256,4 @@ gjs_module_import(JSContext       *cx,
 }
 
 decltype(GjsModule::klass) constexpr GjsModule::klass;
+decltype(GjsModule::class_ops) constexpr GjsModule::class_ops;
diff --git a/installed-tests/js/testImporter.js b/installed-tests/js/testImporter.js
index cad4773..3b93546 100644
--- a/installed-tests/js/testImporter.js
+++ b/installed-tests/js/testImporter.js
@@ -163,6 +163,23 @@ describe('Importer', function () {
             LexicalScope = imports.lexicalScope;
         });
 
+        it('will log a compatibility warning when accessed', function () {
+            const GLib = imports.gi.GLib;
+            GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+                "Some code accessed the property 'b' on the module " +
+                "'lexicalScope'.*");
+            GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+                "Some code accessed the property 'c' on the module " +
+                "'lexicalScope'.*");
+
+            void LexicalScope.b;
+            void LexicalScope.c;
+
+            // g_test_assert_expected_messages() is a macro, not introspectable
+            GLib.test_assert_expected_messages_internal('Gjs',
+                'testImporter.js', 179, '');
+        });
+
         it('can be accessed', function () {
             expect(LexicalScope.a).toEqual(1);
             expect(LexicalScope.b).toEqual(2);