[gjs/306-too-much-recursion: 3/4] GObject: Allow omitting setters and getters



commit 56807a2d3d70b3c429d74a5139ebc6c0bdca5c3c
Author: Philip Chimento <philip chimento gmail com>
Date:   Wed Mar 11 22:17:51 2020 -0700

    GObject: Allow omitting setters and getters
    
    In the case where you don't need anything special from your GObject
    property getters and setters, just correct handling of notify signals
    and default values, it's now possible to leave out the getters and
    setters, and GJS will just do the right thing. This is convenient for
    JS code authors as well as avoiding problems caused by missing getters
    and setters.
    
    This only works for read-write properties. If a read-only or write-only
    property is missing its accessor, then one is generated which throws.
    
    If the property name consists of multiple words, then accessors will be
    generated for all three variants: kebab-case, snake_case, and camelCase.
    Additionally, if you do provide accessors for one of these, then they
    will be copied to the other two variants.
    
    See: #306

 doc/Mapping.md                          |  19 ++--
 examples/gtk-application.js             |  17 +---
 installed-tests/js/testGObjectClass.js  | 158 ++++++++++++++++++++++++++++++++
 installed-tests/js/testLegacyGObject.js |  15 +++
 js.gresource.xml                        |   1 +
 modules/core/_common.js                 | 107 +++++++++++++++++++++
 modules/core/overrides/GObject.js       |   3 +
 modules/script/_legacy.js               |   3 +
 8 files changed, 297 insertions(+), 26 deletions(-)
