diff --git a/docs/rules/prefer-arrow-callback.md b/docs/rules/prefer-arrow-callback.md index 4942295a5c1..600acb4539f 100644 --- a/docs/rules/prefer-arrow-callback.md +++ b/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. diff --git a/lib/rules/prefer-arrow-callback.js b/lib/rules/prefer-arrow-callback.js index e4da8e1e97e..dd2505cb533 100644 --- a/lib/rules/prefer-arrow-callback.js +++ b/lib/rules/prefer-arrow-callback.js @@ -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 //------------------------------------------------------------------------------ @@ -140,7 +151,9 @@ module.exports = { }, additionalProperties: false } - ] + ], + + fixable: "code" }, create(context) { @@ -148,6 +161,7 @@ module.exports = { const allowUnboundThis = options.allowUnboundThis !== false; // default to true const allowNamedFunctions = options.allowNamedFunctions; + const sourceCode = context.getSourceCode(); /* * {Array<{this: boolean, super: boolean, meta: boolean}>} @@ -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 + " => "); + } + }); } } }; diff --git a/tests/lib/rules/prefer-arrow-callback.js b/tests/lib/rules/prefer-arrow-callback.js index 7b65d956227..5b870531ddb 100644 --- a/tests/lib/rules/prefer-arrow-callback.js +++ b/tests/lib/rules/prefer-arrow-callback.js @@ -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; })" + } ] });