From 515de778ca756e181cc56c23b0c2841ef4eda8fa Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 30 Oct 2012 07:12:04 +0100 Subject: [PATCH] Lint: Tighten .jshintrc options and enforce them. Re-ordered .jshintrc to be alphabetical once again (like on jshint.com/docs/) and by section (predef, restriction, toleration, environment, legacy). Enforce eqeqeq:true, and hard-code camelcase:false since we have a few underscore ones. "delete x" problem: * "delete x" never passes jshint, we should use window.x instead. * "delete window.x" doesn't work in IE6-8 (throws exception) * "delete x" only works for "x = 1", not for "window.x". * Only solution I see: use IE @cc comments for the IE version of the test. * See also http://jsfiddle.net/8X6cS/6/ IE6: supportDeleteProperty = false, jscriptVersion = 5.6 IE7: supportDeleteProperty = false, jscriptVersion = 5.7 IE8: supportDeleteProperty = false, jscriptVersion = 5.8 IE9: supportDeleteProperty = true, jscriptVersion = 9 IE10: supportDeleteProperty = true, jscriptVersion = 10 Follows-up dbd005333d23dc1b248a3b6d17f32ab2b2396c66 --- grunt.js | 5 ++--- qunit/.jshintrc | 23 +++++++++++-------- qunit/qunit.js | 28 +++++++++++++---------- test/.jshintrc | 6 +++-- test/deepEqual.js | 67 ++++++++++++++++++++++++++++++------------------------- test/test.js | 40 ++++++++++++++++++++++++--------- 6 files changed, 102 insertions(+), 67 deletions(-) diff --git a/grunt.js b/grunt.js index ad60639..c0cb61f 100644 --- a/grunt.js +++ b/grunt.js @@ -18,9 +18,8 @@ grunt.initConfig({ lint: { qunit: 'qunit/qunit.js', addons: 'addons/**.js', - grunt: 'grunt.js' - // TODO fix remaining warnings - // tests: 'test/**.js' + grunt: 'grunt.js', + tests: 'test/**.js' }, // TODO rmeove this one grunt 0.4 is out, see jquery-ui for other details jshint: (function() { diff --git a/qunit/.jshintrc b/qunit/.jshintrc index 04aa9de..e867457 100644 --- a/qunit/.jshintrc +++ b/qunit/.jshintrc @@ -1,22 +1,27 @@ { - "onevar": true, - "browser": true, + "predef": [ + "jQuery", + "exports" + ], + "bitwise": true, + "camelcase": false, "curly": true, - "trailing": true, + "eqeqeq": true, "immed": true, "latedef": false, "newcap": true, "noarg": false, "noempty": true, "nonew": true, - "sub": true, "undef": true, - "eqnull": true, + "proto": true, "smarttabs": true, - "predef": [ - "jQuery", - "exports" - ] + "sub": true, + "trailing": true, + + "browser": true, + + "onevar": true } diff --git a/qunit/qunit.js b/qunit/qunit.js index 85fdaef..e1b355b 100644 --- a/qunit/qunit.js +++ b/qunit/qunit.js @@ -162,11 +162,11 @@ Test.prototype = { }, finish: function() { config.current = this; - if ( config.requireExpects && this.expected == null ) { + if ( config.requireExpects && this.expected === null ) { QUnit.pushFailure( "Expected number of assertions to be defined, but expect() was not called.", this.stack ); - } else if ( this.expected != null && this.expected != this.assertions.length ) { + } else if ( this.expected !== null && this.expected !== this.assertions.length ) { QUnit.pushFailure( "Expected " + this.expected + " assertions, but " + this.assertions.length + " were run", this.stack ); - } else if ( this.expected == null && !this.assertions.length ) { + } else if ( this.expected === null && !this.assertions.length ) { QUnit.pushFailure( "Expected at least one assertion, but none were run - call expect(0) to accept zero assertions.", this.stack ); } @@ -225,7 +225,7 @@ Test.prototype = { addEvent(b, "dblclick", function( e ) { var target = e && e.target ? e.target : window.event.srcElement; - if ( target.nodeName.toLowerCase() == "span" || target.nodeName.toLowerCase() == "b" ) { + if ( target.nodeName.toLowerCase() === "span" || target.nodeName.toLowerCase() === "b" ) { target = target.parentNode; } if ( window.location && target.nodeName.toLowerCase() === "strong" ) { @@ -460,6 +460,7 @@ assert = { * @example equal( format( "Received {0} bytes.", 2), "Received 2 bytes.", "format() replaces {0} with next argument" ); */ equal: function( actual, expected, message ) { + /*jshint eqeqeq:false */ QUnit.push( expected == actual, actual, expected, message ); }, @@ -468,6 +469,7 @@ assert = { * @function */ notEqual: function( actual, expected, message ) { + /*jshint eqeqeq:false */ QUnit.push( expected != actual, actual, expected, message ); }, @@ -758,7 +760,7 @@ extend( QUnit, { // Safe object type checking is: function( type, obj ) { - return QUnit.objectType( obj ) == type; + return QUnit.objectType( obj ) === type; }, objectType: function( obj ) { @@ -817,7 +819,7 @@ extend( QUnit, { actual = escapeInnerText( QUnit.jsDump.parse(actual) ); output += ""; - if ( actual != expected ) { + if ( actual !== expected ) { output += ""; output += ""; } @@ -1227,7 +1229,7 @@ function extractStacktrace( e, offset ) { if ( fileName ) { include = []; for ( i = offset; i < stack.length; i++ ) { - if ( stack[ i ].indexOf( fileName ) != -1 ) { + if ( stack[ i ].indexOf( fileName ) !== -1 ) { break; } include.push( stack[ i ] ); @@ -1448,6 +1450,7 @@ QUnit.equiv = (function() { // for string, boolean, number and null function useStrictEquality( b, a ) { + /*jshint eqeqeq:false */ if ( b instanceof a.constructor || a instanceof b.constructor ) { // to catch short annotaion VS 'new' annotation of a // declaration @@ -1653,16 +1656,16 @@ QUnit.jsDump = (function() { type = typeof parser; inStack = inArray( obj, stack ); - if ( inStack != -1 ) { + if ( inStack !== -1 ) { return "recursion(" + (inStack - stack.length) + ")"; } - if ( type == "function" ) { + if ( type === "function" ) { stack.push( obj ); res = parser.call( this, obj, stack ); stack.pop(); return res; } - return ( type == "string" ) ? parser : this.parsers.error; + return ( type === "string" ) ? parser : this.parsers.error; }, typeOf: function( obj ) { var type; @@ -1878,6 +1881,7 @@ function inArray( elem, array ) { * QUnit.diff( "the quick brown fox jumped over", "the quick fox jumps over" ) == "the quick brown fox jumped jumps over" */ QUnit.diff = (function() { + /*jshint eqeqeq:false, eqnull:true */ function diff( o, n ) { var i, ns = {}, @@ -1907,7 +1911,7 @@ QUnit.diff = (function() { if ( !hasOwn.call( ns, i ) ) { continue; } - if ( ns[i].rows.length == 1 && typeof os[i] != "undefined" && os[i].rows.length == 1 ) { + if ( ns[i].rows.length === 1 && os[i] !== undefined && os[i].rows.length === 1 ) { n[ ns[i].rows[0] ] = { text: n[ ns[i].rows[0] ], row: os[i].rows[0] @@ -2013,7 +2017,7 @@ QUnit.diff = (function() { // for CommonJS enviroments, export everything if ( typeof exports !== "undefined" ) { - extend(exports, QUnit); + extend( exports, QUnit ); } // get at whatever the global object is, like window in browsers diff --git a/test/.jshintrc b/test/.jshintrc index 7edcc8f..b14a0bc 100644 --- a/test/.jshintrc +++ b/test/.jshintrc @@ -2,6 +2,8 @@ "predef": [ "QUnit" ], - "browser": true, - "smarttabs": true + + "smarttabs": true, + + "browser": true } diff --git a/test/deepEqual.js b/test/deepEqual.js index 6f1e901..7894485 100644 --- a/test/deepEqual.js +++ b/test/deepEqual.js @@ -70,38 +70,43 @@ test("Primitive types and constants", function () { equal(QUnit.equiv('', null), false, "string"); equal(QUnit.equiv('', undefined), false, "string"); + // Rename for lint validation. + // We know this is bad, we are asserting whether we can coop with bad code like this. + var SafeNumber = Number, SafeString = String, SafeBoolean = Boolean, SafeObject = Object; + // primitives vs. objects - equal(QUnit.equiv(0, new Number()), true, "primitives vs. objects"); - equal(QUnit.equiv(new Number(), 0), true, "primitives vs. objects"); - equal(QUnit.equiv(1, new Number(1)), true, "primitives vs. objects"); - equal(QUnit.equiv(new Number(1), 1), true, "primitives vs. objects"); - equal(QUnit.equiv(new Number(0), 1), false, "primitives vs. objects"); - equal(QUnit.equiv(0, new Number(1)), false, "primitives vs. objects"); - - equal(QUnit.equiv(new String(), ""), true, "primitives vs. objects"); - equal(QUnit.equiv("", new String()), true, "primitives vs. objects"); - equal(QUnit.equiv(new String("My String"), "My String"), true, "primitives vs. objects"); - equal(QUnit.equiv("My String", new String("My String")), true, "primitives vs. objects"); - equal(QUnit.equiv("Bad String", new String("My String")), false, "primitives vs. objects"); - equal(QUnit.equiv(new String("Bad String"), "My String"), false, "primitives vs. objects"); - - equal(QUnit.equiv(false, new Boolean()), true, "primitives vs. objects"); - equal(QUnit.equiv(new Boolean(), false), true, "primitives vs. objects"); - equal(QUnit.equiv(true, new Boolean(true)), true, "primitives vs. objects"); - equal(QUnit.equiv(new Boolean(true), true), true, "primitives vs. objects"); - equal(QUnit.equiv(true, new Boolean(1)), true, "primitives vs. objects"); - equal(QUnit.equiv(false, new Boolean(false)), true, "primitives vs. objects"); - equal(QUnit.equiv(new Boolean(false), false), true, "primitives vs. objects"); - equal(QUnit.equiv(false, new Boolean(0)), true, "primitives vs. objects"); - equal(QUnit.equiv(true, new Boolean(false)), false, "primitives vs. objects"); - equal(QUnit.equiv(new Boolean(false), true), false, "primitives vs. objects"); - - equal(QUnit.equiv(new Object(), {}), true, "object literal vs. instantiation"); - equal(QUnit.equiv({}, new Object()), true, "object literal vs. instantiation"); - equal(QUnit.equiv(new Object(), {a:1}), false, "object literal vs. instantiation"); - equal(QUnit.equiv({a:1}, new Object()), false, "object literal vs. instantiation"); - equal(QUnit.equiv({a:undefined}, new Object()), false, "object literal vs. instantiation"); - equal(QUnit.equiv(new Object(), {a:undefined}), false, "object literal vs. instantiation"); + + equal(QUnit.equiv(0, new SafeNumber()), true, "primitives vs. objects"); + equal(QUnit.equiv(new SafeNumber(), 0), true, "primitives vs. objects"); + equal(QUnit.equiv(1, new SafeNumber(1)), true, "primitives vs. objects"); + equal(QUnit.equiv(new SafeNumber(1), 1), true, "primitives vs. objects"); + equal(QUnit.equiv(new SafeNumber(0), 1), false, "primitives vs. objects"); + equal(QUnit.equiv(0, new SafeNumber(1)), false, "primitives vs. objects"); + + equal(QUnit.equiv(new SafeString(), ""), true, "primitives vs. objects"); + equal(QUnit.equiv("", new SafeString()), true, "primitives vs. objects"); + equal(QUnit.equiv(new SafeString("My String"), "My String"), true, "primitives vs. objects"); + equal(QUnit.equiv("My String", new SafeString("My String")), true, "primitives vs. objects"); + equal(QUnit.equiv("Bad String", new SafeString("My String")), false, "primitives vs. objects"); + equal(QUnit.equiv(new SafeString("Bad String"), "My String"), false, "primitives vs. objects"); + + equal(QUnit.equiv(false, new SafeBoolean()), true, "primitives vs. objects"); + equal(QUnit.equiv(new SafeBoolean(), false), true, "primitives vs. objects"); + equal(QUnit.equiv(true, new SafeBoolean(true)), true, "primitives vs. objects"); + equal(QUnit.equiv(new SafeBoolean(true), true), true, "primitives vs. objects"); + equal(QUnit.equiv(true, new SafeBoolean(1)), true, "primitives vs. objects"); + equal(QUnit.equiv(false, new SafeBoolean(false)), true, "primitives vs. objects"); + equal(QUnit.equiv(new SafeBoolean(false), false), true, "primitives vs. objects"); + equal(QUnit.equiv(false, new SafeBoolean(0)), true, "primitives vs. objects"); + equal(QUnit.equiv(true, new SafeBoolean(false)), false, "primitives vs. objects"); + equal(QUnit.equiv(new SafeBoolean(false), true), false, "primitives vs. objects"); + + equal(QUnit.equiv(new SafeObject(), {}), true, "object literal vs. instantiation"); + equal(QUnit.equiv({}, new SafeObject()), true, "object literal vs. instantiation"); + equal(QUnit.equiv(new SafeObject(), {a:1}), false, "object literal vs. instantiation"); + equal(QUnit.equiv({a:1}, new SafeObject()), false, "object literal vs. instantiation"); + equal(QUnit.equiv({a:undefined}, new SafeObject()), false, "object literal vs. instantiation"); + equal(QUnit.equiv(new SafeObject(), {a:undefined}), false, "object literal vs. instantiation"); }); test("Objects Basics.", function() { diff --git a/test/test.js b/test/test.js index a761604..1e3a121 100644 --- a/test/test.js +++ b/test/test.js @@ -62,13 +62,32 @@ module("setup/teardown test", { setup: function() { state = true; ok(true); - x = 1; + // Assert that we can introduce and delete globals in setup/teardown + // without noglobals sounding any alarm. + + // Using an implied global variable instead of explicit window property + // because there is no way to delete a window.property in IE6-8 + // `delete x` only works for `x = 1, and `delete window.x` throws exception. + // No one-code fits all solution possible afaic. Resort to @cc. + + /*@cc_on + @if (@_jscript_version < 9) + x = 1; + @else @*/ + window.x = 1; + /*@end + @*/ }, teardown: function() { ok(true); - // can introduce and delete globals in setup/teardown - // without noglobals sounding the alarm - delete x; + + /*@cc_on + @if (@_jscript_version < 9) + delete x; + @else @*/ + delete window.x; + /*@end + @*/ } }); @@ -84,20 +103,18 @@ test("module without setup/teardown", function() { ok(true); }); - - var orgDate; module("Date test", { setup: function() { orgDate = Date; - Date = function () { + window.Date = function () { ok( false, 'QUnit should internally be independant from Date-related manipulation and testing' ); return new orgDate(); }; }, teardown: function() { - Date = orgDate; + window.Date = orgDate; } }); @@ -129,7 +146,8 @@ test("parameter passed to stop increments semaphore n times", function() { stop(3); setTimeout(function() { state = "not enough starts"; - start(), start(); + start(); + start(); }, 13); setTimeout(function() { state = "done"; @@ -139,7 +157,9 @@ test("parameter passed to stop increments semaphore n times", function() { test("parameter passed to start decrements semaphore n times", function() { expect(1); - stop(), stop(), stop(); + stop(); + stop(); + stop(); setTimeout(function() { state = "done"; start(3); -- 2.11.0
Expected: 
" + expected + "
Result: 
" + actual + "
Diff: 
" + QUnit.diff( expected, actual ) + "