Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Update: add fixer for prefer-arrow-callback (fixes #7002) (#7004)
  • Loading branch information
not-an-aardvark authored and mysticatea committed Sep 8, 2016
1 parent 7502eed commit 883316d
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 17 deletions.
2 changes: 2 additions & 0 deletions docs/rules/prefer-arrow-callback.md
@@ -1,5 +1,7 @@
# Suggest using arrow functions as callbacks. (prefer-arrow-callback)

(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixes problems reported by this rule.

Arrow functions are suited to callbacks, because:

- `this` keywords in arrow functions bind to the upper scope's.
Expand Down
44 changes: 42 additions & 2 deletions lib/rules/prefer-arrow-callback.js
Expand Up @@ -115,6 +115,17 @@ function getCallbackInfo(node) {
throw new Error("unreachable");
}

/**
* Checks whether a simple list of parameters contains any duplicates. This does not handle complex
parameter lists (e.g. with destructuring), since complex parameter lists are a SyntaxError with duplicate
parameter names anyway. Instead, it always returns `false` for complex parameter lists.
* @param {ASTNode[]} paramsList The list of parameters for a function
* @returns {boolean} `true` if the list of parameters contains any duplicates
*/
function hasDuplicateParams(paramsList) {
return paramsList.every(param => param.type === "Identifier") && paramsList.length !== new Set(paramsList.map(param => param.name)).size;
}

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -140,14 +151,17 @@ module.exports = {
},
additionalProperties: false
}
]
],

fixable: "code"
},

