Add rule collection-ordering (fixes #190)

docs/rules/collection-ordering.md

@@ -0,0 +1,135 @@
+# Collection Ordering
+Lodash has two methods for sorting a collection by a specific order: `sortBy` and `orderBy`.
+Both methods accept one or several iteratees, but `orderBy` also accepts an optional parameter whether the order is ascending or descending.
+This means that ordering any array by ascending order can be done in several different ways:
+
+```js
+var users = [
+  { 'user': 'fred',   'age': 48 },
+  { 'user': 'barney', 'age': 34 },
+  { 'user': 'fred',   'age': 40 },
+  { 'user': 'barney', 'age': 36 }
+]
+
+_.sortBy(users, 'age')
+_.orderBy(users, 'age')
+_.orderBy(users, ['age'], ['asc'])
+_.orderBy(users, 'age', 'asc')
+```
+
+## Rule Details
+
+This rule takes one argument: an options object with two fields:
+
+- `method`: Which method should be used for ordering. Accepts `sortBy`, `orderBy` or `orderByExplicit` (default is `sortBy`):
+  - `sortBy`: Prefer the `sortBy` method, if no ordering is descending.
+  - `orderBy`: Prefer the `orderBy` method, omitting the orders if all are ascending.
+  - `orderByExplicit`: Prefer the `orderBy` method, and always declare the ordering.
+- `useArray`: When to wrap the iteratees and orders in arrays. Accepts `always` or `as-needed` (default is `always`).
+  - `always`: Wrap the iteratees and ordering in arrays, even if there is a single iteratee or ordering.
+  - `as-needed`: Wrap the iteratees and ordering in arrays only if there is more than one.
+
+
+### When `method` is `sortBy`
+
+The following patterns are considered errors when the `method` option is `sortBy`:
+
+```js
+_.orderBy(arr, [f])
+
+_.orderBy(arr, ["name"])
+
+_.orderBy(arr, [f], ["asc"])
+```
+
+The following patterns are *not* considered warnings:
+
+```js
+_.sortBy(arr, [f])
+
+_.sortBy(arr, ['name'])
+
+_.orderBy(arr, ['name'], ['desc'])
+```
+
+### When `method` is `orderBy`
+
+The following patterns are considered errors when the `method` option is `orderBy`:
+
+```js
+_.sortBy(arr, [f])
+
+_.orderBy(arr, [f], ['asc'])
+```
+
+The following patterns are *not* considered warnings:
+
+```js
+_.orderBy(arr, [f])
+
+_.orderBy(arr, [f], ['desc'])
+```
+
+### When `method` is `orderByExplicit`
+
+The following patterns are considered errors when the `method` option is `orderByExplicit`:
+
+```js
+_.sortBy(arr, [f])
+
+_.orderBy(arr, [f])
+```
+
+The following patterns are *not* considered warnings:
+
+```js
+_.orderBy(arr, [f], ['asc'])
+
+_.orderBy(arr, [f], ['desc'])
+```
+
+### When `useArray` is `always`
+
+The following patterns are considered errors when the `useArray` option is `always`:
+
+```js
+_.sortBy(arr, f)
+
+_.orderBy(arr, f)
+
+_.orderBy(arr, f, 'desc')
+```
+
+The following patterns are *not* considered warnings:
+
+```js
+_.sortBy(arr, [f])
+
+_.sortBy(arr, ['name'])
+
+_.orderBy(arr, ['name'], ['desc'])
+```
+
+### When `useArray` is `as-needed`
+
+The following patterns are considered errors when the `useArray` option is `as-needed`:
+
+```js
+_.sortBy(arr, [f])
+
+_.sortBy(arr, ['name'])
+
+_.orderBy(arr, ['name'], ['desc'])
+```
+
+The following patterns are *not* considered warnings:
+
+```js
+_.sortBy(arr, f)
+
+_.orderBy(arr, f)
+
+_.orderBy(arr, f, 'desc')
+
+_.orderBy(arr, [f, g], ['asc', 'desc'])
+```

src/index.js

@@ -10,6 +10,7 @@ const recommended = {
         'lodash/chain-style': [2, 'as-needed'],
         'lodash/chaining': 2,
         'lodash/collection-method-value': 2,
+        'lodash/collection-ordering': 2,
         'lodash/collection-return': 2,
         'lodash/consistent-compose': [2, 'flow'],
         'lodash/identity-shorthand': [2, 'always'],
@@ -81,7 +82,6 @@ module.exports = {
                 'lodash/no-double-unwrap': 2,
                 'lodash/no-extra-args': 2,
                 'lodash/path-style': [2, 'string'],
-
                 'lodash/prefer-compact': 2,
                 'lodash/prefer-constant': 2,
                 'lodash/prefer-filter': [2, 3],

src/rules/collection-ordering.js

@@ -0,0 +1,98 @@
+/**
+ * @fileoverview Rule to enforce usage of collection method values
+ */
+'use strict'
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+const _ = require('lodash')
+const getDocsUrl = require('../util/getDocsUrl')
+const {getLodashMethodVisitors} = require('../util/lodashUtil')
+const {getMainAlias} = require('../util/methodDataUtil')
+
+function isAsc(node) {
+    return node.type === 'Literal' && node.value === 'asc'
+}
+
+function allAsc(node) {
+    return node.type === 'ArrayExpression' && _.every(node.elements, isAsc)
+}
+
+function canOmitOrders(orderingNode) {
+    return orderingNode && (isAsc(orderingNode) || allAsc(orderingNode))
+}
+
+function ordersAreOmitted(orderingNode) {
+    return !orderingNode
+}
+
+function getNonCollectionArgs(node, callType) {
+    return callType === 'chained' ? node.arguments.slice(0, 2) : node.arguments.slice(1, 3)
+}
+
+function canUseSortBy(orderingNode) {
+    return !orderingNode || isAsc(orderingNode) || allAsc(orderingNode)
+}
+
+function enforceArraysOption(node, useArray, method, context) {
+    if (node) {
+        if (useArray === 'always' && node.type !== 'ArrayExpression') {
+            context.report({node, messageId: 'useArrayAlways', data: {method}})
+        } else if (useArray === 'as-needed' && node.type === 'ArrayExpression' && node.elements.length === 1) {
+            context.report({node, messageId: 'useArrayAsNeeded', data: {method}})
+        }
+    }
+}
+
+module.exports = {
+    meta: {
+        docs: {
+            url: getDocsUrl('collection-ordering')
+        },
+        schema: [{
+            type: 'object',
+            properties: {
+                method: {
+                    enum: ['sortBy', 'orderBy', 'orderByExplicit']
+                },
+                useArray: {
+                    enum: ['always', 'as-needed']
+                }
+            }
+        }],
+        messages: {
+            sortBy: 'Use _.sortBy for sorting only in ascending order.',
+            orderBy: 'Use _.orderBy for ordering in ascending order.',
+            omitOrders: 'Omit the order when all orders are ascending.',
+            orderByExplicit: 'Use _.orderBy and specify the orders.',
+            useArrayAlways: 'Use arrays when calling _.{{method}}.',
+            useArrayAsNeeded: 'Use an array when specifying iteratees and order in _.{{method}}'
+        }
+    },
+    create(context) {
+        const {method: mode = 'sortBy', useArray = 'always'} = context.options[0] || {}
+
+        return getLodashMethodVisitors(context, (node, iteratee, {version, method, callType}) => {
+            const mainAlias = getMainAlias(version, method)
+            if (mainAlias === 'sortBy') {
+                const [iteratees] = getNonCollectionArgs(node, callType)
+                if (mode === 'orderBy' || mode === 'orderByExplicit') {
+                    context.report({node, messageId: mode})
+                }
+                enforceArraysOption(iteratees, useArray, method, context)
+            } else if (mainAlias === 'orderBy') {
+                const [iteratees, ordering] = getNonCollectionArgs(node, callType)
+                if (mode === 'sortBy' && canUseSortBy(ordering)) {
+                    context.report({node, messageId: 'sortBy'})
+                } else if (mode === 'orderBy' && canOmitOrders(ordering)) {
+                    context.report({node, messageId: 'omitOrders'})
+                } else if (mode === 'orderByExplicit' && ordersAreOmitted(ordering)) {
+                    context.report({node, messageId: 'orderByExplicit'})
+                }
+                enforceArraysOption(iteratees, useArray, method, context)
+                enforceArraysOption(ordering, useArray, method, context)
+            }
+        })
+    }
+}

tests/lib/rules/collection-ordering.js

@@ -0,0 +1,99 @@
+'use strict'
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+const rule = require('../../../src/rules/collection-ordering')
+const ruleTesterUtils = require('../testUtil/ruleTesterUtil')
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+const ruleTester = ruleTesterUtils.getRuleTester()
+const {withDefaultPragma, fromMessageId, fromOptions} = require('../testUtil/optionsUtil')
+
+const testCases = {
+    valid: {
+        sortBy: {
+            always: [
+                '_.sortBy(arr, [f])',
+                '_.sortBy(arr, ["name"])',
+                '_.orderBy(arr, ["name"], ["desc"])'
+            ].map(withDefaultPragma),
+            asNeeded: [
+                '_.sortBy(arr, f)',
+                '_.sortBy(arr, "name")',
+                '_.orderBy(arr, "name", "desc")',
+                '_.sortBy(arr, [f, g])',
+                '_(arr).map(f).sortBy(g).value()'
+            ].map(withDefaultPragma).map(fromOptions({options: [{useArray: 'as-needed'}]}))
+        },
+        orderBy: {
+            always: [
+                '_.orderBy(arr, [f])',
+                '_.orderBy(arr, ["name"])',
+                '_.orderBy(arr, ["name"], ["desc"])'
+            ].map(withDefaultPragma).map(fromOptions({options: [{method: 'orderBy'}]})),
+            asNeeded: [
+                '_.orderBy(arr, f)',
+                '_.orderBy(arr, "name")',
+                '_.orderBy(arr, "name", "desc")'
+            ].map(withDefaultPragma).map(fromOptions({options: [{method: 'orderBy', useArray: 'as-needed'}]}))
+        },
+        orderByExplicit: {
+            always: [
+                '_.orderBy(arr, [f], ["asc"])',
+                '_.orderBy(arr, [f], ["desc"])',
+                '_.orderBy(arr, [f, g], ["asc", "asc"])'
+            ].map(withDefaultPragma).map(fromOptions({options: [{method: 'orderByExplicit'}]})),
+            asNeeded: [
+                '_.orderBy(arr, f, "asc")',
+                '_.orderBy(arr, [f, g], ["asc", "asc"])'
+            ].map(withDefaultPragma).map(fromOptions({options: [{method: 'orderByExplicit', useArray: 'as-needed'}]}))
+        }
+    },
+    invalid: {
+        sortBy: [
+            '_.orderBy(arr, [f])',
+            '_.orderBy(arr, ["name"])',
+            '_.orderBy(arr, [f], ["asc"])',
+            '_(arr).map(f).orderBy([g]).value()'
+        ].map(withDefaultPragma).map(fromMessageId('sortBy')),
+        orderBy: [
+            '_.sortBy([f])'
+        ].map(withDefaultPragma).map(fromMessageId('orderBy')).map(fromOptions({options: [{method: 'orderBy'}]})),
+        omitOrders: [
+            '_.orderBy(arr, [f], ["asc"])'
+        ].map(withDefaultPragma).map(fromMessageId('omitOrders')).map(fromOptions({options: [{method: 'orderBy'}]})),
+        orderByExplicit: [
+            '_.sortBy(a, [f])',
+            '_.orderBy(a, [f])'
+        ].map(withDefaultPragma).map(fromMessageId('orderByExplicit')).map(fromOptions({options: [{method: 'orderByExplicit'}]})),
+        useArrayAlways: [
+            '_.sortBy(a, f)'
+        ].map(withDefaultPragma).map(fromMessageId('useArrayAlways')),
+        useArrayAsNeeded: [
+            '_.sortBy(a, [f])'
+        ].map(withDefaultPragma).map(fromMessageId('useArrayAsNeeded')).map(fromOptions({options: [{useArray: 'as-needed'}]}))
+    }
+}
+ruleTester.run('collection-ordering', rule, {
+    valid: [
+        ...testCases.valid.sortBy.always,
+        ...testCases.valid.sortBy.asNeeded,
+        ...testCases.valid.orderBy.always,
+        ...testCases.valid.orderBy.asNeeded,
+        ...testCases.valid.orderByExplicit.always,
+        ...testCases.valid.orderByExplicit.asNeeded
+    ],
+    invalid: [
+        ...testCases.invalid.sortBy,
+        ...testCases.invalid.orderBy,
+        ...testCases.invalid.omitOrders,
+        ...testCases.invalid.orderByExplicit,
+        ...testCases.invalid.useArrayAlways,
+        ...testCases.invalid.useArrayAsNeeded
+    ]
+})

tests/lib/testUtil/optionsUtil.js

@@ -5,6 +5,10 @@ function fromMessage(message) {
     return fromOptions({errors: [{message}]})
 }
 
+function fromMessageId(messageId) {
+    return fromOptions({errors: [{messageId}]})
+}
+
 function fromOptions(options) {
     return function (testCase) {
         return _.isString(testCase) ? _.assign({code: testCase}, options) : _.defaultsDeep(testCase, options)
@@ -13,6 +17,7 @@ function fromOptions(options) {
 
 module.exports = {
     fromMessage,
+    fromMessageId,
     fromVersion3: fromOptions({settings: {lodash: {version: 3}}}),
     fromVersion3WithDefaultPragma: fromOptions({settings: {lodash: {version: 3, pragma: '_'}}}),
     fromOptions,