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

[Philip Chimento <pchimento src gnome org>]

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

    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.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784196

 gjs/module.cpp                     |   96 +++++++++++++++++++++++++++++++++++-
 installed-tests/js/testImporter.js |   17 ++++++
 2 files changed, 111 insertions(+), 2 deletions(-)
---
diff --git a/gjs/module.cpp b/gjs/module.cpp
index d09ab04..a31dea0 100644
--- a/gjs/module.cpp
+++ b/gjs/module.cpp
@@ -23,6 +23,7 @@
 
 #include <gio/gio.h>
 
+#include "global.h"
 #include "jsapi-private.h"
 #include "jsapi-util.h"
 #include "jsapi-wrapper.h"
@@ -34,6 +35,7 @@ class GjsModule {
 
     enum Slot {
         NAME,
+        LEXICAL_ENV,
         NSLOTS
     };
 
@@ -49,6 +51,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 +69,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;
     }
 
@@ -80,6 +91,35 @@ class GjsModule {
         return true;
     }
 
+    static bool
+    obtain_lexical_scope(JSContext       *cx,
+                         JS::HandleObject module,
+                         JS::HandleScript script)
+    {
+        JS::RootedObject global(cx, gjs_create_global_object(cx));
+        if (!global)
+            return false;
+
+        JS::RootedObject lexical(cx);
+        {
+            JSAutoCompartment ac(cx, global);
+            if (!gjs_define_global_properties(cx, global))
+                return false;
+
+            JS::RootedValue ignored_retval(cx);
+            if (!JS::CloneAndExecuteScript(cx, script, &ignored_retval))
+                return false;
+
+            lexical = JS_GlobalLexicalEnvironment(global);
+        }
+
+        if (!JS_WrapObject(cx, &lexical))
+            return false;
+
+        JS_SetReservedSlot(module, Slot::LEXICAL_ENV, JS::ObjectValue(*lexical));
+        return true;
+    }
+
     /* Carries out the actual execution of the module code */
     static bool
     evaluate_import(JSContext       *cx,
@@ -95,13 +135,15 @@ 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);
         scope_chain.append(module);
         JS::RootedValue ignored_retval(cx);
-        if (!JS_ExecuteScript(cx, scope_chain, compiled_script, &ignored_retval))
+        if (!JS_ExecuteScript(cx, scope_chain, compiled_script, &ignored_retval) ||
+            !obtain_lexical_scope(cx, module, compiled_script))
             return false;
 
         gjs_schedule_gc_if_needed(cx);
@@ -142,9 +184,58 @@ 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)
+            return true;
+
+        /* The property is present in the lexical environment. This should not
+         * be supported according to ES6. For compatibility with earlier GJS,
+         * we treat it as if it were a real property, but warn about it. */
+
+        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 previously 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);
+    }
+
+    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 +282,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);