From df8c4551dcba95218cf6021dc1f3542f1095b56f Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 16 May 2014 16:07:41 +0200 Subject: [PATCH] Update jscs and jshint config Continue specifying our coding style more accurately in jscs, and phase out more deprecated jshint coding style options. jshint: * Update to grunt-contrib-jshint v0.10.0 (jshint v2.5.0). * Remove coding style option "curly" (already covered by jscs). * Remove coding style option "smarttabs" (already covered by jscs). * Remove option "regexp". * Enable new option "freeze" (prohibits changing native prototypes). http://www.jshint.com/blog/new-in-jshint-oct-2013/#option-freeze * Re-order to match http://www.jshint.com/docs/options/ jscs: * Update to grunt-contrib-jshint v0.4.4 (jscs v1.4.5). * Format .jscsrc file in a more spacious way and order the properties less arbitrarily (using the jscs's readme order). * Improve rule "requireCurlyBraces": Add more keywords that should have their code block wrapped in curly braces. * Improve rule "requireMultipleVarDecl": Use onevar instead of true. * Remove rules for "sticky operators", these rules are buggy and have been deprecated. Using the SpaceBefore/After rules for Unary and Binary operators instead. * Enable rule "disallowYodaConditions". Change-Id: I6f385e8860e91d9ef4d1f5abecf517d36af45565 --- .jscsrc | 120 ++++++++++++++---- .jshintrc | 33 ++--- resources/src/jquery/jquery.highlightText.js | 9 +- resources/src/jquery/jquery.placeholder.js | 2 +- .../jquery/jquery.qunit.completenessTest.js | 2 +- resources/src/jquery/jquery.suggestions.js | 4 +- resources/src/jquery/jquery.tablesorter.js | 76 ++++++----- .../mediawiki.special.preferences.js | 2 +- .../src/mediawiki/mediawiki.debug.profile.js | 3 +- .../src/mediawiki/mediawiki.jqueryMsg.js | 5 +- resources/src/mediawiki/mediawiki.js | 2 +- resources/src/mediawiki/mediawiki.util.js | 2 +- skins/common/ajax.js | 7 +- tests/frontend/package.json | 4 +- .../jquery/jquery.accessKeyLabel.test.js | 2 +- .../jquery/jquery.textSelection.test.js | 24 ++-- .../mediawiki/mediawiki.jqueryMsg.test.js | 6 +- 17 files changed, 185 insertions(+), 118 deletions(-) diff --git a/.jscsrc b/.jscsrc index b5481ea418..0da9aa5a24 100644 --- a/.jscsrc +++ b/.jscsrc @@ -1,30 +1,94 @@ { - "requireCurlyBraces": ["if", "else", "for", "while", "do"], - "requireSpaceAfterKeywords": ["if", "else", "for", "while", "do", "switch", "return", "function"], - "requireParenthesesAroundIIFE": true, - "requireSpacesInFunctionExpression": { - "beforeOpeningCurlyBrace": true - }, - "requireMultipleVarDecl": true, - "disallowEmptyBlocks": true, - "requireSpacesInsideObjectBrackets": "all", - "disallowSpaceAfterObjectKeys": true, - "requireCommaBeforeLineBreak": true, - "disallowLeftStickedOperators": ["?", ">", ">=", "<", "<="], - "disallowRightStickedOperators": ["?", "/", "*", ":", "=", "==", "===", "!=", "!==", ">", ">=", "<", "<="], - "requireRightStickedOperators": ["!"], - "requireLeftStickedOperators": [","], - "disallowSpaceAfterPrefixUnaryOperators": ["++", "--", "+", "-", "~"], - "disallowSpaceBeforePostfixUnaryOperators": ["++", "--"], - "requireSpaceBeforeBinaryOperators": ["+", "-", "/", "*", "=", "==", "===", "!=", "!=="], - "requireSpaceAfterBinaryOperators": ["+", "-", "/", "*", "=", "==", "===", "!=", "!=="], - "disallowKeywords": [ "with" ], - "disallowMultipleLineBreaks": true, - "validateLineBreaks": "LF", - "validateQuoteMarks": "'", - "disallowMixedSpacesAndTabs": "smart", - "disallowTrailingWhitespace": true, - "requireLineFeedAtFileEnd": true, - "requireCapitalizedConstructors": true, - "requireDotNotation": true + "requireCurlyBraces": [ + "if", + "else", + "for", + "while", + "do", + "try", + "catch" + ], + "requireSpaceAfterKeywords": [ + "if", + "else", + "for", + "while", + "do", + "switch", + "return", + "try", + "catch", + "function" + ], + "requireSpaceBeforeBlockStatements": true, + "requireParenthesesAroundIIFE": true, + "requireSpacesInConditionalExpression": true, + "disallowSpacesInNamedFunctionExpression": { + "beforeOpeningRoundBrace": true + }, + "disallowSpacesInFunctionDeclaration": { + "beforeOpeningRoundBrace": true + }, + "requireMultipleVarDecl": "onevar", + "requireBlocksOnNewline": 1, + "disallowEmptyBlocks": true, + "requireSpacesInsideObjectBrackets": "all", + "disallowSpaceAfterObjectKeys": true, + "requireCommaBeforeLineBreak": true, + "disallowSpaceAfterPrefixUnaryOperators": [ + "++", + "--", + "+", + "-", + "~", + "!" + ], + "disallowSpaceBeforePostfixUnaryOperators": [ + "++", + "--" + ], + "disallowSpaceBeforeBinaryOperators": [ + "," + ], + "requireSpaceBeforeBinaryOperators": [ + "=", + "+", + "-", + "/", + "*", + "==", + "===", + "!=", + "!==", + ">", + ">=", + "<", + "<=" + ], + "requireSpaceAfterBinaryOperators": [ + "=", + "+", + "-", + "/", + "*", + "==", + "===", + "!=", + "!==", + ">", + ">=", + "<", + "<=" + ], + "disallowKeywords": [ "with" ], + "disallowMultipleLineBreaks": true, + "validateLineBreaks": "LF", + "validateQuoteMarks": "'", + "disallowMixedSpacesAndTabs": true, + "disallowTrailingWhitespace": true, + "disallowTrailingComma": true, + "requireLineFeedAtFileEnd": true, + "requireCapitalizedConstructors": true, + "requireDotNotation": true, + "disallowYodaConditions": true } diff --git a/.jshintrc b/.jshintrc index 4eec7a0eb8..c136dfc6c8 100644 --- a/.jshintrc +++ b/.jshintrc @@ -1,37 +1,30 @@ { - /* Common */ - // Enforcing + "bitwise": true, "eqeqeq": true, "es3": true, + "freeze": true, "latedef": true, "noarg": true, "nonew": true, "undef": true, "unused": true, - - /* Local */ - - // FIXME: Deprecated, handle these with node-jscs instead. - // Handled here because we still have inline overrides in some places. - "camelcase": true, - "nomen": true, - - // Enforcing - "bitwise": true, - "forin": false, - "regexp": false, "strict": false, + // Relaxing "laxbreak": true, - "smarttabs": true, "multistr": true, + // Environment "browser": true, - "predef": [ - "mediaWiki", - "jQuery", - "QUnit" - ] + "globals": { + "mediaWiki": true, + "jQuery": false, + "QUnit": false + }, + + // Legacy (to be handled by jscs once supported) + "camelcase": true, + "nomen": true } diff --git a/resources/src/jquery/jquery.highlightText.js b/resources/src/jquery/jquery.highlightText.js index 040815128a..13382182b7 100644 --- a/resources/src/jquery/jquery.highlightText.js +++ b/resources/src/jquery/jquery.highlightText.js @@ -47,8 +47,13 @@ middlebit.parentNode.replaceChild( spannode, middlebit ); } // if this is an element with childnodes, and not a script, style or an element we created - } else if ( node.nodeType === 1 && node.childNodes && !/(script|style)/i.test( node.tagName ) - && !( node.tagName.toLowerCase() === 'span' && node.className.match( /\bhighlight/ ) ) ) { + } else if ( node.nodeType === 1 + && node.childNodes + && !/(script|style)/i.test( node.tagName ) + && !( node.tagName.toLowerCase() === 'span' + && node.className.match( /\bhighlight/ ) + ) + ) { for ( i = 0; i < node.childNodes.length; ++i ) { // call the highlight function for each child node $.highlightText.innerHighlight( node.childNodes[i], pat ); diff --git a/resources/src/jquery/jquery.placeholder.js b/resources/src/jquery/jquery.placeholder.js index 6f7ada3166..d458019020 100644 --- a/resources/src/jquery/jquery.placeholder.js +++ b/resources/src/jquery/jquery.placeholder.js @@ -178,7 +178,7 @@ if (!$input.data('placeholder-textinput')) { try { $replacement = $input.clone().attr({ 'type': 'text' }); - } catch(e) { + } catch (e) { $replacement = $('').attr($.extend(args(this), { 'type': 'text' })); } $replacement diff --git a/resources/src/jquery/jquery.qunit.completenessTest.js b/resources/src/jquery/jquery.qunit.completenessTest.js index 86fcaea309..1c47febcaf 100644 --- a/resources/src/jquery/jquery.qunit.completenessTest.js +++ b/resources/src/jquery/jquery.qunit.completenessTest.js @@ -101,7 +101,7 @@ this.methodCallTracker = {}; this.missingTests = {}; - this.ignoreFn = undefined === ignoreFn ? function () { return false; } : ignoreFn; + this.ignoreFn = ignoreFn === undefined ? function () { return false; } : ignoreFn; // Lazy limit in case something weird happends (like recurse (part of) ourself). this.lazyLimit = 2000; diff --git a/resources/src/jquery/jquery.suggestions.js b/resources/src/jquery/jquery.suggestions.js index 7d200ff93e..8768c24b3d 100644 --- a/resources/src/jquery/jquery.suggestions.js +++ b/resources/src/jquery/jquery.suggestions.js @@ -215,10 +215,10 @@ $.suggestions = { } if ( expandFrom === 'start' ) { - expandFrom = docDir === 'rtl' ? 'right': 'left'; + expandFrom = docDir === 'rtl' ? 'right' : 'left'; } else if ( expandFrom === 'end' ) { - expandFrom = docDir === 'rtl' ? 'left': 'right'; + expandFrom = docDir === 'rtl' ? 'left' : 'right'; } return expandFrom; diff --git a/resources/src/jquery/jquery.tablesorter.js b/resources/src/jquery/jquery.tablesorter.js index 405a0e4eb4..5b1e2a7f30 100644 --- a/resources/src/jquery/jquery.tablesorter.js +++ b/resources/src/jquery/jquery.tablesorter.js @@ -160,15 +160,14 @@ } function buildParserCache( table, $headers ) { - var rows = table.tBodies[0].rows, - sortType, + var sortType, cells, len, i, parser, + rows = table.tBodies[0].rows, parsers = []; if ( rows[0] ) { - var cells = rows[0].cells, - len = cells.length, - i, parser; + cells = rows[0].cells; + len = cells.length; for ( i = 0; i < len; i++ ) { parser = false; @@ -190,7 +189,8 @@ /* Other utility functions */ function buildCache( table ) { - var totalRows = ( table.tBodies[0] && table.tBodies[0].rows.length ) || 0, + var i, j, $row, cols, + totalRows = ( table.tBodies[0] && table.tBodies[0].rows.length ) || 0, totalCells = ( table.tBodies[0].rows[0] && table.tBodies[0].rows[0].cells.length ) || 0, parsers = table.config.parsers, cache = { @@ -198,11 +198,11 @@ normalized: [] }; - for ( var i = 0; i < totalRows; ++i ) { + for ( i = 0; i < totalRows; ++i ) { // Add the table data to main data array - var $row = $( table.tBodies[0].rows[i] ), - cols = []; + $row = $( table.tBodies[0].rows[i] ); + cols = []; // if this is a child row, add it to the last row's children and // continue to the next row @@ -214,7 +214,7 @@ cache.row.push( $row ); - for ( var j = 0; j < totalCells; ++j ) { + for ( j = 0; j < totalCells; ++j ) { cols.push( parsers[j].format( getElementSortKey( $row[0].cells[j] ), table, $row[0].cells[j] ) ); } @@ -292,18 +292,19 @@ colspanOffset = 0, columns, i, + rowspan, + colspan, + headerCount, + longestTR, + matrixRowIndex, + matrixColumnIndex, + exploded, $tableHeaders = $( [] ), $tableRows = $( 'thead:eq(0) > tr', table ); if ( $tableRows.length <= 1 ) { $tableHeaders = $tableRows.children( 'th' ); } else { - var rowspan, - colspan, - headerCount, - longestTR, - matrixRowIndex, - matrixColumnIndex, - exploded = []; + exploded = []; // Loop through all the dom cells of the thead $tableRows.each( function ( rowIndex, row ) { @@ -416,8 +417,9 @@ } function isValueInArray( v, a ) { - var l = a.length; - for ( var i = 0; i < l; i++ ) { + var i, + len = a.length; + for ( i = 0; i < len; i++ ) { if ( a[i][0] === v ) { return true; } @@ -780,9 +782,10 @@ // Legacy fix of .sortbottoms // Wrap them inside inside a tfoot (because that's what they actually want to be) & // and put the at the end of the - var $sortbottoms = $table.find( '> tbody > tr.sortbottom' ); + var $tfoot, + $sortbottoms = $table.find( '> tbody > tr.sortbottom' ); if ( $sortbottoms.length ) { - var $tfoot = $table.children( 'tfoot' ); + $tfoot = $table.children( 'tfoot' ); if ( $tfoot.length ) { $tfoot.eq( 0 ).prepend( $sortbottoms ); } else { @@ -799,6 +802,10 @@ // Apply event handling to headers // this is too big, perhaps break it out? $headers.not( '.' + table.config.unsortableClass ).on( 'keypress click', function ( e ) { + var cell, columns, newSortList, i, + totalRows, + j, s, o; + if ( e.type === 'click' && e.target.nodeName.toLowerCase() === 'a' ) { // The user clicked on a link inside a table header. // Do nothing and let the default link click action continue. @@ -822,14 +829,12 @@ // cells get event .change() and bubbles up to the
here cache = buildCache( table ); - var totalRows = ( $table[0].tBodies[0] && $table[0].tBodies[0].rows.length ) || 0; + totalRows = ( $table[0].tBodies[0] && $table[0].tBodies[0].rows.length ) || 0; if ( !table.sortDisabled && totalRows > 0 ) { // Get current column sort order this.order = this.count % 2; this.count++; - var cell, columns, newSortList, i; - cell = this; // Get current column index columns = table.headerToColumns[ this.headerIndex ]; @@ -851,9 +856,9 @@ if ( isValueInArray( i, config.sortList ) ) { // The user has clicked on an already sorted column. // Reverse the sorting direction for all tables. - for ( var j = 0; j < config.sortList.length; j++ ) { - var s = config.sortList[j], - o = config.headerList[s[0]]; + for ( j = 0; j < config.sortList.length; j++ ) { + s = config.sortList[j]; + o = config.headerList[s[0]]; if ( isValueInArray( s[0], newSortList ) ) { o.count = s[1]; o.count++; @@ -934,9 +939,10 @@ }, addParser: function ( parser ) { - var l = parsers.length, + var i, + len = parsers.length, a = true; - for ( var i = 0; i < l; i++ ) { + for ( i = 0; i < len; i++ ) { if ( parsers[i].id.toLowerCase() === parser.id.toLowerCase() ) { a = false; } @@ -1013,11 +1019,12 @@ return ts.rgx.IPAddress[0].test( s ); }, format: function ( s ) { - var a = s.split( '.' ), + var i, item, + a = s.split( '.' ), r = '', - l = a.length; - for ( var i = 0; i < l; i++ ) { - var item = a[i]; + len = a.length; + for ( i = 0; i < len; i++ ) { + item = a[i]; if ( item.length === 1 ) { r += '00' + item; } else if ( item.length === 2 ) { @@ -1082,7 +1089,7 @@ return ( ts.dateRegex[0].test( s ) || ts.dateRegex[1].test( s ) || ts.dateRegex[2].test( s ) ); }, format: function ( s ) { - var match; + var match, y; s = $.trim( s.toLowerCase() ); if ( ( match = s.match( ts.dateRegex[0] ) ) !== null ) { @@ -1112,7 +1119,6 @@ s[2] = '0' + s[2]; } - var y; if ( ( y = parseInt( s[0], 10 ) ) < 100 ) { // Guestimate years without centuries if ( y < 30 ) { diff --git a/resources/src/mediawiki.special/mediawiki.special.preferences.js b/resources/src/mediawiki.special/mediawiki.special.preferences.js index d0569bd71f..e553f449d6 100644 --- a/resources/src/mediawiki.special/mediawiki.special.preferences.js +++ b/resources/src/mediawiki.special/mediawiki.special.preferences.js @@ -200,7 +200,7 @@ jQuery( function ( $ ) { } } - function updateTimezoneSelection () { + function updateTimezoneSelection() { var minuteDiff, localTime, type = $tzSelect.val(); diff --git a/resources/src/mediawiki/mediawiki.debug.profile.js b/resources/src/mediawiki/mediawiki.debug.profile.js index 49c88c351d..64ec6c394a 100644 --- a/resources/src/mediawiki/mediawiki.debug.profile.js +++ b/resources/src/mediawiki/mediawiki.debug.profile.js @@ -271,8 +271,7 @@ $container.find( '.mw-debug-profile-period' ).tipsy( { fade: true, gravity: function () { - return $.fn.tipsy.autoNS.call( this ) - + $.fn.tipsy.autoWE.call( this ); + return $.fn.tipsy.autoNS.call( this ) + $.fn.tipsy.autoWE.call( this ); }, className: 'mw-debug-profile-tipsy', center: false, diff --git a/resources/src/mediawiki/mediawiki.jqueryMsg.js b/resources/src/mediawiki/mediawiki.jqueryMsg.js index 3731771853..9d34d62cd8 100644 --- a/resources/src/mediawiki/mediawiki.jqueryMsg.js +++ b/resources/src/mediawiki/mediawiki.jqueryMsg.js @@ -117,12 +117,13 @@ var parser = new mw.jqueryMsg.parser( options ); return function ( args ) { - var key = args[0], + var fallback, + key = args[0], argsArray = $.isArray( args[1] ) ? args[1] : slice.call( args, 1 ); try { return parser.parse( key, argsArray ); } catch ( e ) { - var fallback = parser.settings.messages.get( key ); + fallback = parser.settings.messages.get( key ); mw.log.warn( 'mediawiki.jqueryMsg: ' + key + ': ' + e.message ); return $( '' ).text( fallback ); } diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js index fc4635acbb..c1815a598c 100644 --- a/resources/src/mediawiki/mediawiki.js +++ b/resources/src/mediawiki/mediawiki.js @@ -1606,7 +1606,7 @@ version: version !== undefined ? parseInt( version, 10 ) : 0, dependencies: [], group: typeof group === 'string' ? group : null, - source: typeof source === 'string' ? source: 'local', + source: typeof source === 'string' ? source : 'local', state: 'registered' }; if ( typeof dependencies === 'string' ) { diff --git a/resources/src/mediawiki/mediawiki.util.js b/resources/src/mediawiki/mediawiki.util.js index 298415cb6a..eac1088b2d 100644 --- a/resources/src/mediawiki/mediawiki.util.js +++ b/resources/src/mediawiki/mediawiki.util.js @@ -399,7 +399,7 @@ // RegExp is case insensitive 'i' ); - return ( null !== mailtxt.match( html5EmailRegexp ) ); + return ( mailtxt.match( html5EmailRegexp ) !== null ); }, /** diff --git a/skins/common/ajax.js b/skins/common/ajax.js index c017e3ca0f..6b9464a914 100644 --- a/skins/common/ajax.js +++ b/skins/common/ajax.js @@ -19,14 +19,15 @@ function debug( text ) { return false; } - var e = document.getElementById( 'sajax_debug' ); + var b, m, + e = document.getElementById( 'sajax_debug' ); if ( !e ) { e = document.createElement( 'p' ); e.className = 'sajax_debug'; e.id = 'sajax_debug'; - var b = document.getElementsByTagName( 'body' )[0]; + b = document.getElementsByTagName( 'body' )[0]; if ( b.firstChild ) { b.insertBefore( e, b.firstChild ); @@ -35,7 +36,7 @@ function debug( text ) { } } - var m = document.createElement( 'div' ); + m = document.createElement( 'div' ); m.appendChild( document.createTextNode( text ) ); e.appendChild( m ); diff --git a/tests/frontend/package.json b/tests/frontend/package.json index 235ddbf3f0..a398596e35 100644 --- a/tests/frontend/package.json +++ b/tests/frontend/package.json @@ -6,10 +6,10 @@ }, "devDependencies": { "grunt": "0.4.2", - "grunt-contrib-jshint": "0.9.2", + "grunt-contrib-jshint": "0.10.0", "grunt-contrib-watch": "0.6.1", "grunt-banana-checker": "0.1.0", - "grunt-jscs-checker": "0.4.1", + "grunt-jscs-checker": "0.4.4", "grunt-jsonlint": "1.0.4" } } diff --git a/tests/qunit/suites/resources/jquery/jquery.accessKeyLabel.test.js b/tests/qunit/suites/resources/jquery/jquery.accessKeyLabel.test.js index 60a63f9c62..f6ea1b48cc 100644 --- a/tests/qunit/suites/resources/jquery/jquery.accessKeyLabel.test.js +++ b/tests/qunit/suites/resources/jquery/jquery.accessKeyLabel.test.js @@ -30,7 +30,7 @@ //strings appended to title to make sure updateTooltipAccessKeys handles them correctly updateTooltipAccessKeysTestData = [ '', ' [a]', ' [test-a]', ' [alt-b]' ]; - function makeInput ( title, accessKey ) { + function makeInput( title, accessKey ) { //The properties aren't escaped, so make sure you don't call this function with values that need to be escaped! return ''; } diff --git a/tests/qunit/suites/resources/jquery/jquery.textSelection.test.js b/tests/qunit/suites/resources/jquery/jquery.textSelection.test.js index 25d9f736ea..2191c3b1c2 100644 --- a/tests/qunit/suites/resources/jquery/jquery.textSelection.test.js +++ b/tests/qunit/suites/resources/jquery/jquery.textSelection.test.js @@ -32,7 +32,7 @@ }, opt.after ); QUnit.test( opt.description, function ( assert ) { - var $textarea, start, end, options, text, + var $textarea, start, end, options, text, selected, tests = 1; if ( opt.after.selected !== null ) { tests++; @@ -63,7 +63,7 @@ assert.equal( text, opt.after.text, 'Checking full text after encapsulation' ); if ( opt.after.selected !== null ) { - var selected = $textarea.textSelection( 'getSelection' ); + selected = $textarea.textSelection( 'getSelection' ); assert.equal( selected, opt.after.selected, 'Checking selected text after encapsulation.' ); } @@ -248,16 +248,16 @@ caretSample = 'Some big text that we like to work with. Nothing fancy... you know what I mean?'; - /* - // @broken: Disabled per bug 34820 - caretTest({ - description: 'getCaretPosition with original/empty selection - bug 31847 with IE 6/7/8', - text: caretSample, - start: [0, caretSample.length], // Opera and Firefox (prior to FF 6.0) default caret to the end of the box (caretSample.length) - end: [0, caretSample.length], // Other browsers default it to the beginning (0), so check both. - mode: 'get' - }); - */ +/* + // @broken: Disabled per bug 34820 + caretTest({ + description: 'getCaretPosition with original/empty selection - bug 31847 with IE 6/7/8', + text: caretSample, + start: [0, caretSample.length], // Opera and Firefox (prior to FF 6.0) default caret to the end of the box (caretSample.length) + end: [0, caretSample.length], // Other browsers default it to the beginning (0), so check both. + mode: 'get' + }); +*/ caretTest( { description: 'set/getCaretPosition with forced empty selection', diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js index 682eb3d463..bc4b253f97 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js @@ -266,10 +266,8 @@ expectedMultipleBars, expectedSpecialCharacters; - /* - The below three are all identical to or based on real messages. For disambiguations-text, - the bold was removed because it is not yet implemented. - */ + // The below three are all identical to or based on real messages. For disambiguations-text, + // the bold was removed because it is not yet implemented. assert.htmlEqual( formatParse( 'jquerymsg-test-statistics-users' ), -- 2.20.1