Skip to content

Commit

Permalink
add case when identity function is explicit _.identity
Browse files Browse the repository at this point in the history
  • Loading branch information
@ganimomer
ganimomer committed Feb 21, 2016
1 parent 3445c85 commit 5cb384c
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
15 changes: 12 additions & 3 deletions lib/rules/identity-shorthand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,20 @@ module.exports = function (context) {
var settings = require('../util/settingsUtil').getSettings(context);

var supportsProp = require('../util/methodDataUtil').getPropShorthandMethods(settings.version);
function canUseIdentityShorthand(node) {
var firstParamName = astUtil.getFirstParamName(node);
return firstParamName && _.get(astUtil.getValueReturnedInFirstLine(node), 'name') === firstParamName;

function isExplicitIdentityFunction(iteratee) {
var firstParamName = astUtil.getFirstParamName(iteratee);
return firstParamName && _.get(astUtil.getValueReturnedInFirstLine(iteratee), 'name') === firstParamName;
}

var isLodashIdentityFunction = _.matches({
type: 'MemberExpression',
object: {name: settings.pragma},
property: {name: 'identity'}
});

var canUseIdentityShorthand = _.overSome(isExplicitIdentityFunction, isLodashIdentityFunction);

function methodSupportsShorthand(node) {
return _.includes(supportsProp, astUtil.getMethodName(node));
}
Expand Down
3 changes: 3 additions & 0 deletions tests/lib/rules/identity-shorthand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ ruleTester.run('identity-shorthand', rule, {
}, {
code: 'var ids = _([]).map(function (i) { return i; });',
errors: [{message: messages.always, column: 21}]
}, {
code: 'var ids = _([]).map(_.identity).value();',
errors: [{message: messages.always, column: 21}]
}, {
code: 'var ids = _.map(arr);',
options: ['never'],
Expand Down

3 comments on commit 5cb384c

@captbaritone
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this case a bit, but was thinking it might want to be a separate rule, or perhaps an option. Do prop-shorthand and matches-shorthand have the same behavior?

For example, is this an error with matches-shorthand?

_.filter(items, _.matches({a: 'foo'}));

@ganimomer
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They don't have the same behaviour right now, but maybe they should. I actually had to fix a couple of these kinds of _.identity violations once I turned this on for one of our projects.
shouldn't be to hard to implement.

@captbaritone
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think there is ever a time where someone might want to allow/disallow one and not the other? I can't think of one. Assuming you are going this direction, I'll keep parity on my fork. Thanks for talking this through with me!

Please sign in to comment.