From 17120dfba73344b310f0495c4bebbca595eacee8 Mon Sep 17 00:00:00 2001 From: David Petersen Date: Mon, 15 Aug 2016 11:11:50 -0500 Subject: [PATCH] Make no-danger-with-children deal with props that are variables (Fixes #767) --- lib/rules/no-danger-with-children.js | 78 +++++++++++++++------- tests/lib/rules/no-danger-with-children.js | 51 ++++++++++++++ 2 files changed, 104 insertions(+), 25 deletions(-) diff --git a/lib/rules/no-danger-with-children.js b/lib/rules/no-danger-with-children.js index c9deb368cd..8482b0fcb2 100644 --- a/lib/rules/no-danger-with-children.js +++ b/lib/rules/no-danger-with-children.js @@ -4,6 +4,8 @@ */ 'use strict'; +var variableUtil = require('../util/variable'); + // ------------------------------------------------------------------------------ // Rule Definition // ------------------------------------------------------------------------------ @@ -17,31 +19,53 @@ module.exports = { schema: [] // no options }, create: function(context) { + /** + * Takes a ObjectExpression and returns the value of the prop if it has it + * @param {object} node - ObjectExpression node + * @param {string} propName - name of the prop to look for + */ + function findObjectProp(node, propName) { + return node.properties.find(function(prop) { + return prop.key.name === propName; + }); + } + + /** + * Takes a JSXElement and returns the value of the prop if it has it + * @param {object} node - JSXElement node + * @param {string} propName - name of the prop to look for + */ + function findJsxProp(node, propName) { + var attributes = node.openingElement.attributes; + return attributes.find(function (attribute) { + if (attribute.type === 'JSXSpreadAttribute') { + var variable = variableUtil.variablesInScope(context).find(function (item) { + return item.name === attribute.argument.name; + }); + if (variable && variable.defs[0].node.init) { + return findObjectProp(variable.defs[0].node.init, propName); + } + } + return attribute.name && attribute.name.name === propName; + }); + } + return { JSXElement: function (node) { var hasChildren = false; - var attributes = node.openingElement.attributes; if (node.children.length) { hasChildren = true; - } else { - var childrenProp = attributes.find(function (attribute) { - return attribute.name.name === 'children'; - }); - if (childrenProp) { - hasChildren = true; - } + } else if (findJsxProp(node, 'children')) { + hasChildren = true; } - if (attributes && hasChildren) { - var jsxElement = attributes.find(function (attribute) { - return attribute.name.name === 'dangerouslySetInnerHTML'; - }); - - - if (jsxElement) { - context.report(node, 'Only set one of `children` or `props.dangerouslySetInnerHTML`'); - } + if ( + node.openingElement.attributes + && hasChildren + && findJsxProp(node, 'dangerouslySetInnerHTML') + ) { + context.report(node, 'Only set one of `children` or `props.dangerouslySetInnerHTML`'); } }, CallExpression: function (node) { @@ -53,17 +77,21 @@ module.exports = { ) { var hasChildren = false; - var props = node.arguments[1].properties; - var dangerously = props.find(function(prop) { - return prop.key.name === 'dangerouslySetInnerHTML'; - }); + var props = node.arguments[1]; + if (props.type === 'Identifier') { + var variable = variableUtil.variablesInScope(context).find(function (item) { + return item.name === props.name; + }); + if (variable && variable.defs[0].node.init) { + props = variable.defs[0].node.init; + } + } + + var dangerously = findObjectProp(props, 'dangerouslySetInnerHTML'); if (node.arguments.length === 2) { - var childrenProp = props.find(function(prop) { - return prop.key.name === 'children'; - }); - if (childrenProp) { + if (findObjectProp(props, 'children')) { hasChildren = true; } } else { diff --git a/tests/lib/rules/no-danger-with-children.js b/tests/lib/rules/no-danger-with-children.js index a44124bc50..cc045ae5cd 100644 --- a/tests/lib/rules/no-danger-with-children.js +++ b/tests/lib/rules/no-danger-with-children.js @@ -33,6 +33,24 @@ ruleTester.run('no-danger-with-children', rule, { code: '
', parserOptions: parserOptions }, + { + code: '
', + parserOptions: parserOptions + }, + { + code: [ + 'const props = { dangerouslySetInnerHTML: { __html: "HTML" } };', + '
' + ].join('\n'), + parserOptions: parserOptions + }, + { + code: [ + 'const props = { children: "Children" };', + '
' + ].join('\n'), + parserOptions: parserOptions + }, { code: 'Children', parserOptions: parserOptions @@ -73,6 +91,23 @@ ruleTester.run('no-danger-with-children', rule, { errors: [{message: 'Only set one of `children` or `props.dangerouslySetInnerHTML`'}], parserOptions: parserOptions }, + { + code: [ + 'const props = { dangerouslySetInnerHTML: { __html: "HTML" } };', + '
Children
' + ].join('\n'), + errors: [{message: 'Only set one of `children` or `props.dangerouslySetInnerHTML`'}], + parserOptions: parserOptions + }, + { + code: [ + 'const props = { children: "Children", dangerouslySetInnerHTML: { __html: "HTML" } };', + '
', + '//foobar' + ].join('\n'), + errors: [{message: 'Only set one of `children` or `props.dangerouslySetInnerHTML`'}], + parserOptions: parserOptions + }, { code: [ '', @@ -134,6 +169,22 @@ ruleTester.run('no-danger-with-children', rule, { ].join('\n'), errors: [{message: 'Only set one of `children` or `props.dangerouslySetInnerHTML`'}], parserOptions: parserOptions + }, + { + code: [ + 'const props = { dangerouslySetInnerHTML: { __html: "HTML" } };', + 'React.createElement("div", props, "Children");' + ].join('\n'), + errors: [{message: 'Only set one of `children` or `props.dangerouslySetInnerHTML`'}], + parserOptions: parserOptions + }, + { + code: [ + 'const props = { children: "Children", dangerouslySetInnerHTML: { __html: "HTML" } };', + 'React.createElement("div", props);' + ].join('\n'), + errors: [{message: 'Only set one of `children` or `props.dangerouslySetInnerHTML`'}], + parserOptions: parserOptions } ] });