---
diff --git a/doc/Mapping.md b/doc/Mapping.md
index d1b08051..3b2b0613 100644
--- a/doc/Mapping.md
+++ b/doc/Mapping.md
@@ -113,29 +113,28 @@ var MyLabel = GObject.registerClass({
             'ExampleProperty',                  // nickname
             'An example read write property',   // description
             GObject.ParamFlags.READWRITE,       // READABLE/READWRITE/CONSTRUCT/etc
-            ''                                  // implement defaults manually
+            'A default'                         // default value
         )
     }
 }, class MyLabel extends Gtk.Label {
-    _init(params) {
-        super._init(params);
-    }
-
     get example_prop() {
-        if (this._example_prop === undefined) {
+        if (!('_example_prop' in this)
             return 'A default';
-        }
-
         return this._example_prop;
     }
 
     set example_prop(value) {
-        this._example_prop = value;
-        this.notify('example-prop');
+        if (this._example_prop !== value) {
+            this._example_prop = value;
+            this.notify('example-prop');
+        }
     }
 });
 ```
 
+If you just want a simple property that you can get change notifications from, you can leave out the getter 
and setter and GJS will attempt to do the right thing.
+However, if you define one, you have to define both (unless the property is read-only or write-only).
+
 ## Signals
 
 Every object inherited from GObject has `connect()`, `connect_after()`, `disconnect()` and `emit()` methods.
diff --git a/examples/gtk-application.js b/examples/gtk-application.js
index 483a5ea5..514b8bdd 100644
--- a/examples/gtk-application.js
+++ b/examples/gtk-application.js
@@ -20,7 +20,7 @@ var ExampleApplication = GObject.registerClass({
             'ExampleProperty',                  // nickname
             'An example read write property',   // description
             GObject.ParamFlags.READWRITE,       // read/write/construct...
-            '',                                 // implement defaults manually
+            'a default value',
         ),
     },
     Signals: {'examplesig': {param_types: [GObject.TYPE_INT]}},
@@ -32,21 +32,6 @@ var ExampleApplication = GObject.registerClass({
         });
     }
 
-    // Example property getter/setter
-    get exampleprop() {
-        if (typeof this._exampleprop === 'undefined')
-            return 'a default value';
-
-        return this._exampleprop;
-    }
-
-    set exampleprop(value) {
-        this._exampleprop = value;
-
-        // notify() has to be called, if you want it
-        this.notify('exampleprop');
-    }
-
     // Example signal emission
     emitExamplesig(number) {
         this.emit('examplesig', number);
diff --git a/installed-tests/js/testGObjectClass.js b/installed-tests/js/testGObjectClass.js
index 0d80ed38..298559db 100644
--- a/installed-tests/js/testGObjectClass.js
+++ b/installed-tests/js/testGObjectClass.js
@@ -377,6 +377,19 @@ describe('GObject class with decorator', function () {
         }, class BadOverride extends GObject.Object {})).toThrow();
     });
 
+    it('handles gracefully overriding a C property but forgetting the accessors', function () {
+        // This is a random interface in Gio with a read-write property
+        const ForgottenAccessors = GObject.registerClass({
+            Implements: [Gio.TlsFileDatabase],
+            Properties: {
+                'anchors': GObject.ParamSpec.override('anchors', Gio.TlsFileDatabase),
+            },
+        }, class ForgottenAccessors extends Gio.TlsDatabase {});
+        const obj = new ForgottenAccessors();
+        expect(obj.anchors).toBeNull();  // the property's default value
+        obj.anchors = 'foo';
+    });
+
     it('does not pollute the wrong prototype with GObject properties', function () {
         const MyCustomCharset = GObject.registerClass(class MyCustomCharset extends Gio.CharsetConverter {
             _init() {
@@ -702,3 +715,148 @@ describe('Signal handler matching', function () {
         expect(() => GObject.signal_handlers_disconnect_by_data(o, null)).toThrow();
     });
 });
+
+describe('Auto accessor generation', function () {
+    const AutoAccessors = GObject.registerClass({
+        Properties: {
+            'simple': GObject.ParamSpec.int('simple', 'Simple', 'Short-named property',
+                GObject.ParamFlags.READWRITE, 0, 100, 24),
+            'long-long-name': GObject.ParamSpec.int('long-long-name', 'Long long name',
+                'Long-named property', GObject.ParamFlags.READWRITE, 0, 100, 48),
+            'construct': GObject.ParamSpec.int('construct', 'Construct', 'Construct',
+                GObject.ParamFlags.READWRITE | GObject.ParamFlags.CONSTRUCT, 0, 100, 96),
+            'snake-name': GObject.ParamSpec.int('snake-name', 'Snake name',
+                'Snake-cased property', GObject.ParamFlags.READWRITE, 0, 100, 36),
+            'camel-name': GObject.ParamSpec.int('camel-name', 'Camel name',
+                'Camel-cased property', GObject.ParamFlags.READWRITE, 0, 100, 72),
+            'kebab-name': GObject.ParamSpec.int('kebab-name', 'Kebab name',
+                'Kebab-cased property', GObject.ParamFlags.READWRITE, 0, 100, 12),
+            'readonly': GObject.ParamSpec.int('readonly', 'Readonly', 'Readonly property',
+                GObject.ParamFlags.READABLE, 0, 100, 54),
+            'writeonly': GObject.ParamSpec.int('writeonly', 'Writeonly',
+                'Writeonly property', GObject.ParamFlags.WRITABLE, 0, 100, 60),
+            'missing-getter': GObject.ParamSpec.int('missing-getter', 'Missing getter',
+                'Missing a getter', GObject.ParamFlags.READWRITE, 0, 100, 18),
+            'missing-setter': GObject.ParamSpec.int('missing-setter', 'Missing setter',
+                'Missing a setter', GObject.ParamFlags.READWRITE, 0, 100, 42),
+        },
+    }, class AutoAccessors extends GObject.Object {
+        _init(props = {}) {
+            super._init(props);
+            this._snakeNameGetterCalled = 0;
+            this._snakeNameSetterCalled = 0;
+            this._camelNameGetterCalled = 0;
+            this._camelNameSetterCalled = 0;
+            this._kebabNameGetterCalled = 0;
+            this._kebabNameSetterCalled = 0;
+        }
+
+        get snake_name() {
+            this._snakeNameGetterCalled++;
+            return 42;
+        }
+
+        set snake_name(value) {
+            this._snakeNameSetterCalled++;
+        }
+
+        get camelName() {
+            this._camelNameGetterCalled++;
+            return 42;
+        }
+
+        set camelName(value) {
+            this._camelNameSetterCalled++;
+        }
+
+        get ['kebab-name']() {
+            this._kebabNameGetterCalled++;
+            return 42;
+        }
+
+        set ['kebab-name'](value) {
+            this._kebabNameSetterCalled++;
+        }
+
+        set missing_getter(value) {
+            this._missingGetter = value;
+        }
+
+        get missing_setter() {
+            return 42;
+        }
+    });
+
+    let a;
+    beforeEach(function () {
+        a = new AutoAccessors();
+    });
+
+    it('get and set the property', function () {
+        a.simple = 1;
+        expect(a.simple).toEqual(1);
+        a['long-long-name'] = 1;
+        expect(a['long-long-name']).toEqual(1);
+        a.construct = 1;
+        expect(a.construct).toEqual(1);
+    });
+
+    it("initial value is the param spec's default value", function () {
+        expect(a.simple).toEqual(24);
+        expect(a['long-long-name']).toEqual(48);
+        expect(a.construct).toEqual(96);
+    });
+
+    it('notify when the property changes', function () {
+        const notify = jasmine.createSpy('notify');
+        a.connect('notify::simple', notify);
+        a.simple = 1;
+        expect(notify).toHaveBeenCalledTimes(1);
+        notify.calls.reset();
+        a.simple = 1;
+        expect(notify).not.toHaveBeenCalled();
+    });
+
+    it('copies accessors for camel and kebab if snake accessors given', function () {
+        a.snakeName = 42;
+        expect(a.snakeName).toEqual(42);
+        a['snake-name'] = 42;
+        expect(a['snake-name']).toEqual(42);
+        expect(a._snakeNameGetterCalled).toEqual(2);
+        expect(a._snakeNameSetterCalled).toEqual(2);
+    });
+
+    it('copies accessors for snake and kebab if camel accessors given', function () {
+        a.camel_name = 42;
+        expect(a.camel_name).toEqual(42);
+        a['camel-name'] = 42;
+        expect(a['camel-name']).toEqual(42);
+        expect(a._camelNameGetterCalled).toEqual(2);
+        expect(a._camelNameSetterCalled).toEqual(2);
+    });
+
+    it('copies accessors for snake and camel if kebab accessors given', function () {
+        a.kebabName = 42;
+        expect(a.kebabName).toEqual(42);
+        a.kebab_name = 42;
+        expect(a.kebab_name).toEqual(42);
+        expect(a._kebabNameGetterCalled).toEqual(2);
+        expect(a._kebabNameSetterCalled).toEqual(2);
+    });
+
+    it('readonly getter throws', function () {
+        expect(() => a.readonly).toThrowError(/getter/);
+    });
+
+    it('writeonly setter throws', function () {
+        expect(() => (a.writeonly = 1)).toThrowError(/setter/);
+    });
+
+    it('getter throws when setter defined', function () {
+        expect(() => a.missingGetter).toThrowError(/getter/);
+    });
+
+    it('setter throws when getter defined', function () {
+        expect(() => (a.missingSetter = 1)).toThrowError(/setter/);
+    });
+});
diff --git a/installed-tests/js/testLegacyGObject.js b/installed-tests/js/testLegacyGObject.js
index f30bc906..64c164fb 100644
--- a/installed-tests/js/testLegacyGObject.js
+++ b/installed-tests/js/testLegacyGObject.js
@@ -381,6 +381,21 @@ describe('GObject class', function () {
             },
         })).toThrow();
     });
+
+    it('handles gracefully overriding a C property but forgetting the accessors', function () {
+        // This is a random interface in Gio with a read-write property
+        const ForgottenAccessors = new Lang.Class({
+            Name: 'ForgottenAccessors',
+            Extends: Gio.TlsDatabase,
+            Implements: [Gio.TlsFileDatabase],
+            Properties: {
+                'anchors': GObject.ParamSpec.override('anchors', Gio.TlsFileDatabase),
+            },
+        });
+        const obj = new ForgottenAccessors();
+        expect(obj.anchors).toBeNull();
+        obj.anchors = 'foo';
+    });
 });
 
 const AnInterface = new Lang.Interface({
diff --git a/js.gresource.xml b/js.gresource.xml
index 6e142169..f0c3b934 100644
--- a/js.gresource.xml
+++ b/js.gresource.xml
@@ -29,6 +29,7 @@
     <file>modules/core/overrides/Gtk.js</file>
 
     <file>modules/core/_cairo.js</file>
+    <file>modules/core/_common.js</file>
     <file>modules/core/_format.js</file>
     <file>modules/core/_gettext.js</file>
     <file>modules/core/_signals.js</file>
diff --git a/modules/core/_common.js b/modules/core/_common.js
new file mode 100644
index 00000000..73edb05c
--- /dev/null
+++ b/modules/core/_common.js
@@ -0,0 +1,107 @@
+// -*- mode: js; js-indent-level: 4; indent-tabs-mode: nil -*-
+
+// Copyright 2020 Philip Chimento <philip chimento gmail com>
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to
+// deal in the Software without restriction, including without limitation the
+// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+// sell copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+// IN THE SOFTWARE.
+
+/* exported _checkAccessors */
+
+// This is a helper module in which to put code that is common between the
+// legacy GObject.Class system and the new GObject.registerClass system.
+
+function _generateAccessors(pspec, propdesc, GObject) {
+    const {name, flags} = pspec;
+    const readable = flags & GObject.ParamFlags.READABLE;
+    const writable = flags & GObject.ParamFlags.WRITABLE;
+
+    if (!propdesc) {
+        propdesc = {
+            configurable: true,
+            enumerable: true,
+        };
+    }
+
+    if (readable && writable) {
+        if (!propdesc.get && !propdesc.set) {
+            const privateName = Symbol(`__autogeneratedAccessor__${name}`);
+            const defaultValue = pspec.get_default_value();
+            propdesc.get = function () {
+                if (!(privateName in this))
+                    this[privateName] = defaultValue;
+                return this[privateName];
+            };
+            propdesc.set = function (value) {
+                if (value !== this[privateName]) {
+                    this[privateName] = value;
+                    this.notify(name);
+                }
+            };
+        } else if (!propdesc.get) {
+            propdesc.get = function () {
+                throw new Error(`setter defined without getter for property ${name}`);
+            };
+        } else if (!propdesc.set) {
+            propdesc.set = function () {
+                throw new Error(`getter defined without setter for property ${name}`);
+            };
+        }
+    } else if (readable && !propdesc.get) {
+        propdesc.get = function () {
+            throw new Error(`missing getter for read-only property ${name}`);
+        };
+    } else if (writable && !propdesc.set) {
+        propdesc.set = function () {
+            throw new Error(`missing setter for write-only property ${name}`);
+        };
+    }
+
+    return propdesc;
+}
+
+function _checkAccessors(proto, pspec, GObject) {
+    const {name, flags} = pspec;
+
+    const underscoreName = name.replace(/-/g, '_');
+    const camelName = name.replace(/-([a-z])/g, match => match[1].toUpperCase());
+    let propdesc = Object.getOwnPropertyDescriptor(proto, name);
+    let dashPropdesc = propdesc, underscorePropdesc, camelPropdesc;
+    const nameIsCompound = name.includes('-');
+    if (nameIsCompound) {
+        underscorePropdesc = Object.getOwnPropertyDescriptor(proto, underscoreName);
+        camelPropdesc = Object.getOwnPropertyDescriptor(proto, camelName);
+        if (!propdesc)
+            propdesc = underscorePropdesc;
+        if (!propdesc)
+            propdesc = camelPropdesc;
+    }
+
+    const readable = flags & GObject.ParamFlags.READABLE;
+    const writable = flags & GObject.ParamFlags.WRITABLE;
+    if (!propdesc || (readable && !propdesc.get) || (writable && !propdesc.set))
+        propdesc = _generateAccessors(pspec, propdesc, GObject);
+
+    if (!dashPropdesc)
+        Object.defineProperty(proto, name, propdesc);
+    if (nameIsCompound) {
+        if (!underscorePropdesc)
+            Object.defineProperty(proto, underscoreName, propdesc);
+        if (!camelPropdesc)
+            Object.defineProperty(proto, camelName, propdesc);
+    }
+}
diff --git a/modules/core/overrides/GObject.js b/modules/core/overrides/GObject.js
index a5747c21..166b7aac 100644
--- a/modules/core/overrides/GObject.js
+++ b/modules/core/overrides/GObject.js
@@ -22,6 +22,7 @@
 
 const Gi = imports._gi;
 const GjsPrivate = imports.gi.GjsPrivate;
+const {_checkAccessors} = imports._common;
 const Legacy = imports._legacy;
 
 let GObject;
@@ -445,6 +446,8 @@ function _init() {
         let parent = Object.getPrototypeOf(klass);
         let gobjectSignals = klass.hasOwnProperty(signals) ? klass[signals] : [];
 
+        propertiesArray.forEach(pspec => _checkAccessors(klass.prototype, pspec, GObject));
+
         let newClass = Gi.register_type(parent.prototype, gtypename, gflags,
             gobjectInterfaces, propertiesArray);
         Object.setPrototypeOf(newClass, parent);
diff --git a/modules/script/_legacy.js b/modules/script/_legacy.js
index 86970fac..46c3b4c7 100644
--- a/modules/script/_legacy.js
+++ b/modules/script/_legacy.js
@@ -411,6 +411,7 @@ Interface.prototype._init = function (params) {
 
 function defineGObjectLegacyObjects(GObject) {
     const Gi = imports._gi;
+    const {_checkAccessors} = imports._common;
 
     // Some common functions between GObject.Class and GObject.Interface
 
@@ -528,6 +529,8 @@ function defineGObjectLegacyObjects(GObject) {
             let propertiesArray = _propertiesAsArray(params);
             delete params.Properties;
 
+            propertiesArray.forEach(pspec => _checkAccessors(params, pspec, GObject));
+
             let newClass = Gi.register_type(parent.prototype, gtypename,
                 gflags, gobjectInterfaces, propertiesArray);
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]