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

[Philip Chimento <pchimento src gnome org>]

commit df21913bdcf27e36d044f17be31918dbb0538b12
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                     |  121 +++++++++++++++++++++++++++++++++--
 installed-tests/js/testImporter.js |   17 +++++
 2 files changed, 131 insertions(+), 7 deletions(-)
---
diff --git a/gjs/module.cpp b/gjs/module.cpp
index e58206b..a26d2a1 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"
@@ -44,12 +45,25 @@ class GjsModule {
 
     /* Private data accessors */
 
+    enum Slot {
+        LEXICAL_ENV,
+        NSLOTS
+    };
+
     static inline GjsModule *
     priv(JSObject *module)
     {
         return static_cast<GjsModule *>(JS_GetPrivate(module));
     }
 
+    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,
@@ -57,6 +71,7 @@ class GjsModule {
     {
         JSObject *module = JS_NewObject(cx, &GjsModule::klass);
         JS_SetPrivate(module, new GjsModule(name));
+        JS_SetReservedSlot(module, Slot::LEXICAL_ENV, JS::NullValue());
         return module;
     }
 
@@ -77,6 +92,25 @@ class GjsModule {
         return true;
     }
 
+    bool
+    obtain_lexical_scope(JSContext       *cx,
+                         JS::HandleObject module,
+                         JS::HandleObject global)
+    {
+        gjs_debug(GJS_DEBUG_IMPORTER, "Importing lexical environment of module "
+                  "%s for compatibility", m_name);
+
+        JS::RootedObject lexical(cx, JS_GlobalLexicalEnvironment(global));
+
+        JSAutoCompartment ac(cx, module);
+
+        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 */
     bool
     evaluate_import(JSContext       *cx,
@@ -86,19 +120,39 @@ class GjsModule {
                     const char      *filename,
                     int              line_number)
     {
+        JS::RootedObject outer_global(cx, gjs_get_import_global(cx));
+        JS::RootedObject global(cx, gjs_create_global_object(cx));
+        if (!global)
+            return false;
+
+        /* Copy any properties from the global object over to the module-
+         * specific global object, that client code might have defined
+         * before the import. This also makes it unnecessary to call
+         * gjs_define_global_properties(). */
+        if (!JS_CopyPropertiesFrom(cx, global, outer_global))
+            return false;
+
+        JSAutoCompartment ac(cx, global);
+
+        JS::RootedObject wrapped_module(cx, module);
+        if (!JS_WrapObject(cx, &wrapped_module))
+            return false;
+
         JS::CompileOptions options(cx);
         options.setUTF8(true)
                .setFileAndLine(filename, line_number)
                .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);
+        scope_chain.append(wrapped_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, global))
             return false;
 
         gjs_schedule_gc_if_needed(cx);
@@ -138,6 +192,53 @@ class GjsModule {
 
     /* JSClass operations */
 
+    bool
+    resolve_impl(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 prop_name(cx);
+        if (!gjs_get_string_id(cx, id, &prop_name))
+            return false;
+
+        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.", prop_name.get(), m_name);
+
+        JS::Rooted<JS::PropertyDescriptor> desc(cx);
+        return JS_GetPropertyDescriptorById(cx, lexical, id, &desc) &&
+            JS_DefinePropertyById(cx, module, id, desc);
+    }
+
+    static bool
+    resolve(JSContext       *cx,
+            JS::HandleObject module,
+            JS::HandleId     id,
+            bool            *resolved)
+    {
+        return priv(module)->resolve_impl(cx, module, id, resolved);
+    }
+
     static void
     finalize(JSFreeOp *op,
              JSObject *module)
@@ -145,19 +246,24 @@ class GjsModule {
         delete priv(module);
     }
 
-    static constexpr JSClass klass = {
-        "GjsModule",
-        JSCLASS_HAS_PRIVATE | JSCLASS_BACKGROUND_FINALIZE,
+    static constexpr JSClassOps class_ops = {
         nullptr,  /* addProperty */
         nullptr,  /* deleteProperty */
         nullptr,  /* getProperty */
         nullptr,  /* setProperty */
         nullptr,  /* enumerate */
-        nullptr,  /* resolve */
+        &GjsModule::resolve,
         nullptr,  /* mayResolve */
         &GjsModule::finalize,
     };
 
+    static constexpr JSClass klass = {
+        "GjsModule",
+        JSCLASS_HAS_PRIVATE | JSCLASS_BACKGROUND_FINALIZE |
+        JSCLASS_HAS_RESERVED_SLOTS(Slot::NSLOTS),
+        &GjsModule::class_ops,
+    };
+
 public:
 
     /* Carries out the import operation */
@@ -205,3 +311,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 35879b1..e216561 100644
--- a/installed-tests/js/testImporter.js
+++ b/installed-tests/js/testImporter.js
@@ -164,6 +164,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);