create(context) {
const options = context.options[0] || {};

const allowUnboundThis = options.allowUnboundThis !== false; // default to true
const allowNamedFunctions = options.allowNamedFunctions;
const sourceCode = context.getSourceCode();

/*
* {Array<{this: boolean, super: boolean, meta: boolean}>}
Expand Down Expand Up @@ -246,7 +260,33 @@ module.exports = {
!scopeInfo.super &&
!scopeInfo.meta
) {
context.report(node, "Unexpected function expression.");
context.report({
node,
message: "Unexpected function expression.",
fix(fixer) {
if ((!callbackInfo.isLexicalThis && scopeInfo.this) || hasDuplicateParams(node.params)) {

// If the callback function does not have .bind(this) and contains a reference to `this`, there
// is no way to determine what `this` should be, so don't perform any fixes.
// If the callback function has duplicates in its list of parameters (possible in sloppy mode),
// don't replace it with an arrow function, because this is a SyntaxError with arrow functions.
return null;
}

const paramsLeftParen = node.params.length ? sourceCode.getTokenBefore(node.params[0]) : sourceCode.getTokenBefore(node.body, 1);
const paramsRightParen = sourceCode.getTokenBefore(node.body);
const paramsFullText = sourceCode.text.slice(paramsLeftParen.range[0], paramsRightParen.range[1]);

if (callbackInfo.isLexicalThis) {

// If the callback function has `.bind(this)`, replace it with an arrow function and remove the binding.
return fixer.replaceText(node.parent.parent, paramsFullText + " => " + sourceCode.getText(node.body));
}

// Otherwise, only replace the `function` keyword and parameters with the arrow function parameters.
return fixer.replaceTextRange([node.start, node.body.start], paramsFullText + " => ");
}
});
}
}
};
Expand Down
147 changes: 132 additions & 15 deletions tests/lib/rules/prefer-arrow-callback.js
Expand Up @@ -41,22 +41,139 @@ ruleTester.run("prefer-arrow-callback", rule, {
{code: "foo(function bar() { super.a; });", parserOptions: { ecmaVersion: 6 }},
{code: "foo(function bar() { super.a; }.bind(this));", parserOptions: { ecmaVersion: 6 }},
{code: "foo(function bar() { new.target; });", parserOptions: { ecmaVersion: 6 }},
{code: "foo(function bar() { new.target; }.bind(this));", parserOptions: { ecmaVersion: 6 }}
{code: "foo(function bar() { new.target; }.bind(this));", parserOptions: { ecmaVersion: 6 }},
{code: "foo(function bar() { this; }.bind(this, somethingElse));"}
],
invalid: [
{code: "foo(function bar() {});", errors},
{code: "foo(function() {});", options: [{ allowNamedFunctions: true }], errors},
{code: "foo(function bar() {});", options: [{ allowNamedFunctions: false }], errors},
{code: "foo(function() {});", errors},
{code: "foo(nativeCb || function() {});", errors},
{code: "foo(bar ? function() {} : function() {});", errors: [errors[0], errors[0]]},
{code: "foo(function() { (function() { this; }); });", errors},
{code: "foo(function() { this; }.bind(this));", errors},
{code: "foo(function() { (() => this); }.bind(this));", parserOptions: { ecmaVersion: 6 }, errors},
{code: "foo(function bar(a) { a; });", errors},
{code: "foo(function(a) { a; });", errors},
{code: "foo(function(arguments) { arguments; });", errors},
{code: "foo(function() { this; });", options: [{ allowUnboundThis: false }], errors},
{code: "foo(function() { (() => this); });", parserOptions: { ecmaVersion: 6 }, options: [{ allowUnboundThis: false }], errors}
{
code: "foo(function bar() {});",
parserOptions: { ecmaVersion: 6 },
errors,
output: "foo(() => {});"
},
{
code: "foo(function() {});",
parserOptions: { ecmaVersion: 6 },
options: [{ allowNamedFunctions: true }],
errors,
output: "foo(() => {});"
},
{
code: "foo(function bar() {});",
parserOptions: { ecmaVersion: 6 },
options: [{ allowNamedFunctions: false }],
errors,
output: "foo(() => {});"
},
{
code: "foo(function() {});",
parserOptions: { ecmaVersion: 6 },
errors,
output: "foo(() => {});"
},
{
code: "foo(nativeCb || function() {});",
parserOptions: { ecmaVersion: 6 },
errors,
output: "foo(nativeCb || () => {});"
},
{
code: "foo(bar ? function() {} : function() {});",
parserOptions: { ecmaVersion: 6 },
errors: [errors[0], errors[0]],
output: "foo(bar ? () => {} : () => {});"
},
{
code: "foo(function() { (function() { this; }); });",
parserOptions: { ecmaVersion: 6 },
errors,
output: "foo(() => { (function() { this; }); });"
},
{
code: "foo(function() { this; }.bind(this));",
parserOptions: { ecmaVersion: 6 },
errors,
output: "foo(() => { this; });"
},
{
code: "foo(function() { (() => this); }.bind(this));",
parserOptions: { ecmaVersion: 6 },
errors,
output: "foo(() => { (() => this); });"
},
{
code: "foo(function bar(a) { a; });",
parserOptions: { ecmaVersion: 6 },
errors,
output: "foo((a) => { a; });"
},
{
code: "foo(function(a) { a; });",
parserOptions: { ecmaVersion: 6 },
errors,
output: "foo((a) => { a; });"
},
{
code: "foo(function(arguments) { arguments; });",
parserOptions: { ecmaVersion: 6 },
errors,
output: "foo((arguments) => { arguments; });"
},
{
code: "foo(function() { this; });",
parserOptions: { ecmaVersion: 6 },
options: [{ allowUnboundThis: false }],
errors,
output: "foo(function() { this; });" // No fix applied
},
{
code: "foo(function() { (() => this); });",
parserOptions: { ecmaVersion: 6 },
options: [{ allowUnboundThis: false }],
errors,
output: "foo(function() { (() => this); });" // No fix applied
},
{
code: "qux(function(foo, bar, baz) { return foo * 2; })",
parserOptions: { ecmaVersion: 6 },
errors,
output: "qux((foo, bar, baz) => { return foo * 2; })"
},
{
code: "qux(function(foo, bar, baz) { return foo * bar; }.bind(this))",
parserOptions: { ecmaVersion: 6 },
errors,
output: "qux((foo, bar, baz) => { return foo * bar; })"
},
{
code: "qux(function(foo, bar, baz) { return foo * this.qux; }.bind(this))",
parserOptions: { ecmaVersion: 6 },
errors,
output: "qux((foo, bar, baz) => { return foo * this.qux; })"
},
{
code: "qux(function(foo = 1, [bar = 2] = [], {qux: baz = 3} = {foo: 'bar'}) { return foo + bar; });",
parserOptions: { ecmaVersion: 6 },
errors,
output: "qux((foo = 1, [bar = 2] = [], {qux: baz = 3} = {foo: 'bar'}) => { return foo + bar; });"
},
{
code: "qux(function(baz, baz) { })",
parserOptions: { ecmaVersion: 6 },
errors,
output: "qux(function(baz, baz) { })" // Duplicate parameter names are a SyntaxError in arrow functions
},
{
code: "qux(function( /* no params */ ) { })",
parserOptions: { ecmaVersion: 6 },
errors,
output: "qux(( /* no params */ ) => { })"
},
{
code: "qux(function( /* a */ foo /* b */ , /* c */ bar /* d */ , /* e */ baz /* f */ ) { return foo; })",
parserOptions: { ecmaVersion: 6 },
errors,
output: "qux(( /* a */ foo /* b */ , /* c */ bar /* d */ , /* e */ baz /* f */ ) => { return foo; })"
}
]
});

0 comments on commit 883316d

Please sign in to comment.