Daniel Werner has uploaded a new change for review.
https://gerrit.wikimedia.org/r/50185
Change subject: Gives names to all important constructors for improved
debugging experience
......................................................................
Gives names to all important constructors for improved debugging experience
Change-Id: I3f15c65b3225eba12fd51d0ad814c1630e1ee2e7
---
M DataTypes/resources/dataTypes.js
M DataValues/resources/DataValue.js
M DataValues/resources/dataValues.js
M DataValues/resources/dataValues.util.js
M DataValues/resources/values/BoolValue.js
M DataValues/resources/values/MonolingualTextValue.js
M DataValues/resources/values/MultilingualTextValue.js
M DataValues/resources/values/NumberValue.js
M DataValues/resources/values/StringValue.js
M DataValues/resources/values/UnknownValue.js
M DataValues/tests/qunit/dataValues.util.inherit.tests.js
M ValueParsers/resources/Api.js
M ValueParsers/resources/ApiBasedValueParser.js
M ValueParsers/resources/ValueParser.js
M ValueParsers/resources/ValueParserFactory.js
15 files changed, 114 insertions(+), 39 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DataValues
refs/changes/85/50185/1
diff --git a/DataTypes/resources/dataTypes.js b/DataTypes/resources/dataTypes.js
index e0a4958..28d772a 100644
--- a/DataTypes/resources/dataTypes.js
+++ b/DataTypes/resources/dataTypes.js
@@ -15,7 +15,7 @@
* @since 0.1
* @type Object
*/
-var dataTypes = new ( function( $, mw, undefined ) {
+var dataTypes = new ( function Dt( $, mw ) {
'use strict';
// TODO: the whole structure of this is a little weird, perhaps there
should be a
@@ -37,7 +37,7 @@
* @param {Object} formatter
* @param {Object} validators
*/
- dt.DataType = function( typeId, dataValueType, parser, formatter,
validators ) {
+ dt.DataType = function DtDataType( typeId, dataValueType, parser,
formatter, validators ) {
if( dataValueType === undefined ) {
throw new Error( 'All arguments must be provided for
creating a new DataType object' );
}
diff --git a/DataValues/resources/DataValue.js
b/DataValues/resources/DataValue.js
index 7c42a50..164d3a7 100644
--- a/DataValues/resources/DataValue.js
+++ b/DataValues/resources/DataValue.js
@@ -15,7 +15,7 @@
* @abstract
* @since 0.1
*/
-dv.DataValue = function() {};
+dv.DataValue = function DvDataValue() {};
dv.DataValue.prototype = {
/**
diff --git a/DataValues/resources/dataValues.js
b/DataValues/resources/dataValues.js
index 6422367..6886a22 100644
--- a/DataValues/resources/dataValues.js
+++ b/DataValues/resources/dataValues.js
@@ -14,7 +14,7 @@
* @since 0.1
* @type Object
*/
-var dataValues = new( function( $, undefined ) {
+var dataValues = new( function Dv( $ ) {
'use strict';
var dvs = [];
diff --git a/DataValues/resources/dataValues.util.js
b/DataValues/resources/dataValues.util.js
index 8788167..d0f47c7 100644
--- a/DataValues/resources/dataValues.util.js
+++ b/DataValues/resources/dataValues.util.js
@@ -2,9 +2,9 @@
* @file
* @ingroup DataValues
* @licence GNU GPL v2+
- * @author Daniel Werner
+ * @author Daniel Werner < [email protected] >
*/
-( function( $, dv ) {
+( function ( $, dv ) {
'use strict';
/**
@@ -18,16 +18,25 @@
* Helper for prototypical inheritance.
* @since 0.1
*
- * @param {Function} base Constructor which will be used for the
prototype chain.
+ * @param {string} name (optional) The name of the new constructor.
This is handy for debugging
+ * purposes since instances of the constructor might be
displayed under that name.
+ * @param {Function} base Constructor which will be used for the
prototype chain. This function
+ * will not be the constructor returned by the function but will
be called by it.
* @param {Function} [constructor] for overwriting base constructor.
Can be omitted.
* @param {Object} [members] properties overwriting or extending those
of the base.
* @return Function Constructor of the new, extended type.
+ *
+ * @throws {Error} In case a malicious function name is given or a
reserved word is used
*/
- dv.util.inherit = function( base, constructor, members ) {
- // allow to omit constructor since it can be inherited
directly. But if given, require it as
- // second parameter for readability. If no constructor, second
parameter is the prototype
- // extension object.
- if( members === undefined ) {
+ dv.util.inherit = function( name, base, constructor, members ) {
+ // the name is optional
+ if( typeof name !== 'string' ) {
+ members = constructor; constructor = base; base = name;
name = false;
+ }
+
+ // allow to omit constructor since it can be inherited
directly. But if given, require it as second parameter
+ // for readability. If no constructor, second parameter is the
prototype extension object.
+ if( !members ) {
if( $.isFunction( constructor ) ) {
members = {};
} else {
@@ -35,7 +44,22 @@
constructor = false;
}
}
- var NewConstructor = constructor || function() { base.apply(
this, arguments ); };
+ // if no name is given, find suitable constructor's name
+ name = name || constructor.name || ( base.name ? base.name +
'_SubProto' : 'SomeInherited' );
+ // make sure name is just a function name and not some
executable JavaScript
+ name = name.replace( /(?:(^\d+)|[^\w$])/ig, '' );
+
+ if( !name ) { // only bad characters were in the name!
+ throw new Error( 'Bad constructor name given. Only word
characters and $ are allowed.' );
+ }
+
+ // function we execute in our real constructor created by evil
eval:
+ var evilsSeed = constructor || base,
+ NewConstructor;
+
+ // for creating a named function with a variable name, there is
just no other way...
+ eval( 'NewConstructor = function ' + name +
+ '(){ evilsSeed.apply( this, arguments ); }' );
var NewPrototype = function(){}; // new constructor for
avoiding base constructor and with it any side-effects
NewPrototype.prototype = base.prototype;
diff --git a/DataValues/resources/values/BoolValue.js
b/DataValues/resources/values/BoolValue.js
index 1af21b9..462a325 100644
--- a/DataValues/resources/values/BoolValue.js
+++ b/DataValues/resources/values/BoolValue.js
@@ -24,7 +24,7 @@
*
* @param {String} value
*/
-dv.BoolValue = dv.util.inherit( PARENT, constructor, {
+dv.BoolValue = dv.util.inherit( 'DvBoolValue', PARENT, constructor, {
/**
* @see dv.DataValue.getSortKey
diff --git a/DataValues/resources/values/MonolingualTextValue.js
b/DataValues/resources/values/MonolingualTextValue.js
index cf60fec..36a1e4a 100644
--- a/DataValues/resources/values/MonolingualTextValue.js
+++ b/DataValues/resources/values/MonolingualTextValue.js
@@ -5,7 +5,7 @@
* @author Daniel Werner
* @author Jeroen De Dauw < [email protected] >
*/
-( function( dv, $, undefined ) {
+( function( dv, $ ) {
'use strict';
var PARENT = dv.DataValue,
@@ -26,7 +26,7 @@
* @param {String} languageCode
* @param {String} value
*/
-dv.MonolingualTextValue = dv.util.inherit( PARENT, constructor, {
+dv.MonolingualTextValue = dv.util.inherit( 'DvMonolingualTextValue', PARENT,
constructor, {
/**
* @see dv.DataValue.getSortKey
diff --git a/DataValues/resources/values/MultilingualTextValue.js
b/DataValues/resources/values/MultilingualTextValue.js
index 631967b..f062018 100644
--- a/DataValues/resources/values/MultilingualTextValue.js
+++ b/DataValues/resources/values/MultilingualTextValue.js
@@ -5,7 +5,7 @@
* @author Jeroen De Dauw < [email protected] >
* @author Daniel Werner
*/
-( function( dv, $, undefined ) {
+( function( dv, $ ) {
'use strict';
var PARENT = dv.DataValue,
@@ -24,7 +24,7 @@
*
* @param {dv.MonolingualTextValue[]} monoLingualValues
*/
-dv.MultilingualTextValue = dv.util.inherit( PARENT, constructor, {
+dv.MultilingualTextValue = dv.util.inherit( 'DvMultilingualTextValue', PARENT,
constructor, {
/**
* @see dv.DataValue.getSortKey
diff --git a/DataValues/resources/values/NumberValue.js
b/DataValues/resources/values/NumberValue.js
index b04a2ed..b0071df 100644
--- a/DataValues/resources/values/NumberValue.js
+++ b/DataValues/resources/values/NumberValue.js
@@ -6,7 +6,7 @@
*
* @author Daniel Werner < [email protected] >
*/
-( function( dv, $, undefined ) {
+( function( dv, $ ) {
'use strict';
var PARENT = dv.DataValue,
@@ -24,7 +24,7 @@
*
* @param {Number} value
*/
- dv.NumberValue = dv.util.inherit( PARENT, constructor, {
+ dv.NumberValue = dv.util.inherit( 'DvNumberValue', PARENT, constructor,
{
/**
* @see dv.DataValue.getSortKey
*
diff --git a/DataValues/resources/values/StringValue.js
b/DataValues/resources/values/StringValue.js
index 4eac002..d7e094c 100644
--- a/DataValues/resources/values/StringValue.js
+++ b/DataValues/resources/values/StringValue.js
@@ -7,7 +7,7 @@
* @author Daniel Werner
* @author Jeroen De Dauw < [email protected] >
*/
-( function( dv, $, undefined ) {
+( function( dv, $ ) {
'use strict';
var PARENT = dv.DataValue,
@@ -25,7 +25,7 @@
*
* @param {String} value
*/
-dv.StringValue = dv.util.inherit( PARENT, constructor, {
+dv.StringValue = dv.util.inherit( 'StringValue', PARENT, constructor, {
/**
* @see dv.DataValue.getSortKey
diff --git a/DataValues/resources/values/UnknownValue.js
b/DataValues/resources/values/UnknownValue.js
index 433e7da..ad2c308 100644
--- a/DataValues/resources/values/UnknownValue.js
+++ b/DataValues/resources/values/UnknownValue.js
@@ -24,7 +24,7 @@
*
* @param {String} value
*/
- dv.UnknownValue = dv.util.inherit( PARENT, constructor, {
+ dv.UnknownValue = dv.util.inherit( 'DvUnknownValue', PARENT,
constructor, {
/**
* @see dv.DataValue.getSortKey
diff --git a/DataValues/tests/qunit/dataValues.util.inherit.tests.js
b/DataValues/tests/qunit/dataValues.util.inherit.tests.js
index d09b35f..a301bdd 100644
--- a/DataValues/tests/qunit/dataValues.util.inherit.tests.js
+++ b/DataValues/tests/qunit/dataValues.util.inherit.tests.js
@@ -8,7 +8,7 @@
* @licence GNU GPL v2+
* @author Daniel Werner
*/
-( function( dv, $, QUnit, undefined ) {
+( function( dv, $, QUnit ) {
'use strict';
QUnit.module( 'dataValues.util.inherit', QUnit.newMwEnvironment() );
@@ -35,7 +35,13 @@
else if( constructor === null ) {
// constructor omitted, check for right param mapping:
C = dv.util.inherit( base, members );
- var C2 = dv.util.inherit( base,
C.prototype.constructor, members );
+ var C2 = dv.util.inherit( base,
C.prototype.constructor, members ),
+ origConstructorOfC = C.prototype.constructor;
+
+ // constructors will never be the same since a new
function will be created in inherit(),
+ // so we have to set them to the same to test for all
the other prototype members.
+ C.prototype.constructor = null;
+ C2.prototype.constructor = null;
QUnit.assert.deepEqual(
C.prototype,
@@ -43,6 +49,7 @@
'inherit() works as expected if "constructor"
parameter was omitted.'
);
C2 = null;
+ C.prototype.constructor = origConstructorOfC;
}
else if( members === null ) {
C = dv.util.inherit( base, constructor );
@@ -60,14 +67,6 @@
( new C() ) instanceof base && ( new C() ) instanceof C,
'"instanceof" is working like it should'
);
-
- if( constructor !== null ) {
- QUnit.assert.equal(
- C,
- constructor,
- 'prototypes constructor property is set to
given constructor'
- );
- }
var proto = $.extend( {}, C.prototype );
if( members === null || !members.hasOwnProperty( 'constructor'
) ) {
@@ -89,7 +88,7 @@
foo: 'baa'
},
- inheritConstructor = function() {
+ inheritConstructor = function InheritTestConstructor() {
this.foo = 'test';
},
@@ -120,9 +119,20 @@
var C1 = inheritTest( Object, inheritConstructor, null );
inheritConstructorTest( C1 );
+ QUnit.assert.equal(
+ C1.name,
+ inheritConstructor.name,
+ 'Constructor returned by inherit() takes name of given
constructor function'
+ );
+
// inherit from C2:
var C2 = inheritTest( C1, null, inheritMembers );
inheritConstructorTest( C2 );
+
+ QUnit.assert.ok(
+ C2.name.indexOf( inheritConstructor.name ) === 0,
+ 'Constructor returned by inherit() uses name of given
constructor plus suffix'
+ );
} );
QUnit.test( 'inherit( base, constructor, members )', function( assert )
{
@@ -131,4 +141,45 @@
inheritConstructorTest( C );
} );
+ QUnit.test( 'inherit( name, ... ) with different names', function(
assert ) {
+ var names = [
+ [ '$' ],
+ [ '_' ],
+ [ '$foo' ],
+ [ '_foo' ],
+ [ 'foo' ],
+ [ 'foo42' ],
+ [ '42foo', 'foo' ],
+ [ 'function()xxx(){};', 'functionxxx' ],
+ [ 'xyz;${]123', 'xyz$123' ],
+ [ ';a;1;$;b;_;', 'a1$b_' ],
+ [ '();', false ],
+ [ '42', false ], // can't start function name with
number
+ [ 'class', false ], // reserved word
+ [ 'function', false ] // reserved word
+ ];
+
+ $.each( names, function( i, definition ) {
+ var testName = definition[0],
+ expectedName = definition[1] || (
definition[1] === undefined ? testName : false );
+
+ if( !expectedName ) {
+ assert.throws(
+ function() {
+ dv.util.inherit( testName,
Object );
+ },
+ 'inherit( \'' + testName + '\', ... );
will throw and error because of malicious constructor name.'
+ );
+ } else {
+ assert.equal(
+ dv.util.inherit( testName, Object
).name,
+ expectedName,
+ 'inherit( \'' + testName + '\', ... );
will use "' + expectedName + '" as name.'
+ );
+
+ }
+
+ } );
+ } );
+
}( dataValues, jQuery, QUnit ) );
diff --git a/ValueParsers/resources/Api.js b/ValueParsers/resources/Api.js
index c7d459b..45f70b6 100644
--- a/ValueParsers/resources/Api.js
+++ b/ValueParsers/resources/Api.js
@@ -4,7 +4,7 @@
* @licence GNU GPL v2+
* @author Jeroen De Dauw < [email protected] >
*/
-( function( mw, vp, $, dv, undefined ) {
+( function( mw, vp, $, dv ) {
'use strict';
/**
diff --git a/ValueParsers/resources/ApiBasedValueParser.js
b/ValueParsers/resources/ApiBasedValueParser.js
index 1b5e64a..a82ba30 100644
--- a/ValueParsers/resources/ApiBasedValueParser.js
+++ b/ValueParsers/resources/ApiBasedValueParser.js
@@ -6,7 +6,7 @@
*
* @author Daniel Werner < [email protected] >
*/
-( function( vp, dv, $, undefined ) {
+( function( vp, dv, $ ) {
'use strict';
var PARENT = vp.ValueParser;
@@ -20,7 +20,7 @@
* @abstract
* @since 0.1
*/
- vp.ApiBasedValueParser = dv.util.inherit( PARENT, {
+ vp.ApiBasedValueParser = dv.util.inherit( 'VpApiBasedValueParser',
PARENT, {
/**
* The key of the related API parser.
* @type String
diff --git a/ValueParsers/resources/ValueParser.js
b/ValueParsers/resources/ValueParser.js
index 50efb4d..857e54b 100644
--- a/ValueParsers/resources/ValueParser.js
+++ b/ValueParsers/resources/ValueParser.js
@@ -16,7 +16,7 @@
*
* @param {Object} options
*/
- vp.ValueParser = function( options ) {
+ vp.ValueParser = function VpValueParser( options ) {
this._options = options || {};
};
diff --git a/ValueParsers/resources/ValueParserFactory.js
b/ValueParsers/resources/ValueParserFactory.js
index 55e844d..fd7e88e 100644
--- a/ValueParsers/resources/ValueParserFactory.js
+++ b/ValueParsers/resources/ValueParserFactory.js
@@ -14,7 +14,7 @@
*
* @param {Object} valueParsers
*/
- vp.ValueParserFactory = function( valueParsers ) {
+ vp.ValueParserFactory = function VpValueParserFactory( valueParsers ) {
this._parsers = {};
for ( var parserId in valueParsers ) {
--
To view, visit https://gerrit.wikimedia.org/r/50185
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3f15c65b3225eba12fd51d0ad814c1630e1ee2e7
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DataValues
Gerrit-Branch: master
Gerrit-Owner: Daniel Werner <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
