support long property paths and conjunctions in prefer-filter, prefer-reject and prop-shorthand

Author: mariawix

README.md

@@ -37,8 +37,8 @@ Finally, enable all of the rules that you would like to use.
     "lodash3/prefer-chain": 1,
     "lodash3/preferred-alias": 1,
     "lodash3/no-single-chain": 1,
-    "lodash3/prefer-reject": 1,
-    "lodash3/prefer-filter": 1,
+    "lodash3/prefer-reject": [1,3],
+    "lodash3/prefer-filter": [1,3],
     "lodash3/no-unnecessary-bind": 1,
     "lodash3/unwrap": 1,
     "lodash3/prefer-compact": 1,

docs/rules/matches-shorthand.md

@@ -4,7 +4,7 @@ When using certain method in lodash such as filter, code can be shorter and more
 
 ## Rule Details
 
-This rule takes one argument, maximum path length.
+This rule takes one argument, maximum path length (default is 3).
 
 The following patterns are considered warnings:
 

docs/rules/prefer-filter.md

@@ -4,25 +4,25 @@ When using _.forEach with a single `if` statement, you should probably use `_.fi
 
 ## Rule Details
 
-This rule takes no arguments.
+This rule takes one argument, maximum path length (default is 3).
 
 The following patterns are considered warnings:
 
 ```js
