Merge "mw.Map: Avoid using 'undefined' to check for real existance."

diff --git a/resources/mediawiki/mediawiki.js b/resources/mediawiki/mediawiki.js
index 1212fd4..4dbf04c 100644 (file)
--- a/resources/mediawiki/mediawiki.js
+++ b/resources/mediawiki/mediawiki.js
@@ -42,22 +42,22 @@ var mw = ( function ( $, undefined ) {
                 */
                get: function ( selection, fallback ) {
                        var results, i;
+                       // If we only do this in the `return` block, it'll fail for the
+                       // call to get() from the mutli-selection block.
+                       fallback = arguments.length > 1 ? fallback : null;
 
                        if ( $.isArray( selection ) ) {
                                selection = slice.call( selection );
                                results = {};
-                               for ( i = 0; i < selection.length; i += 1 ) {
+                               for ( i = 0; i < selection.length; i++ ) {
                                        results[selection[i]] = this.get( selection[i], fallback );
                                }
                                return results;
                        }
 
                        if ( typeof selection === 'string' ) {
-                               if ( this.values[selection] === undefined ) {
-                                       if ( fallback !== undefined ) {
-                                               return fallback;
-                                       }
-                                       return null;
+                               if ( !hasOwn.call( this.values, selection ) ) {
+                                       return fallback;
                                }
                                return this.values[selection];
                        }
@@ -86,7 +86,7 @@ var mw = ( function ( $, undefined ) {
                                }
                                return true;
                        }
-                       if ( typeof selection === 'string' && value !== undefined ) {
+                       if ( typeof selection === 'string' && arguments.length > 1 ) {
                                this.values[selection] = value;
                                return true;
                        }
@@ -103,14 +103,14 @@ var mw = ( function ( $, undefined ) {
                        var s;
 
                        if ( $.isArray( selection ) ) {
-                               for ( s = 0; s < selection.length; s += 1 ) {
-                                       if ( this.values[selection[s]] === undefined ) {
+                               for ( s = 0; s < selection.length; s++ ) {
+                                       if ( typeof selection[s] !== 'string' || !hasOwn.call( this.values, selection[s] ) ) {
                                                return false;
                                        }
                                }
                                return true;
                        }
-                       return this.values[selection] !== undefined;
+                       return typeof selection === 'string' && hasOwn.call( this.values, selection );
                }
        };
 

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
index 45c3c5a..b599b02 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
@@ -37,26 +37,52 @@
                assert.strictEqual( window.mw, window.mediaWiki, 'mw alias to mediaWiki' );
        } );
 
-       QUnit.test( 'mw.Map', 17, function ( assert ) {
+       QUnit.test( 'mw.Map', 27, function ( assert ) {
                var arry, conf, funky, globalConf, nummy, someValues;
 
-               assert.ok( mw.Map, 'mw.Map defined' );
-
                conf = new mw.Map();
                // Dummy variables
                funky = function () {};
                arry = [];
                nummy = 7;
 
-               // Tests for input validation
-               assert.strictEqual( conf.get( 'inexistantKey' ), null, 'Map.get returns null if selection was a string and the key was not found' );
-               assert.strictEqual( conf.set( 'myKey', 'myValue' ), true, 'Map.set returns boolean true if a value was set for a valid key string' );
+               // Single get and set
+
+               assert.strictEqual( conf.set( 'foo', 'Bar' ), true, 'Map.set returns boolean true if a value was set for a valid key string' );
+               assert.equal( conf.get( 'foo' ), 'Bar', 'Map.get returns a single value value correctly' );
+
+               assert.strictEqual( conf.get( 'example' ), null, 'Map.get returns null if selection was a string and the key was not found' );
+               assert.strictEqual( conf.get( 'example', arry ), arry, 'Map.get returns fallback by reference if the key was not found' );
+               assert.strictEqual( conf.get( 'example', undefined ), undefined, 'Map.get supports `undefined` as fallback instead of `null`' );
+
+               assert.strictEqual( conf.get( 'constructor' ), null, 'Map.get does not look at Object.prototype of internal storage (constructor)' );
+               assert.strictEqual( conf.get( 'hasOwnProperty' ), null, 'Map.get does not look at Object.prototype of internal storage (hasOwnProperty)' );
+
+               conf.set( 'hasOwnProperty', function () { return true; } );
+               assert.strictEqual( conf.get( 'example', 'missing' ), 'missing', 'Map.get uses neutral hasOwnProperty method (positive)' );
+
+               conf.set( 'example', 'Foo' );
+               conf.set( 'hasOwnProperty', function () { return false; } );
+               assert.strictEqual( conf.get( 'example' ), 'Foo', 'Map.get uses neutral hasOwnProperty method (negative)' );
+
+               assert.strictEqual( conf.set( 'constructor', 42 ), true, 'Map.set for key "constructor"' );
+               assert.strictEqual( conf.get( 'constructor' ), 42, 'Map.get for key "constructor"' );
+
+               assert.strictEqual( conf.set( 'ImUndefined', undefined ), true, 'Map.set allows setting value to `undefined`' );
+               assert.equal( conf.get( 'ImUndefined', 'fallback' ), undefined , 'Map.get supports retreiving value of `undefined`' );
+
                assert.strictEqual( conf.set( funky, 'Funky' ), false, 'Map.set returns boolean false if key was invalid (Function)' );
                assert.strictEqual( conf.set( arry, 'Arry' ), false, 'Map.set returns boolean false if key was invalid (Array)' );
                assert.strictEqual( conf.set( nummy, 'Nummy' ), false, 'Map.set returns boolean false if key was invalid (Number)' );
-               assert.equal( conf.get( 'myKey' ), 'myValue', 'Map.get returns a single value value correctly' );
-               assert.strictEqual( conf.get( nummy ), null, 'Map.get ruturns null if selection was invalid (Number)' );
+
                assert.strictEqual( conf.get( funky ), null, 'Map.get ruturns null if selection was invalid (Function)' );
+               assert.strictEqual( conf.get( nummy ), null, 'Map.get ruturns null if selection was invalid (Number)' );
+
+               conf.set( String( nummy ), 'I used to be a number' );
+
+               assert.strictEqual( conf.exists( 'doesNotExist' ), false, 'Map.exists where property does not exist' );
+               assert.strictEqual( conf.exists( 'ImUndefined' ), true, 'Map.exists where value is `undefined`' );
+               assert.strictEqual( conf.exists( nummy ), false, 'Map.exists where key is invalid but looks like an existing key' );
 
                // Multiple values at once
                someValues = {
@@ -75,15 +101,13 @@
                        'notExist': null
                }, 'Map.get return includes keys that were not found as null values' );
 
-               assert.strictEqual( conf.exists( 'foo' ), true, 'Map.exists returns boolean true if a key exists' );
-               assert.strictEqual( conf.exists( 'notExist' ), false, 'Map.exists returns boolean false if a key does not exists' );
 
                // Interacting with globals and accessing the values object
                assert.strictEqual( conf.get(), conf.values, 'Map.get returns the entire values object by reference (if called without arguments)' );
 
                conf.set( 'globalMapChecker', 'Hi' );
 
-               assert.ok( false === 'globalMapChecker' in window, 'new mw.Map did not store its values in the global window object by default' );
+               assert.ok( 'globalMapChecker' in window === false, 'new mw.Map did not store its values in the global window object by default' );
 
                globalConf = new mw.Map( true );
                globalConf.set( 'anotherGlobalMapChecker', 'Hello' );