js: Allow access to modules' lexical scopewip/ptomato/mozjs52

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

2 files changed, 65 insertions, 1 deletions

diff --git a/gjs/module.cpp b/gjs/module.cpp
index 5028d06f..6b03c967 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 35879b1a..e2165619 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);