Skip to content

Commit

Permalink
add rule prefer-get
Browse files Browse the repository at this point in the history
  • Loading branch information
ganimomer committed Oct 8, 2015
1 parent b590937 commit ffbe336
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 1 deletion.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ Finally, enable all of the rules that you would like to use.
* [prefer-lodash-method](docs/rules/prefer-lodash-method.md): Prefer using Lodash collection methods (e.g. `_.map`) over native array methods.
* [prefer-lodash-typecheck](docs/rules/prefer-lodash-typecheck.md): Prefer using `_.is*` methods over `typeof` and `instanceof` checks when applicable.
* [no-commit](docs/rules/no-commit.md): Do not use `.commit()` on chains that should end with `.value()`
* [prefer-get](docs/rules/prefer-get.md): Prefer using `_.get` or `_.has` over expression chains like `a && a.b && a.b.c`.


# License
Expand Down
36 changes: 36 additions & 0 deletions docs/rules/prefer-get.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Prefer get

When writing an expression like `a && a.b && a.b.c` just to make sure the path exists, it is more readable to use the functions `_.get`, `_.set` and `_.has` instead.

## Rule Details

This rule takes one argument - the minimal depth (default is 3).

The following patterns are considered warnings:

```js

var isThree = a && a.b && a.b.c === 3;

if (a && a.b && a.b.c) {
// ...
}

```

The following patterns are not considered warnings:

```js

var isThree = _.get(a, 'b.c') === 3;

if (_.has(a, 'b.c')) {
// ...
}

```


## When Not To Use It

If you do not want to enforce using `get`, you should not use this rule.
43 changes: 43 additions & 0 deletions lib/rules/prefer-get.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* @fileoverview Rule to check if an "&&" experssion should be a call to _.get or _.has
*/
'use strict';

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = function (context) {
var DEFAULT_LENGTH = 3;
var astUtil = require('../util/astUtil');
var ruleDepth = Math.max(parseInt(context.options[0], 10) || DEFAULT_LENGTH, 2);

var expStates = [];
function getState() {
return expStates[expStates.length - 1] || {depth: 0};
}

function shouldCheckDeeper(node, nodeRight, toCompare) {
return node.operator === '&&' && nodeRight && (!toCompare || astUtil.isEquivalentExp(nodeRight, toCompare));
}

return {
LogicalExpression: function (node) {
var state = getState();
var rightMemberExp = (node.right.type === 'MemberExpression' || state.depth > 0) ? node.right : node.right.left;

if (shouldCheckDeeper(node, rightMemberExp, state.node)) {
expStates.push({depth: state.depth + 1, node: rightMemberExp.object});
if (astUtil.isEquivalentExp(node.left, rightMemberExp.object) && state.depth >= ruleDepth - 2) {
context.report(node, "Prefer _.get or _.has over an '&&' chain");
}
}
},
'LogicalExpression:exit': function (node) {
var state = getState();
if (state && state.node === node.right.object) {
expStates.pop();
}
}
};
};
17 changes: 16 additions & 1 deletion lib/util/astUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,20 @@ function isCallFromObject(node, objName) {
return node && node.type === 'CallExpression' && _.get(node, 'callee.object.name') === objName;
}


function isEquivalentExp(a, b) {
return _.isEqual(a, b, function(left, right, key) {
if (_.includes(['loc', 'range', 'computed'], key)) {
return true;
}
if (key === 'property') {
var leftValue = left.name || left.value;
var rightValue = right.name || right.value;
return leftValue === rightValue;
}
});
}

module.exports = {
getCaller: getCaller,
getMethodName: getMethodName,
Expand All @@ -108,5 +122,6 @@ module.exports = {
isIdentifierOfParam: isIdentifierOfParam,
isNegationExpression: isNegationExpression,
getValueReturnedInFirstLine: getValueReturnedInFirstLine,
isCallFromObject: isCallFromObject
isCallFromObject: isCallFromObject,
isEquivalentExp: isEquivalentExp
};
41 changes: 41 additions & 0 deletions tests/lib/rules/prefer-get.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
'use strict';

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

var rule = require('../../../lib/rules/prefer-get');
var RuleTester = require('eslint').RuleTester;

// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------

var ruleTester = new RuleTester();
var errors = [{message: 'Prefer _.get or _.has over an \'&&\' chain'}];
ruleTester.run('prefer-get', rule, {
valid: [{
code: 'var x = _.get(a, "b.c");'
}, {
code: 'var x = _.has(a, "b.c");'
}, {
code: 'var x = a && a.b'
}, {
code: 'a && a.b && f()'
}],
invalid: [{
code: 'x = a && a.b && a.b.c === 8',
errors: errors
}, {
code: 'x = a && a.b && a["b"].c && a.b.c.d',
errors: errors
}, {
code: 'x = a && a.b',
errors: errors,
options: [2]
}, {
code: 'x = a && a.b && a.b.c && a.b.c.d',
errors: errors,
options: [2]
}]
});

0 comments on commit ffbe336

Please sign in to comment.