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

[Philip Chimento <pchimento src gnome org>]

commit b81eda642a6f7f323bca2a2d0afb085d966b8d07
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                     |   49 +++++++++++++++++++++++++++++++++++-
 installed-tests/js/testImporter.js |   17 ++++++++++++
 2 files changed, 65 insertions(+), 1 deletions(-)
---
diff --git a/gjs/module.cpp b/gjs/module.cpp
index 5028d06..6b03c96 100644
--- a/gjs/module.cpp
+++ b/gjs/module.cpp
@@ -138,6 +138,53 @@ class GjsModule {
 
     /* JSClass operations */
 
+    bool
+    resolve_impl(JSContext       *cx,
+                 JS::HandleObject module,
+                 JS::HandleId     id,
+                 bool            *resolved)
+    {
+        JS::RootedObject lexical(cx, JS_ExtensibleLexicalEnvironment(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)
@@ -151,7 +198,7 @@ class GjsModule {
         nullptr,  /* getProperty */
         nullptr,  /* setProperty */
         nullptr,  /* enumerate */
-        nullptr,  /* resolve */
+        &GjsModule::resolve,
         nullptr,  /* mayResolve */
         &GjsModule::finalize,
     };
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);