diff --git a/docs/rules/jsx-no-bind.md b/docs/rules/jsx-no-bind.md index fb3adeb9e5..a4410f3cb3 100644 --- a/docs/rules/jsx-no-bind.md +++ b/docs/rules/jsx-no-bind.md @@ -24,6 +24,7 @@ The following patterns are not considered warnings: "react/jsx-no-bind": [, { "ignoreRefs": || false, "allowArrowFunctions": || false, + "allowFunctions": || false, "allowBind": || false }] ``` @@ -45,6 +46,14 @@ When `true` the following is not considered a warning:
alert("1337")} /> ``` +### `allowFunctions` + +When `true` the following is not considered a warning: + +```jsx +
+``` + ### `allowBind` When `true` the following is not considered a warning: diff --git a/lib/rules/jsx-no-bind.js b/lib/rules/jsx-no-bind.js index e413e92e48..aef6a6aab8 100644 --- a/lib/rules/jsx-no-bind.js +++ b/lib/rules/jsx-no-bind.js @@ -1,7 +1,8 @@ /** * @fileoverview Prevents usage of Function.prototype.bind and arrow functions - * in React component definition. + * in React component props. * @author Daniel Lo Nigro + * @author Jacky Ho */ 'use strict'; @@ -12,10 +13,17 @@ const propName = require('jsx-ast-utils/propName'); // Rule Definition // ----------------------------------------------------------------------------- +const violationMessageStore = { + bindCall: 'JSX props should not use .bind()', + arrowFunc: 'JSX props should not use arrow functions', + bindExpression: 'JSX props should not use ::', + func: 'JSX props should not use functions' +}; + module.exports = { meta: { docs: { - description: 'Prevents usage of Function.prototype.bind and arrow functions in React component definition', + description: 'Prevents usage of Function.prototype.bind and arrow functions in React component props', category: 'Best Practices', recommended: false }, @@ -31,6 +39,10 @@ module.exports = { default: false, type: 'boolean' }, + allowFunctions: { + default: false, + type: 'boolean' + }, ignoreRefs: { default: false, type: 'boolean' @@ -40,33 +52,100 @@ module.exports = { }] }, - create: Components.detect((context, components, utils) => { + create: Components.detect(context => { const configuration = context.options[0] || {}; + // Keep track of all the variable names pointing to a bind call, + // bind expression or an arrow function in different block statements + const blockVariableNameSets = {}; + + function setBlockVariableNameSet(blockStart) { + blockVariableNameSets[blockStart] = { + arrowFunc: new Set(), + bindCall: new Set(), + bindExpression: new Set(), + func: new Set() + }; + } + + function getNodeViolationType(node) { + const nodeType = node.type; + + if ( + !configuration.allowBind && + nodeType === 'CallExpression' && + node.callee.type === 'MemberExpression' && + node.callee.property.type === 'Identifier' && + node.callee.property.name === 'bind' + ) { + return 'bindCall'; + } else if ( + !configuration.allowArrowFunctions && + nodeType === 'ArrowFunctionExpression' + ) { + return 'arrowFunc'; + } else if ( + !configuration.allowFunctions && + nodeType === 'FunctionExpression' + ) { + return 'func'; + } else if ( + !configuration.allowBind && + nodeType === 'BindExpression' + ) { + return 'bindExpression'; + } + + return null; + } + + function addVariableNameToSet(violationType, variableName, blockStart) { + blockVariableNameSets[blockStart][violationType].add(variableName); + } + + function getBlockStatementAncestors(node) { + return context.getAncestors(node).reverse().filter( + ancestor => ancestor.type === 'BlockStatement' + ); + } + + function reportVariableViolation(node, name, blockStart) { + const blockSets = blockVariableNameSets[blockStart]; + const violationTypes = Object.keys(blockSets); + + return violationTypes.find(type => { + if (blockSets[type].has(name)) { + context.report({node: node, message: violationMessageStore[type]}); + return true; + } + + return false; + }); + } + + function findVariableViolation(node, name) { + getBlockStatementAncestors(node).find( + block => reportVariableViolation(node, name, block.start) + ); + } + return { - CallExpression: function(node) { - const callee = node.callee; + BlockStatement(node) { + setBlockVariableNameSet(node.start); + }, + + VariableDeclarator(node) { + const blockAncestors = getBlockStatementAncestors(node); + const variableViolationType = getNodeViolationType(node.init); + if ( - !configuration.allowBind && - (callee.type !== 'MemberExpression' || callee.property.name !== 'bind') + blockAncestors.length > 0 && + variableViolationType && + node.parent.kind === 'const' // only support const right now ) { - return; - } - const ancestors = context.getAncestors(callee).reverse(); - for (let i = 0, j = ancestors.length; i < j; i++) { - if ( - !configuration.allowBind && - (ancestors[i].type === 'MethodDefinition' && ancestors[i].key.name === 'render') || - (ancestors[i].type === 'Property' && ancestors[i].key.name === 'render') - ) { - if (utils.isReturningJSX(ancestors[i])) { - context.report({ - node: callee, - message: 'JSX props should not use .bind()' - }); - } - break; - } + addVariableNameToSet( + variableViolationType, node.id.name, blockAncestors[0].start + ); } }, @@ -76,31 +155,14 @@ module.exports = { return; } const valueNode = node.value.expression; - if ( - !configuration.allowBind && - valueNode.type === 'CallExpression' && - valueNode.callee.type === 'MemberExpression' && - valueNode.callee.property.name === 'bind' - ) { - context.report({ - node: node, - message: 'JSX props should not use .bind()' - }); - } else if ( - !configuration.allowArrowFunctions && - valueNode.type === 'ArrowFunctionExpression' - ) { - context.report({ - node: node, - message: 'JSX props should not use arrow functions' - }); - } else if ( - !configuration.allowBind && - valueNode.type === 'BindExpression' - ) { + const valueNodeType = valueNode.type; + const nodeViolationType = getNodeViolationType(valueNode); + + if (valueNodeType === 'Identifier') { + findVariableViolation(node, valueNode.name); + } else if (nodeViolationType) { context.report({ - node: node, - message: 'JSX props should not use ::' + node: node, message: violationMessageStore[nodeViolationType] }); } } diff --git a/tests/lib/rules/jsx-no-bind.js b/tests/lib/rules/jsx-no-bind.js index 7941df9b3b..ec467f98f7 100644 --- a/tests/lib/rules/jsx-no-bind.js +++ b/tests/lib/rules/jsx-no-bind.js @@ -31,42 +31,57 @@ ruleTester.run('jsx-no-bind', rule, { valid: [ // Not covered by the rule { - code: '
', - parser: 'babel-eslint' + code: '
' }, { - code: '
', - parser: 'babel-eslint' + code: '
' }, { - code: '
', - parser: 'babel-eslint' + code: '
' }, // bind() and arrow functions in refs explicitly ignored { code: '
this._input = c}>
', - options: [{ignoreRefs: true}], - parser: 'babel-eslint' + options: [{ignoreRefs: true}] }, { code: '
', - options: [{ignoreRefs: true}], - parser: 'babel-eslint' + options: [{ignoreRefs: true}] + }, + { + code: '
', + options: [{ignoreRefs: true}] }, // bind() explicitly allowed { code: '
', - options: [{allowBind: true}], - parser: 'babel-eslint' + options: [{allowBind: true}] }, // Arrow functions explicitly allowed { code: '
alert("1337")}>
', - options: [{allowArrowFunctions: true}], - parser: 'babel-eslint' + options: [{allowArrowFunctions: true}] + }, + { + code: '
alert("1337")}>
', + options: [{allowArrowFunctions: true}] + }, + + // Functions explicitly allowed + { + code: '
', + options: [{allowFunctions: true}] + }, + { + code: '
', + options: [{allowFunctions: true}] + }, + { + code: '
', + options: [{allowFunctions: true}] }, // Redux connect @@ -79,9 +94,9 @@ ruleTester.run('jsx-no-bind', rule, { } export default connect()(Hello); `, - options: [{allowBind: true}], - parser: 'babel-eslint' + options: [{allowBind: true}] }, + // Backbone view with a bind { code: ` @@ -91,8 +106,7 @@ ruleTester.run('jsx-no-bind', rule, { this.onTap.bind(this); } }); - `, - parser: 'babel-eslint' + ` }, { code: ` @@ -102,8 +116,7 @@ ruleTester.run('jsx-no-bind', rule, { return true; } }; - `, - parser: 'babel-eslint' + ` }, { code: ` @@ -113,7 +126,137 @@ ruleTester.run('jsx-no-bind', rule, { return true; } }; - `, + ` + }, + + { + code: [ + 'class Hello extends Component {', + ' render() {', + ' const click = this.onTap.bind(this);', + ' return
Hello
;', + ' }', + '};' + ].join('\n') + }, + { + code: [ + 'class Hello extends Component {', + ' render() {', + ' foo.onClick = this.onTap.bind(this);', + ' return
Hello
;', + ' }', + '};' + ].join('\n') + }, + { + code: [ + 'class Hello extends Component {', + ' render() {', + ' return (
{', + ' this.props.list.map(this.wrap.bind(this, "span"))', + ' }
);', + ' }', + '};' + ].join('\n') + }, + { + code: [ + 'class Hello extends Component {', + ' render() {', + ' const click = () => true;', + ' return
Hello
;', + ' }', + '};' + ].join('\n') + }, + { + code: [ + 'class Hello extends Component {', + ' render() {', + ' return (
{', + ' this.props.list.map(item => )', + ' }
);', + ' }', + '};' + ].join('\n') + }, + { + code: [ + 'class Hello extends Component {', + ' render() {', + ' const click = this.bar::baz', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + parser: 'babel-eslint' + }, + { + code: [ + 'class Hello extends Component {', + ' render() {', + ' return (
{', + ' this.props.list.map(this.bar::baz)', + ' }
);', + ' }', + '};' + ].join('\n'), + parser: 'babel-eslint' + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' return (
{', + ' this.props.list.map(this.wrap.bind(this, "span"))', + ' }
);', + ' }', + '});' + ].join('\n') + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' const click = this.bar::baz', + ' return
Hello
;', + ' }', + '});' + ].join('\n'), + parser: 'babel-eslint' + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' const click = () => true', + ' return
Hello
;', + ' }', + '});' + ].join('\n') + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = () => {', + ' const onClick = this.doSomething.bind(this, "no")', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + parser: 'babel-eslint' + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = async () => {', + ' return (
{', + ' this.props.list.map(this.wrap.bind(this, "span"))', + ' }
);', + ' }', + '};' + ].join('\n'), parser: 'babel-eslint' } ], @@ -122,23 +265,19 @@ ruleTester.run('jsx-no-bind', rule, { // .bind() { code: '
', - errors: [{message: 'JSX props should not use .bind()'}], - parser: 'babel-eslint' + errors: [{message: 'JSX props should not use .bind()'}] }, { code: '
', - errors: [{message: 'JSX props should not use .bind()'}], - parser: 'babel-eslint' + errors: [{message: 'JSX props should not use .bind()'}] }, { code: '
', - errors: [{message: 'JSX props should not use .bind()'}], - parser: 'babel-eslint' + errors: [{message: 'JSX props should not use .bind()'}] }, { code: '
', - errors: [{message: 'JSX props should not use .bind()'}], - parser: 'babel-eslint' + errors: [{message: 'JSX props should not use .bind()'}] }, { code: ` @@ -149,8 +288,7 @@ ruleTester.run('jsx-no-bind', rule, { } }); `, - errors: [{message: 'JSX props should not use .bind()'}], - parser: 'babel-eslint' + errors: [{message: 'JSX props should not use .bind()'}] }, { code: ` @@ -161,18 +299,40 @@ ruleTester.run('jsx-no-bind', rule, { } }; `, + errors: [{message: 'JSX props should not use .bind()'}] + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv() {', + ' const click = this.doSomething.bind(this, "no")', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [{message: 'JSX props should not use .bind()'}] + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = () => {', + ' const click = this.doSomething.bind(this, "no")', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), errors: [{message: 'JSX props should not use .bind()'}], parser: 'babel-eslint' }, { - code: ` - const foo = { - render: function() { - const click = this.onTap.bind(this); - return
Hello
; - } - }; - `, + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = async () => {', + ' const click = this.doSomething.bind(this, "no")', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), errors: [{message: 'JSX props should not use .bind()'}], parser: 'babel-eslint' }, @@ -184,43 +344,315 @@ ruleTester.run('jsx-no-bind', rule, { ) }; `, - errors: [{message: 'JSX props should not use .bind()'}], - parser: 'babel-eslint' + errors: [{message: 'JSX props should not use .bind()'}] }, { - code: ` - const foo = { - render() { - const click = this.onTap.bind(this); - return
Hello
; - } - }; - `, - errors: [{message: 'JSX props should not use .bind()'}], + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' return
', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use .bind()'}] + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' const doThing = this.doSomething.bind(this, "hey")', + ' return
', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use .bind()'}] + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = () => {', + ' const click = () => true', + ' const renderStuff = () => {', + ' const click = this.doSomething.bind(this, "hey")', + ' return
', + ' }', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [ + {message: 'JSX props should not use .bind()'}, + {message: 'JSX props should not use arrow functions'} + ], parser: 'babel-eslint' }, // Arrow functions { code: '
alert("1337")}>
', - errors: [{message: 'JSX props should not use arrow functions'}], - parser: 'babel-eslint' + errors: [{message: 'JSX props should not use arrow functions'}] + }, + { + code: '
alert("1337")}>
', + errors: [{message: 'JSX props should not use arrow functions'}] }, { code: '
42}>
', + errors: [{message: 'JSX props should not use arrow functions'}] + }, + { + code: '
{ first(); second(); }}>
', + errors: [{message: 'JSX props should not use arrow functions'}] + }, + { + code: '
this._input = c}>
', + errors: [{message: 'JSX props should not use arrow functions'}] + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = () => {', + ' const click = () => true', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), errors: [{message: 'JSX props should not use arrow functions'}], parser: 'babel-eslint' }, { - code: '
{ first(); second(); }}>
', + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = async () => {', + ' const click = () => true', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), errors: [{message: 'JSX props should not use arrow functions'}], parser: 'babel-eslint' }, { - code: '
this._input = c}>
', + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = async () => {', + ' const click = async () => true', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), errors: [{message: 'JSX props should not use arrow functions'}], parser: 'babel-eslint' }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' return
true} />', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use arrow functions'}] + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' return
true} />', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use arrow functions'}] + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' const doThing = () => true', + ' return
', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use arrow functions'}] + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' const doThing = async () => true', + ' return
', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use arrow functions'}] + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = () => {', + ' const click = ::this.onChange', + ' const renderStuff = () => {', + ' const click = () => true', + ' return
', + ' }', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [ + {message: 'JSX props should not use arrow functions'}, + {message: 'JSX props should not use ::'} + ], + parser: 'babel-eslint' + }, + + // Functions + { + code: '
', + errors: [{message: 'JSX props should not use functions'}] + }, + { + code: '
', + errors: [{message: 'JSX props should not use functions'}] + }, + { + code: '
', + errors: [{message: 'JSX props should not use functions'}] + }, + { + code: '
', + errors: [{message: 'JSX props should not use functions'}] + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = () => {', + ' const click = function () { return true }', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [{message: 'JSX props should not use functions'}], + parser: 'babel-eslint' + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = () => {', + ' const click = function * () { return true }', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [{message: 'JSX props should not use functions'}], + parser: 'babel-eslint' + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = async () => {', + ' const click = function () { return true }', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [{message: 'JSX props should not use functions'}], + parser: 'babel-eslint' + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = async () => {', + ' const click = async function () { return true }', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [{message: 'JSX props should not use functions'}], + parser: 'babel-eslint' + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' return
', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use functions'}] + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' return
', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use functions'}] + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' return
', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use functions'}] + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' const doThing = function () { return true }', + ' return
', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use functions'}] + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' const doThing = async function () { return true }', + ' return
', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use functions'}] + }, + { + code: [ + 'var Hello = React.createClass({', + ' render: function() { ', + ' const doThing = function * () { return true }', + ' return
', + ' }', + '});' + ].join('\n'), + errors: [{message: 'JSX props should not use functions'}] + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = () => {', + ' const click = ::this.onChange', + ' const renderStuff = () => {', + ' const click = function () { return true }', + ' return
', + ' }', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [ + {message: 'JSX props should not use functions'}, + {message: 'JSX props should not use ::'} + ], + parser: 'babel-eslint' + }, // Bind expression { @@ -237,6 +669,58 @@ ruleTester.run('jsx-no-bind', rule, { code: '
', errors: [{message: 'JSX props should not use ::'}], parser: 'babel-eslint' + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv() {', + ' const click = ::this.onChange', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [{message: 'JSX props should not use ::'}], + parser: 'babel-eslint' + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv() {', + ' const click = this.bar::baz', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [{message: 'JSX props should not use ::'}], + parser: 'babel-eslint' + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = async () => {', + ' const click = this.bar::baz', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [{message: 'JSX props should not use ::'}], + parser: 'babel-eslint' + }, + { + code: [ + 'class Hello23 extends React.Component {', + ' renderDiv = () => {', + ' const click = true', + ' const renderStuff = () => {', + ' const click = this.bar::baz', + ' return
', + ' }', + ' return
Hello
;', + ' }', + '};' + ].join('\n'), + errors: [{message: 'JSX props should not use ::'}], + parser: 'babel-eslint' } ] });