Skip to content

Commit

Permalink
rename rule no-unnecessary-bind to callback-binding with explanation …
Browse files Browse the repository at this point in the history
…about shared settings
  • Loading branch information
ganimomer committed Jan 24, 2016
1 parent 33f37ab commit a107ad5
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 62 deletions.
22 changes: 19 additions & 3 deletions README.md
Expand Up @@ -18,10 +18,26 @@ If you installed `ESLint` globally, you have to install lodash3 plugin globally
# Configuration

Add `plugins` section and specify ESLint-plugin-lodash3 as a plugin.
You can additionally add settings for the plugin.

### Shared Rule Settings

#### Pragma
Specifies the name you use for the Lodash variable in your code. Default is `_`.

#### Version
Specifies the major Lodash Version you are using (default is `4`).
If you wish to use this plugin with Lodash v3, change this value to 3.

```json
{
"plugins": ["lodash3"]
"plugins": ["lodash3"],
"settings": {
"lodash": {
"version": 4,
"pragma": "_"
}
}
}
```

Expand All @@ -39,7 +55,7 @@ Finally, enable all of the rules that you would like to use.
"lodash3/no-single-chain": 2,
"lodash3/prefer-reject": [2, 3],
"lodash3/prefer-filter": [2, 3],
"lodash3/no-unnecessary-bind": 2,
"lodash3/callback-binding": 2,
"lodash3/unwrap": 2,
"lodash3/prefer-compact": 2,
"lodash3/no-double-unwrap": 2,
Expand Down Expand Up @@ -74,7 +90,7 @@ Finally, enable all of the rules that you would like to use.
* [no-single-chain](docs/rules/no-single-chain.md): Prevent chaining syntax for single method, e.g. `_(x).map().value()`
* [prefer-reject](docs/rules/prefer-reject.md): Prefer `_.reject` over filter with `!(expression)` or `x.prop1 !== value`
* [prefer-filter](docs/rules/prefer-filter.md): Prefer `_.filter` over `_.forEach` with an `if` statement inside.
* [no-unnecessary-bind](docs/rules/no-unnecessary-bind.md): Prefer passing `thisArg` over binding.
* [callback-binding](docs/rules/callback-binding.md): Use or avoid `thisArg` for Lodash method callbacks, depending on major version.
* [unwrap](docs/rules/unwrap.md): Prevent chaining without evaluation via `value()` or non-chainable methods like `max()`.,
* [prefer-compact](docs/rules/prefer-compact.md): Prefer `_.compact` over `_.filter` for only truthy values.
* [no-double-unwrap](docs/rules/no-double-unwrap.md): Do not use `.value()` on chains that have already ended (e.g. with `max()` or `reduce()`)
Expand Down
44 changes: 44 additions & 0 deletions docs/rules/callback-binding.md
@@ -0,0 +1,44 @@
# Callback binding

In Lodash version 3, it was possible to pass an additional parameter to methods that require binding, `thisArg`.
However, in Lodash 4, this option was removed in favor of regular binding, and still using the old method could cause unexpected results.

## Rule Details

This rule takes no arguments. However, it is affected by the lodash `version` defined in the config's [shared settings for Lodash](/README.md#shared-rule-settings).

### Lodash Version 4 (default)
In version 4, the following patterns are considered warnings:

```js
var r = _.filter(users, function (user) {
return user.age > this.age;
}, this);

var r = _.reduce(numbers, multiply, 1, this);
```

The following patterns are not considered warnings:

```js
var r = _.filter(users, function (user) {
return user.age > this.age;
}.bind(this));
```

### Lodash version 3

In version 3, the following patterns are considered warnings:
```js
var r = _.filter(users, function (user) {
return user.age > this.age;
}, this);
```

The following patterns are not considered warnings:

```js
var r = _.filter(users, function (user) {
return user.age > this.age;
}.bind(this));
```
28 changes: 0 additions & 28 deletions docs/rules/no-unnecessary-bind.md

This file was deleted.

41 changes: 41 additions & 0 deletions lib/rules/callback-binding.js
@@ -0,0 +1,41 @@
/**
* @fileoverview Rule to disallow the use of a chain for a single method
*/
'use strict';

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

module.exports = function (context) {
var lodashUtil = require('../util/lodashUtil');
var astUtil = require('../util/astUtil');
var settingsUtil = require('../util/settingsUtil');

var lodashPragma = settingsUtil.getLodashPragma(context);
var majorVersion = settingsUtil.getMajorVersion(context);
var transformerMethods = ['reduce', 'reduceRight', 'transform'];

function isBound(node) {
return node && node.type === 'CallExpression' && astUtil.getMethodName(node) === 'bind' && node.arguments.length === 1;
}

var callExpressionReporters = {
3: function (node, iteratee) {
if (isBound(iteratee)) {
context.report(iteratee.callee.property, 'Unnecessary bind, pass `thisArg` to lodash method instead');
}
},
4: function (node, iteratee) {
var isTransformerMethod = transformerMethods.some(lodashUtil.isCallToMethod.bind(null, node));
var iterateeIndex = node.arguments.indexOf(iteratee);
if ((isTransformerMethod && node.arguments[iterateeIndex + 2]) || (!isTransformerMethod && node.arguments[iterateeIndex + 1])) {
context.report(iteratee, 'Do not use Lodash 3 thisArg, use binding instead');
}
}
};

return {
CallExpression: lodashUtil.getLodashMethodVisitor(lodashPragma, callExpressionReporters[majorVersion])
};
};
26 changes: 0 additions & 26 deletions lib/rules/no-unnecessary-bind.js

This file was deleted.

Expand Up @@ -4,23 +4,29 @@
// Requirements
// ------------------------------------------------------------------------------

var rule = require('../../../lib/rules/no-unnecessary-bind');
var rule = require('../../../lib/rules/callback-binding');
var RuleTester = require('eslint').RuleTester;

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

var ruleTester = new RuleTester();
var toErrorObject = require('../testUtil/toErrorObject').fromMessage('Unnecessary bind, pass `thisArg` to lodash method instead');
ruleTester.run('no-unnecessary-bind', rule, {
var optionsUtil = require('../testUtil/optionsUtil');
ruleTester.run('callback-binding', rule, {
valid: [
'var x = _.map(arr, f)',
'var r = _.find(themeStyleList, function (themeStyle) { return this.x; }, this);',
'var r = _.find(arr, function (i) { return this.x; }.bind(this, x));'
],
].map(optionsUtil.fromVersion3).concat([
'var x = _.map(arr, f.bind(this))',
'var x = _.find(users, function(user) { return user.age > this.age}.bind(this));'
]),
invalid: [
'var r = _.find(users, function (user) { return user.age > this.age; }.bind(this));',
'var r = _.find(users, predicate.bind(this));'
].map(toErrorObject)
].map(optionsUtil.fromVersion3).map(optionsUtil.fromMessage('Unnecessary bind, pass `thisArg` to lodash method instead')).concat([
'var t = _.find(users, function(user) { return this.x === user}, this);',
'var t = _.reduce(users, func, initial, this);'
].map(optionsUtil.fromMessage('Do not use Lodash 3 thisArg, use binding instead')))
});

0 comments on commit a107ad5

Please sign in to comment.