Make no-danger-with-children correctly deal with props that are varia…

…bles (Fixes jsx-eslint#767)
petersendidit · Aug 15, 2016 · dca6ec6 · dca6ec6
1 parent b0747ea
commit dca6ec6
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 25 deletions.
diff --git 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
@@ -33,6 +33,24 @@ ruleTester.run('no-danger-with-children', rule, {
       code: '<div dangerouslySetInnerHTML={{ __html: "HTML" }} />',
       parserOptions: parserOptions
     },
+    {
+      code: '<div children="Children" />',
+      parserOptions: parserOptions
+    },
+    {
+      code: [
+        'const props = { dangerouslySetInnerHTML: { __html: "HTML" } };',
+        '<div {...props} />'
+      ].join('\n'),
+      parserOptions: parserOptions
+    },
+    {
+      code: [
+        'const props = { children: "Children" };',
+        '<div {...props} />'
+      ].join('\n'),
+      parserOptions: parserOptions
+    },
     {
       code: '<Hello>Children</Hello>',
       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" } };',
+        '<div {...props}>Children</div>'
+      ].join('\n'),
+      errors: [{message: 'Only set one of `children` or `props.dangerouslySetInnerHTML`'}],
+      parserOptions: parserOptions
+    },
+    {
+      code: [
+        'const props = { children: "Children", dangerouslySetInnerHTML: { __html: "HTML" } };',
+        '<div {...props} />',
+        '//foobar'
+      ].join('\n'),
+      errors: [{message: 'Only set one of `children` or `props.dangerouslySetInnerHTML`'}],
+      parserOptions: parserOptions
+    },
     {
       code: [
         '<Hello dangerouslySetInnerHTML={{ __html: "HTML" }}>',
@@ -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
     }
   ]
 });