-
-_(arr).forEach(function(x) { 
-  if (x.a) {
+_(users).forEach(function(user) { 
+  if (user.name.familyName) {
   // ...
   }
 });
 
-  if (!x.a) {
+_(users).forEach(function(user) { 
+  if (!user.active) {
   // ...
   }
 });
 
-_.forEach(arr, function(x) { 
-  if (x.a === b) {
+_.forEach(users, function(user) { 
+  if (user.name.givenName === 'Bob') {
   // ...
   }
 });
@@ -31,7 +31,8 @@ _.forEach(arr, function(x) {
 The following patterns are not considered warnings:
 
 ```js
-
-var x = _.filter(arr, function(x) {return !x.a && p});
+var x = _.filter(users, function(user) {
+  return !user.active && user.name.givenName === 'Bob'
+});
 
 ```

docs/rules/prefer-matches.md

@@ -4,7 +4,7 @@ When writing an expression like `a.foo === 1 && a.bar === 2 && a.baz === 3`, it
 
 ## Rule Details
 
-This rule takes one argument - the minimal length of the condition(default is 3).
+This rule takes one argument - the minimal length of the condition (default is 3).
 
 The following patterns are considered warnings:
 

docs/rules/prefer-reject.md

@@ -4,24 +4,30 @@ When using _.filter with a negative condition, it could improve readability by s
 
 ## Rule Details
 
-This rule takes no arguments.
+This rule takes one argument, maximum path length (default is 3).
 
 The following patterns are considered warnings:
 
 ```js
+_.filter(users, function(user) {
+  return user.name.givenName !== 'Bob';
+});
 
-_.filter(arr, function(x) { return x.a !== b});
-
-_.filter(arr, function(x) {return !x.isSomething})
+_.filter(users, function(user) {
+  return !user.isSomething;
+});
 ```
 
 The following patterns are not considered warnings:
 
 ```js
+_.filter(users, function(user) {
+  return !user.active && isSomething;
+});
 
-var x = _.filter(arr, function(x) {return !x.a && p});
-
-var x = _.filter(arr, function(x) {return !f(x)}; // The function f could take multiple arguments, e.g. parseInt 
+_.filter(users, function(user) {
+  return !f(user);     // The function f could take multiple arguments, e.g. parseInt 
+}); 
 ```
 
 

docs/rules/prop-shorthand.md

@@ -9,13 +9,15 @@ This rule takes no arguments.
 The following patterns are considered warnings:
 
 ```js
-var ids = _.map([], function (i) { return i.id; });
+var ids = _.map([], function (user) {
+  return user.name.familyName;
+});
 ```
 
 The following patterns are not considered warnings:
 
 ```js
-var ids = _.map([], 'id');
+var ids = _.map([], 'name.familyName');
 ```
 
 

lib/rules/matches-prop-shorthand.js

@@ -15,7 +15,7 @@ module.exports = function (context) {
     var SUPPORT_MATCHES_PROPERTY_STYLE_CB = ['find', 'detect', 'filter', 'select', 'reject', 'findIndex', 'findLastIndex', 'some', 'every'];
 
     function shouldPreferMatches(func) {
-        return astUtil.isEqEqEqToParamMember(1, astUtil.getValueReturnedInFirstLine(func), astUtil.getFirstParamName(func));
+        return astUtil.isEqEqEqToParamMember(astUtil.getValueReturnedInFirstLine(func), astUtil.getFirstParamName(func), 1);
     }
 
     function methodSupportsShorthand(node) {

lib/rules/prefer-filter.js

@@ -19,8 +19,9 @@ module.exports = function (context) {
     }
 
     function canBeShorthand(exp, paramName) {
-        return astUtil.isIdentifierOfParam(exp, paramName) || astUtil.isMemberExpOfArg(exp, paramName) || astUtil.isNegationOfParamMember(exp, paramName) ||
-            astUtil.isEqEqEqToParamMember(maxPropertyPathLength, exp, paramName) || astUtil.isNotEqEqToParamMember(exp, paramName);
+        return astUtil.isIdentifierOfParam(exp, paramName) ||
+            astUtil.isMemberExpOfArg(exp, paramName, maxPropertyPathLength) || astUtil.isNegationOfParamMember(exp, paramName, maxPropertyPathLength) ||
+            astUtil.isEqEqEqToParamMember(exp, paramName, maxPropertyPathLength) || astUtil.isNotEqEqToParamMember(exp, paramName, maxPropertyPathLength);
     }
 
     function onlyHasSimplifiableIf(func) {

lib/rules/prefer-reject.js

@@ -11,11 +11,14 @@ module.exports = function (context) {
     var lodashUtil = require('../util/lodashUtil');
     var astUtil = require('../util/astUtil');
 
+    var DEFAULT_MAX_PROPERTY_PATH_LENGTH = 3;
+    var maxPropertyPathLength = parseInt(context.options[0], 10) || DEFAULT_MAX_PROPERTY_PATH_LENGTH;
+
     function isNegativeExpressionFunction(func) {
         var returnValue = astUtil.getValueReturnedInFirstLine(func);
         var firstParamName = astUtil.getFirstParamName(func);
-        return astUtil.isNegationOfParamMember(returnValue, firstParamName) ||
-            astUtil.isNotEqEqToParamMember(returnValue, firstParamName);
+        return astUtil.isNegationOfParamMember(returnValue, firstParamName, maxPropertyPathLength) ||
+            astUtil.isNotEqEqToParamMember(returnValue, firstParamName, maxPropertyPathLength);
     }
 
     return {

lib/rules/prop-shorthand.js

@@ -14,7 +14,7 @@ module.exports = function (context) {
     var astUtil = require('../util/astUtil');
 
     function canUseShorthand(func) {
-        return astUtil.isMemberExpOfArg(astUtil.getValueReturnedInFirstLine(func), astUtil.getFirstParamName(func));
+        return astUtil.isMemberExpOfArg(astUtil.getValueReturnedInFirstLine(func), astUtil.getFirstParamName(func), Number.MAX_VALUE);
     }
 
     function methodSupportsShorthand(node) {

lib/util/astUtil.js

@@ -35,13 +35,9 @@ function isPropAccess(node) {
         (node.computed === true && node.property.type === 'Literal' && _.isString(node.property.value));
 }
 
-function isMemberExpOfArg(node, argName) {
-    return argName && _.get(node, 'type') === 'MemberExpression' && _.get(node, 'object.name') === argName && isPropAccess(node);
-}
-
-function isNestedMemberExpOfArg(node, argName, maxPropertyPathLength) {
+function isMemberExpOfArg(node, argName, maxPropertyPathLength) {
     if (maxPropertyPathLength > 0 && argName && node && node.object && node.type === 'MemberExpression' && isPropAccess(node)) {
-        return node.object.name === argName || isNestedMemberExpOfArg(node.object, argName, maxPropertyPathLength - 1);
+        return node.object.name === argName || isMemberExpOfArg(node.object, argName, maxPropertyPathLength - 1);
     }
     return false;
 }
@@ -62,25 +58,25 @@ function isObjectOfMethodCall(node) {
     return _.get(node, 'parent.object') === node && _.get(node, 'parent.parent.type') === 'CallExpression';
 }
 
-function isBinaryExpWithParamMember(operator, maxPropertyPathLength, exp, paramName) {
+function isBinaryExpWithParamMember(operator, exp, paramName, maxPropertyPathLength) {
     return exp && exp.type === 'BinaryExpression' && exp.operator === operator &&
-          (isNestedMemberExpOfArg(exp.left, paramName, maxPropertyPathLength) || isNestedMemberExpOfArg(exp.right, paramName, maxPropertyPathLength));
+          (isMemberExpOfArg(exp.left, paramName, maxPropertyPathLength) || isMemberExpOfArg(exp.right, paramName, maxPropertyPathLength));
 }
 
 function isConjunctionOfEqEqEqToParamMember(exp, paramName, maxPropertyPathLength) {
     if (exp && exp.type === 'LogicalExpression' && exp.operator === '&&') {
         return isConjunctionOfEqEqEqToParamMember(exp.left, paramName, maxPropertyPathLength) &&
                isConjunctionOfEqEqEqToParamMember(exp.right, paramName, maxPropertyPathLength);
     }
-    return isBinaryExpWithParamMember('===', maxPropertyPathLength, exp, paramName);
+    return isBinaryExpWithParamMember('===', exp, paramName, maxPropertyPathLength);
 }
 
 function isNegationExpression(exp) {
     return exp && exp.type === 'UnaryExpression' && exp.operator === '!';
 }
 
-function isNegationOfParamMember(exp, firstParamName) {
-    return isNegationExpression(exp) && isMemberExpOfArg(exp.argument, firstParamName);
+function isNegationOfParamMember(exp, firstParamName, maxPropertyPathLength) {
+    return isNegationExpression(exp) && isMemberExpOfArg(exp.argument, firstParamName, maxPropertyPathLength);
 }
 
 function isIdentifierOfParam(exp, paramName) {
@@ -139,7 +135,7 @@ module.exports = {
     hasOnlyOneStatement: hasOnlyOneStatement,
     isObjectOfMethodCall: isObjectOfMethodCall,
     isEqEqEqToParamMember: isBinaryExpWithParamMember.bind(null, '==='),
-    isNotEqEqToParamMember: isBinaryExpWithParamMember.bind(null, '!==', 1),
+    isNotEqEqToParamMember: isBinaryExpWithParamMember.bind(null, '!=='),
     isNegationOfParamMember: isNegationOfParamMember,
     isIdentifierOfParam: isIdentifierOfParam,
     isNegationExpression: isNegationExpression,

tests/lib/rules/prefer-filter.js

@@ -21,19 +21,19 @@ ruleTester.run('prefer-filter', rule, {
         '_.forEach(arr, function(x, y) { if (x){} })'
     ],
     invalid: [{
-        code: '_(arr).forEach(function(x) { if (x.a) {}})',
+        code: '_(arr).forEach(function(x) { if (x.a.b.c) {}})',
         errors: [ruleError]
     }, {
         code: '_(arr).forEach(function(x) { if (x) {}})',
         errors: [ruleError]
     }, {
-        code: '_.forEach(arr, function(x) { if (x.a === b) {}})',
+        code: '_.forEach(arr, function(x) { if (x.a.b.c === d) {}})',
         errors: [ruleError]
     }, {
-        code: '_.forEach(arr, function(x) { if (x.a !== b) {}})',
+        code: '_.forEach(arr, function(x) { if (x.a.b.c !== d) {}})',
         errors: [ruleError]
     }, {
-        code: '_.forEach(arr, function(x) { if (!x.a) {}})',
+        code: '_.forEach(arr, function(x) { if (!x.a.b.c) {}})',
         errors: [ruleError]
     }]
 });

tests/lib/rules/prefer-reject.js

@@ -15,22 +15,20 @@ var ruleTester = new RuleTester();
 var ruleError = {message: 'Prefer _.reject over negative condition'};
 ruleTester.run('prefer-reject', rule, {
     valid: [
-        'var x = _.filter(arr, function(x) {return !x.a && p})'
+        '_.filter(users, function(user) {return !user.active && isSomething;});',
+        '_.filter(users, function(user) {return !f(user);});'
     ],
     invalid: [{
-        code: '_(arr).map(f).filter(function(x) {return !x.isSomething})',
+        code: '_(users).map(t).filter(function(user) {return !user.name.givenName})',
         errors: [ruleError]
     }, {
-        code: '_.filter(arr, function(x) { return x.a !== b})',
+        code: '_.filter(users, function(user) {return user.name.givenName !== "Bob";});',
         errors: [ruleError]
     }, {
-        code: '_.filter(arr, function(x) { return b !== x.a})',
+        code: '_.filter(users, function(user) {return !user.isSomething;});',
         errors: [ruleError]
     }, {
-        code: '_.filter(arr, function(x) {return !x.isSomething})',
-        errors: [ruleError]
-    }, {
-        code: '_.filter(arr, x => !x.isSomething)',
+        code: '_.filter(arr, user => !user.active)',
         ecmaFeatures: {arrowFunctions: true},
         errors: [ruleError]
     }]

tests/lib/rules/prop-shorthand.js

@@ -26,19 +26,19 @@ ruleTester.run('prop-shorthand', rule, {
         'var r = _.map([])'
     ],
     invalid: [{
-        code: 'var ids = _(users).map(function (i) { return i.id; });',
+        code: 'var ids = _(arr).map(function (i) { return i.a.b.c; });',
         errors: errors
     }, {
-        code: 'var ids = _.map([], function (i) { return i.id; });',
+        code: 'var ids = _.map([], function (i) { return i.a; });',
         errors: errors
     }, {
-        code: 'var ids = _(users).map("x").map("y").map(function (i) { return i.id; });',
+        code: 'var ids = _(arr).map("x").map("y").map(function (i) { return i.a.b; });',
         errors: errors
     }, {
-        code: 'var ids = _.map([], function (i) { return i["id"]; });',
+        code: 'var ids = _.map([], function (i) { return i["a"]; });',
         errors: errors
     }, {
-        code: 'var ids = _.map([], i => i.id);',
+        code: 'var ids = _.map([], i => i.a.b.c);',
         ecmaFeatures: {arrowFunctions: true},
         errors: errors
     }]