Merge pull request #124 from Cellule/prop_types_destructuring

Fix `prop-types` desctructuring with properties as string (fixes #118)
jsx-eslint · Jun 19, 2015 · 9583f81 · 9583f81
2 parents 1ab838c + c73d9b8
commit 9583f81
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 8 deletions.
diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js
@@ -164,6 +164,16 @@ module.exports = function(context) {
     return tokens.length && tokens[0].value === '...';
   }
 
+  function getKeyValue(node) {
+    var key = node.key;
+    if (key) {
+      if (key.type === 'Identifier') {
+        return key.name;
+      }
+      return key.value;
+    }
+  }
+
   /**
    * Iterates through a properties node, like a customized forEach.
    * @param {Object[]} properties Array of properties to iterate.
@@ -174,11 +184,10 @@ module.exports = function(context) {
     if (properties.length && typeof fn === 'function') {
       for (var i = 0, j = properties.length; i < j; i++) {
         var node = properties[i];
-        var key = node.key;
-        var keyName = key.type === 'Identifier' ? key.name : key.value;
+        var key = getKeyValue(node);
 
         var value = node.value;
-        fn(keyName, value);
+        fn(key, value);
       }
     }
   }
@@ -331,7 +340,7 @@ module.exports = function(context) {
     } else if (
       node.parent.parent.declarations &&
       node.parent.parent.declarations[0].id.properties &&
-      node.parent.parent.declarations[0].id.properties[0].key.name
+      getKeyValue(node.parent.parent.declarations[0].id.properties[0])
     ) {
       type = 'destructuring';
     }
@@ -355,10 +364,13 @@ module.exports = function(context) {
           if (hasSpreadOperator(properties[i])) {
             continue;
           }
-          usedPropTypes.push({
-            name: properties[i].key.name,
-            node: properties[i]
-          });
+          var propName = getKeyValue(properties[i]);
+          if (propName) {
+            usedPropTypes.push({
+              name: propName,
+              node: properties[i]
+            });
+          }
         }
         break;
       default:

diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js
@@ -415,6 +415,23 @@ eslintTester.addRuleTest('lib/rules/prop-types', {
         classes: true,
         jsx: true
       }
+    }, {
+      code: [
+        'class Hello extends React.Component {',
+        '  render() {',
+        '    var { ',
+        '      propX,',
+        '      "aria-controls": ariaControls, ',
+        '      ...props } = this.props;',
+        '    return <div>Hello</div>;',
+        '  }',
+        '}',
+        'Hello.propTypes = {',
+        '  "propX": React.PropTypes.string,',
+        '  "aria-controls": React.PropTypes.string',
+        '};'
+      ].join('\n'),
+      parser: 'babel-eslint'
     }
   ],
 
@@ -736,6 +753,25 @@ eslintTester.addRuleTest('lib/rules/prop-types', {
         {message: '\'numb.propX\' is missing in props validation for Hello'},
         {message: '\'stri.tooString\' is missing in props validation for Hello'}
       ]
+    }, {
+      code: [
+        'class Hello extends React.Component {',
+        '  render() {',
+        '    var { ',
+        '      "aria-controls": ariaControls, ',
+        '      propX,',
+        '      ...props } = this.props;',
+        '    return <div>Hello</div>;',
+        '  }',
+        '}',
+        'Hello.propTypes = {',
+        '  "aria-controls": React.PropTypes.string',
+        '};'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      errors: [
+        {message: '\'propX\' is missing in props validation for Hello'}
+      ]
     }
   ]
 });