diff --git a/TODO b/TODO index 0c9e606..db9997a 100644 --- a/TODO +++ b/TODO @@ -8,6 +8,9 @@ - Disallow member redeclaration in definition - Permit binding on class methods - Provide ability to free class from memory (class data stored in internal vars) + - Move tests to directly test propobj + - Was never done after refactoring. It's tested as a consequence of being + used in the class module. Member Keywords - Restrictions; throw exceptions when unknown keywords are used diff --git a/lib/class.js b/lib/class.js index 3aae42e..a081056 100644 --- a/lib/class.js +++ b/lib/class.js @@ -39,10 +39,23 @@ var class_meta = {}; /** * Stores class instance visibility object * - * For each instance id, an object exists that contains the private and - * protected members. + * An entry in this table exists for each instance, with the instance id (iid) + * as the key. For each instance, there is always a base. The base will contain + * a proxy to the public members on the instance itself. The base will also + * contain all protected members. * - * @type {Object.} + * Atop the base object is a private member object, with the base as its + * prototype. There exists a private member object for the instance itself and + * one for each supertype. This is stored by the class id (cid) as the key. This + * permits the private member object associated with the class of the method + * call to be bound to that method. For example, if a parent method is called, + * that call must be invoked in the context of the parent, so the private + * members of the parent must be made available. + * + * The resulting structure looks something like this: + * class_instance = { iid: { cid: {} } } + * + * @type {Object.>} */ var class_instance = {}; diff --git a/lib/propobj.js b/lib/propobj.js index 7ea372d..38d59e6 100644 --- a/lib/propobj.js +++ b/lib/propobj.js @@ -46,12 +46,17 @@ var util = require( './util' ), */ exports.setup = function( dest, properties, methods ) { + var obj = dest; + // this constructor is an extra layer atop of the destination object, which // will contain the private methods - var obj_ctor = function() {}; - obj_ctor.prototype = dest; + if ( defprop ) + { + var obj_ctor = function() {}; + obj_ctor.prototype = dest; - var obj = new obj_ctor(); + obj = new obj_ctor(); + } // initialize each of the properties for this instance to // ensure we're not sharing references to prototype values diff --git a/test/test-class-visibility.js b/test/test-class-visibility.js index 5c82bf2..72c72a3 100644 --- a/test/test-class-visibility.js +++ b/test/test-class-visibility.js @@ -54,6 +54,15 @@ var common = require( './common' ), }, + /** + * Does the same as the above, but we won't override this one + */ + 'public nonOverrideGetProp': function( name ) + { + return this[ name ]; + }, + + /** * Allows us to set a value from within the class */ @@ -116,6 +125,9 @@ var common = require( './common' ), // protected/private properties (for testing purposes) return this[ name ]; }, + + + 'private myOwnPrivateFoo': function() {}, }), sub_foo = SubFoo(), @@ -442,3 +454,52 @@ var common = require( './common' ), ); } )(); + +/** + * When a parent method is invoked, the parent should not be given access to the + * private members of the invoking subtype. Why? + * + * This is not a matter of whether or not this is possible to do. In fact it's + * relatively simple to implement. The issue is whether or not it makes sense. + * Consider a compiled language. Let's say Foo and SubFoo (as defined in this + * test case) were written in C++. Should Foo have access to a private property + * on SubFoo when it is overridden? + * + * No - that doesn't make sense. The private member is not a member of Foo and + * therefore Foo would fail to even compile. Alright, but we don't have such a + * restriction in our case. So why not implement it? + * + * Proponents of such an implementation are likely thinking of the act of + * inheriting methods as a copy/paste type of scenario. If we inherit public + * method baz(), and it were a copy/paste type of situation, then surely baz() + * would have access to all of SubFoo's private members. But that is not the + * case. Should baz() be defined as a member of Foo, then its scope is + * restricted to Foo and its supertypes. That is not how OO works. It is /not/ + * copy/paste. It is inheriting functionality. + */ +( function testParentsShouldNotHaveAccessToPrivateMembersOfSubtypes() +{ + // browsers that do not support the property proxy will not support + // encapsulating properties + if ( !( propobj.supportsPropProxy() ) ) + { + return; + } + + // property + assert.equal( + sub_foo.nonOverrideGetProp( '_pfoo' ), + undefined, + "Parent should not have access to private properties of subtype when " + + "a parent method is invoked" + ); + + // member + assert.equal( + sub_foo.nonOverrideGetProp( '_myOwnPrivateFoo' ), + undefined, + "Parent should not have access to private methods of subtype when " + + "a parent method is invoked" + ); +} )(); + diff --git a/test/test-combine-pre-es5.js b/test/test-combine-pre-es5.js new file mode 100644 index 0000000..cbca10e --- /dev/null +++ b/test/test-combine-pre-es5.js @@ -0,0 +1,77 @@ +/** + * Tests combined file, attempting to emulate a pre-ECMAScript5 environment. + * This will ensure fallbacks will work properly on older browsers, such as IE6. + * + * This is /not/ an alternative to running the test suite in the browser of your + * choice. It is intended to catch errors early, to ensure bugs are not + * committed between browser tests. + * + * Copyright (C) 2010 Mike Gerwitz + * + * This file is part of ease.js. + * + * ease.js is free software: you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * @author Mike Gerwitz + * @package test + */ + +var common = require( './common' ), + assert = require( 'assert' ), + Class = common.require( 'class' ), + Script = process.binding( 'evals' ).Script, + + // sandbox in which combined script will be run + sandbox = { + // stub document.write() so we don't blow up + document: { write: function() {} }, + }; + + +var file = 'ease-full.js'; + +// attempt to read the combined file +try +{ + var data = require( 'fs' ) + .readFileSync( ( __dirname + '/../build/' + file ), 'ascii' ); +} +catch ( e ) +{ + // if the file doesn't exit, just skip the test + console.log( + "Combined file not found. Test skipped. Please run `make combined`." + ); + process.exit( 0 ); +} + +// Let's take this bitch back in time (this is not a complete list, but +// satisfies what we need). +// +// It is important to note that we prepend this to the script that we'll be +// executing, because the script will be executed within a new scope. Any +// clobbering we do in our scope will not affect it, nor will any clobbering we +// do to it affect us. +data = "delete Object.defineProperty;" + + "delete Array.prototype.forEach;" + + data +; + +// run the script (if this fails to compile, the generated code is invalid) +var cmb_script = new Script( data ); +cmb_script.runInNewContext( sandbox ); + +// cross your fingers +sandbox.easejs.runTests(); + diff --git a/tools/combine b/tools/combine index db82305..79b49b8 100755 --- a/tools/combine +++ b/tools/combine @@ -103,7 +103,7 @@ if [ "$INC_TEST" ]; then \) \ -exec basename {} \; \ | sort \ - | grep -v 'test-\(combine\|index\).js' \ + | grep -v 'test-\(combine\(-pre-es5\)\?\|index\).js' \ ) # include test combine template