From 446545a6eef73565258aa35874cdab2bcaa746a6 Mon Sep 17 00:00:00 2001 From: Jared Wein Date: Thu, 5 Jan 2017 13:30:25 -0500 Subject: [PATCH] Bug 1329013 - Enable no-lone-blocks rule for eslint and remove the seven unnecessary blocks that it found. r=mossop MozReview-Commit-ID: 9DJGO4en378 --HG-- extra : rebase_source : 820f939e9a751c9be4461ef225fa73fff7829675 --- toolkit/.eslintrc.js | 3 + .../tests/unit/test_stringGroups.js | 128 ++++----- .../places/tests/unit/test_sync_utils.js | 44 ++- toolkit/content/widgets/calendar.js | 264 +++++++++--------- 4 files changed, 214 insertions(+), 225 deletions(-) diff --git a/toolkit/.eslintrc.js b/toolkit/.eslintrc.js index 7ab22efea96a..72d4bb14ba02 100644 --- a/toolkit/.eslintrc.js +++ b/toolkit/.eslintrc.js @@ -84,6 +84,9 @@ module.exports = { // No labels "no-labels": "error", + // Disallow unnecessary nested blocks + "no-lone-blocks": "error", + // If an if block ends with a return no need for an else block "no-else-return": "error", diff --git a/toolkit/components/contentprefs/tests/unit/test_stringGroups.js b/toolkit/components/contentprefs/tests/unit/test_stringGroups.js index afd7e5c5ed77..74792520509b 100644 --- a/toolkit/components/contentprefs/tests/unit/test_stringGroups.js +++ b/toolkit/components/contentprefs/tests/unit/test_stringGroups.js @@ -17,85 +17,79 @@ function run_test() { var stringURI = "www.example.com"; // typeof = "string" var stringObjectURI = new String("www.example.com"); // typeof = "object" - { - // First check that all the methods work or don't work. - function simple_test_methods(aGroup, shouldThrow) { - var prefName = "test.pref.0"; - var prefValue = Math.floor(Math.random() * 100); - - if (shouldThrow) { - do_check_thrown(function() { cps.getPref(aGroup, prefName); }); - do_check_thrown(function() { cps.setPref(aGroup, prefName, prefValue); }); - do_check_thrown(function() { cps.hasPref(aGroup, prefName); }); - do_check_thrown(function() { cps.removePref(aGroup, prefName); }); - do_check_thrown(function() { cps.getPrefs(aGroup); }); - } else { - do_check_eq(cps.setPref(aGroup, prefName, prefValue), undefined); - do_check_true(cps.hasPref(aGroup, prefName)); - do_check_eq(cps.getPref(aGroup, prefName), prefValue); - do_check_eq(cps.removePref(aGroup, prefName), undefined); - do_check_false(cps.hasPref(aGroup, prefName)); - } - } - - simple_test_methods(cps, true); // arbitrary nsISupports object, should throw too - simple_test_methods(anObject, true); - simple_test_methods(uri, false); - simple_test_methods(stringURI, false); - simple_test_methods(stringObjectURI, false); - } - - { - // Now we'll check that each argument produces the same result. - function complex_test_methods(aGroup) { - var prefName = "test.pref.1"; - var prefValue = Math.floor(Math.random() * 100); + // First check that all the methods work or don't work. + function simple_test_methods(aGroup, shouldThrow) { + var prefName = "test.pref.0"; + var prefValue = Math.floor(Math.random() * 100); + if (shouldThrow) { + do_check_thrown(function() { cps.getPref(aGroup, prefName); }); + do_check_thrown(function() { cps.setPref(aGroup, prefName, prefValue); }); + do_check_thrown(function() { cps.hasPref(aGroup, prefName); }); + do_check_thrown(function() { cps.removePref(aGroup, prefName); }); + do_check_thrown(function() { cps.getPrefs(aGroup); }); + } else { do_check_eq(cps.setPref(aGroup, prefName, prefValue), undefined); - - do_check_true(cps.hasPref(uri, prefName)); - do_check_true(cps.hasPref(stringURI, prefName)); - do_check_true(cps.hasPref(stringObjectURI, prefName)); - - do_check_eq(cps.getPref(uri, prefName), prefValue); - do_check_eq(cps.getPref(stringURI, prefName), prefValue); - do_check_eq(cps.getPref(stringObjectURI, prefName), prefValue); - + do_check_true(cps.hasPref(aGroup, prefName)); + do_check_eq(cps.getPref(aGroup, prefName), prefValue); do_check_eq(cps.removePref(aGroup, prefName), undefined); - - do_check_false(cps.hasPref(uri, prefName)); - do_check_false(cps.hasPref(stringURI, prefName)); - do_check_false(cps.hasPref(stringObjectURI, prefName)); + do_check_false(cps.hasPref(aGroup, prefName)); } - - complex_test_methods(uri); - complex_test_methods(stringURI); - complex_test_methods(stringObjectURI); } - { - // test getPrefs returns the same prefs - do_check_eq(cps.setPref(stringObjectURI, "test.5", 5), undefined); - do_check_eq(cps.setPref(stringURI, "test.2", 2), undefined); - do_check_eq(cps.setPref(uri, "test.1", 1), undefined); + simple_test_methods(cps, true); // arbitrary nsISupports object, should throw too + simple_test_methods(anObject, true); + simple_test_methods(uri, false); + simple_test_methods(stringURI, false); + simple_test_methods(stringObjectURI, false); - enumerateAndCheck(cps.getPrefs(uri), 8, ["test.1", "test.2", "test.5"]); - enumerateAndCheck(cps.getPrefs(stringURI), 8, ["test.1", "test.2", "test.5"]); - enumerateAndCheck(cps.getPrefs(stringObjectURI), 8, ["test.1", "test.2", "test.5"]); + // Now we'll check that each argument produces the same result. + function complex_test_methods(aGroup) { + var prefName = "test.pref.1"; + var prefValue = Math.floor(Math.random() * 100); - do_check_eq(cps.setPref(uri, "test.4", 4), undefined); - do_check_eq(cps.setPref(stringObjectURI, "test.0", 0), undefined); + do_check_eq(cps.setPref(aGroup, prefName, prefValue), undefined); - enumerateAndCheck(cps.getPrefs(uri), 12, ["test.0", "test.1", "test.2", "test.4", "test.5"]); - enumerateAndCheck(cps.getPrefs(stringURI), 12, ["test.0", "test.1", "test.2", "test.4", "test.5"]); - enumerateAndCheck(cps.getPrefs(stringObjectURI), 12, ["test.0", "test.1", "test.2", "test.4", "test.5"]); + do_check_true(cps.hasPref(uri, prefName)); + do_check_true(cps.hasPref(stringURI, prefName)); + do_check_true(cps.hasPref(stringObjectURI, prefName)); - do_check_eq(cps.setPref(stringURI, "test.3", 3), undefined); + do_check_eq(cps.getPref(uri, prefName), prefValue); + do_check_eq(cps.getPref(stringURI, prefName), prefValue); + do_check_eq(cps.getPref(stringObjectURI, prefName), prefValue); - enumerateAndCheck(cps.getPrefs(uri), 15, ["test.0", "test.1", "test.2", "test.3", "test.4", "test.5"]); - enumerateAndCheck(cps.getPrefs(stringURI), 15, ["test.0", "test.1", "test.2", "test.3", "test.4", "test.5"]); - enumerateAndCheck(cps.getPrefs(stringObjectURI), 15, ["test.0", "test.1", "test.2", "test.3", "test.4", "test.5"]); + do_check_eq(cps.removePref(aGroup, prefName), undefined); + + do_check_false(cps.hasPref(uri, prefName)); + do_check_false(cps.hasPref(stringURI, prefName)); + do_check_false(cps.hasPref(stringObjectURI, prefName)); } + + complex_test_methods(uri); + complex_test_methods(stringURI); + complex_test_methods(stringObjectURI); + + // test getPrefs returns the same prefs + do_check_eq(cps.setPref(stringObjectURI, "test.5", 5), undefined); + do_check_eq(cps.setPref(stringURI, "test.2", 2), undefined); + do_check_eq(cps.setPref(uri, "test.1", 1), undefined); + + enumerateAndCheck(cps.getPrefs(uri), 8, ["test.1", "test.2", "test.5"]); + enumerateAndCheck(cps.getPrefs(stringURI), 8, ["test.1", "test.2", "test.5"]); + enumerateAndCheck(cps.getPrefs(stringObjectURI), 8, ["test.1", "test.2", "test.5"]); + + do_check_eq(cps.setPref(uri, "test.4", 4), undefined); + do_check_eq(cps.setPref(stringObjectURI, "test.0", 0), undefined); + + enumerateAndCheck(cps.getPrefs(uri), 12, ["test.0", "test.1", "test.2", "test.4", "test.5"]); + enumerateAndCheck(cps.getPrefs(stringURI), 12, ["test.0", "test.1", "test.2", "test.4", "test.5"]); + enumerateAndCheck(cps.getPrefs(stringObjectURI), 12, ["test.0", "test.1", "test.2", "test.4", "test.5"]); + + do_check_eq(cps.setPref(stringURI, "test.3", 3), undefined); + + enumerateAndCheck(cps.getPrefs(uri), 15, ["test.0", "test.1", "test.2", "test.3", "test.4", "test.5"]); + enumerateAndCheck(cps.getPrefs(stringURI), 15, ["test.0", "test.1", "test.2", "test.3", "test.4", "test.5"]); + enumerateAndCheck(cps.getPrefs(stringObjectURI), 15, ["test.0", "test.1", "test.2", "test.3", "test.4", "test.5"]); } function do_check_thrown(aCallback) { diff --git a/toolkit/components/places/tests/unit/test_sync_utils.js b/toolkit/components/places/tests/unit/test_sync_utils.js index df20cceae829..245bb0a6d5cd 100644 --- a/toolkit/components/places/tests/unit/test_sync_utils.js +++ b/toolkit/components/places/tests/unit/test_sync_utils.js @@ -1219,24 +1219,20 @@ add_task(function* test_insert_tag_query() { } do_print("Use the public tagging API to ensure we added the tag correctly"); - { - yield PlacesUtils.bookmarks.insert({ - parentGuid: PlacesUtils.bookmarks.menuGuid, - type: PlacesUtils.bookmarks.TYPE_BOOKMARK, - url: "https://mozilla.org", - title: "Mozilla", - }); - PlacesUtils.tagging.tagURI(uri("https://mozilla.org"), ["taggy"]); - assertURLHasTags("https://mozilla.org/", ["taggy"], - "Should set tags using the tagging API"); - } + yield PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.menuGuid, + type: PlacesUtils.bookmarks.TYPE_BOOKMARK, + url: "https://mozilla.org", + title: "Mozilla", + }); + PlacesUtils.tagging.tagURI(uri("https://mozilla.org"), ["taggy"]); + assertURLHasTags("https://mozilla.org/", ["taggy"], + "Should set tags using the tagging API"); do_print("Removing the tag should clean up the tag folder"); - { - PlacesUtils.tagging.untagURI(uri("https://mozilla.org"), null); - deepEqual(PlacesUtils.tagging.allTags, [], - "Should remove tag folder once last item is untagged"); - } + PlacesUtils.tagging.untagURI(uri("https://mozilla.org"), null); + deepEqual(PlacesUtils.tagging.allTags, [], + "Should remove tag folder once last item is untagged"); yield PlacesUtils.bookmarks.eraseEverything(); yield PlacesSyncUtils.bookmarks.reset(); @@ -1269,15 +1265,13 @@ add_task(function* test_insert_orphans() { } do_print("Insert the grandparent"); - { - yield PlacesSyncUtils.bookmarks.insert({ - kind: "folder", - parentSyncId: "menu", - syncId: grandParentGuid, - }); - equal(PlacesUtils.annotations.getItemAnnotation(childId, SYNC_PARENT_ANNO), - parentGuid, "Child should still have orphan anno"); - } + yield PlacesSyncUtils.bookmarks.insert({ + kind: "folder", + parentSyncId: "menu", + syncId: grandParentGuid, + }); + equal(PlacesUtils.annotations.getItemAnnotation(childId, SYNC_PARENT_ANNO), + parentGuid, "Child should still have orphan anno"); do_print("Insert the missing parent"); { diff --git a/toolkit/content/widgets/calendar.js b/toolkit/content/widgets/calendar.js index 72e0d9d610dc..e6d4ee4ce9c3 100644 --- a/toolkit/content/widgets/calendar.js +++ b/toolkit/content/widgets/calendar.js @@ -35,138 +35,136 @@ function Calendar(options, context) { this._attachEventListeners(); } -{ - Calendar.prototype = { +Calendar.prototype = { - /** - * Set new properties and render them. - * - * @param {Object} props - * { - * {Boolean} isVisible: Whether or not the calendar is in view - * {Array} days: Data for days - * { - * {Number} dateValue: Date in milliseconds - * {Number} textContent - * {Array} classNames - * } - * {Array} weekHeaders: Data for weekHeaders - * { - * {Number} textContent - * {Array} classNames - * } - * {Function} getDayString: Transform day number to string - * {Function} getWeekHeaderString: Transform day of week number to string - * {Function} setValue: Set value for dateKeeper - * {Number} selectionValue: The selection date value - * } - */ - setProps(props) { - if (props.isVisible) { - // Transform the days and weekHeaders array for rendering - const days = props.days.map(({ dateValue, textContent, classNames }) => { - return { - dateValue, - textContent: props.getDayString(textContent), - className: dateValue == props.selectionValue ? - classNames.concat("selection").join(" ") : - classNames.join(" ") - }; - }); - const weekHeaders = props.weekHeaders.map(({ textContent, classNames }) => { - return { - textContent: props.getWeekHeaderString(textContent), - className: classNames.join(" ") - }; - }); - // Update the DOM nodes states - this._render({ - elements: this.elements.daysView, - items: days, - prevState: this.state.days - }); - this._render({ - elements: this.elements.weekHeaders, - items: weekHeaders, - prevState: this.state.weekHeaders, - }); - // Update the state to current - this.state.days = days; - this.state.weekHeaders = weekHeaders; - } - - this.props = Object.assign(this.props, props); - }, - - /** - * Render the items onto the DOM nodes - * @param {Object} - * { - * {Array} elements - * {Array} items - * {Array} prevState: state of items from last render - * } - */ - _render({ elements, items, prevState }) { - for (let i = 0, l = items.length; i < l; i++) { - let el = elements[i]; - - // Check if state from last render has changed, if so, update the elements - if (!prevState[i] || prevState[i].textContent != items[i].textContent) { - el.textContent = items[i].textContent; - } - if (!prevState[i] || prevState[i].className != items[i].className) { - el.className = items[i].className; - } - } - }, - - /** - * Generate DOM nodes - * - * @param {Number} size: Number of nodes to generate - * @param {DOMElement} context: Element to append the nodes to - * @return {Array} - */ - _generateNodes(size, context) { - let frag = document.createDocumentFragment(); - let refs = []; - - for (let i = 0; i < size; i++) { - let el = document.createElement("div"); - el.dataset.id = i; - refs.push(el); - frag.appendChild(el); - } - context.appendChild(frag); - - return refs; - }, - - /** - * Handle events - * @param {DOMEvent} event - */ - handleEvent(event) { - switch (event.type) { - case "click": { - if (event.target.parentNode == this.context.daysView) { - let targetId = event.target.dataset.id; - this.props.setValue({ - selectionValue: this.props.days[targetId].dateValue, - dateValue: this.props.days[targetId].dateValue - }); - } - break; - } - } - }, - - /** - * Attach event listener to daysView - */ - _attachEventListeners() { - this.context.daysView.addEventListener("click", this); + /** + * Set new properties and render them. + * + * @param {Object} props + * { + * {Boolean} isVisible: Whether or not the calendar is in view + * {Array} days: Data for days + * { + * {Number} dateValue: Date in milliseconds + * {Number} textContent + * {Array} classNames + * } + * {Array} weekHeaders: Data for weekHeaders + * { + * {Number} textContent + * {Array} classNames + * } + * {Function} getDayString: Transform day number to string + * {Function} getWeekHeaderString: Transform day of week number to string + * {Function} setValue: Set value for dateKeeper + * {Number} selectionValue: The selection date value + * } + */ + setProps(props) { + if (props.isVisible) { + // Transform the days and weekHeaders array for rendering + const days = props.days.map(({ dateValue, textContent, classNames }) => { + return { + dateValue, + textContent: props.getDayString(textContent), + className: dateValue == props.selectionValue ? + classNames.concat("selection").join(" ") : + classNames.join(" ") + }; + }); + const weekHeaders = props.weekHeaders.map(({ textContent, classNames }) => { + return { + textContent: props.getWeekHeaderString(textContent), + className: classNames.join(" ") + }; + }); + // Update the DOM nodes states + this._render({ + elements: this.elements.daysView, + items: days, + prevState: this.state.days + }); + this._render({ + elements: this.elements.weekHeaders, + items: weekHeaders, + prevState: this.state.weekHeaders, + }); + // Update the state to current + this.state.days = days; + this.state.weekHeaders = weekHeaders; } - }; -} + + this.props = Object.assign(this.props, props); + }, + + /** + * Render the items onto the DOM nodes + * @param {Object} + * { + * {Array} elements + * {Array} items + * {Array} prevState: state of items from last render + * } + */ + _render({ elements, items, prevState }) { + for (let i = 0, l = items.length; i < l; i++) { + let el = elements[i]; + + // Check if state from last render has changed, if so, update the elements + if (!prevState[i] || prevState[i].textContent != items[i].textContent) { + el.textContent = items[i].textContent; + } + if (!prevState[i] || prevState[i].className != items[i].className) { + el.className = items[i].className; + } + } + }, + + /** + * Generate DOM nodes + * + * @param {Number} size: Number of nodes to generate + * @param {DOMElement} context: Element to append the nodes to + * @return {Array} + */ + _generateNodes(size, context) { + let frag = document.createDocumentFragment(); + let refs = []; + + for (let i = 0; i < size; i++) { + let el = document.createElement("div"); + el.dataset.id = i; + refs.push(el); + frag.appendChild(el); + } + context.appendChild(frag); + + return refs; + }, + + /** + * Handle events + * @param {DOMEvent} event + */ + handleEvent(event) { + switch (event.type) { + case "click": { + if (event.target.parentNode == this.context.daysView) { + let targetId = event.target.dataset.id; + this.props.setValue({ + selectionValue: this.props.days[targetId].dateValue, + dateValue: this.props.days[targetId].dateValue + }); + } + break; + } + } + }, + + /** + * Attach event listener to daysView + */ + _attachEventListeners() { + this.context.daysView.addEventListener("click", this); + } +};