Fix handling of identifiers in forbid-prop-types.

This fixes incorrect behavior when propTypes were stored in a variable and then referenced later. #1352
jsx-eslint · Aug 11, 2017 · ab769b4 · ab769b4
1 parent dae6574
commit ab769b4
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 36 deletions.
diff --git a/lib/rules/forbid-prop-types.js b/lib/rules/forbid-prop-types.js
@@ -3,6 +3,8 @@
  */
 'use strict';
 
+const variableUtil = require('../util/variable');
+
 // ------------------------------------------------------------------------------
 // Constants
 // ------------------------------------------------------------------------------
@@ -67,14 +69,33 @@ module.exports = {
       );
     }
 
+    /**
+     * Find a variable by name in the current scope.
+     * @param  {string} name Name of the variable to look for.
+     * @returns {ASTNode|null} Return null if the variable could not be found, ASTNode otherwise.
+     */
+    function findVariableByName(name) {
+      const variable = variableUtil.variablesInScope(context).find(item => item.name === name);
+
+      if (!variable || !variable.defs[0] || !variable.defs[0].node) {
+        return null;
+      }
+
+      if (variable.defs[0].node.type === 'TypeAlias') {
+        return variable.defs[0].node.right;
+      }
+
+      return variable.defs[0].node.init;
+    }
+
 
     /**
      * Checks if propTypes declarations are forbidden
      * @param {Array} declarations The array of AST nodes being checked.
      * @returns {void}
      */
-    function checkForbidden(declarations) {
-      (declarations || []).forEach(declaration => {
+    function checkProperties(declarations) {
+      declarations.forEach(declaration => {
         if (declaration.type !== 'Property') {
           return;
         }
@@ -108,49 +129,42 @@ module.exports = {
       });
     }
 
+    function checkNode(node) {
+      switch (node && node.type) {
+        case 'ObjectExpression':
+          checkProperties(node.properties);
+          break;
+        case 'Identifier':
+          const propTypesObject = findVariableByName(node.name);
+          if (propTypesObject) {
+            checkProperties(propTypesObject.properties);
+          }
+          break;
+        case 'CallExpression':
+          const innerNode = node.arguments && node.arguments[0];
+          if (propWrapperFunctions.has(node.callee.name) && innerNode) {
+            checkNode(innerNode);
+          }
+          break;
+        default:
+          break;
+      }
+    }
+
     return {
       ClassProperty: function(node) {
         if (!isPropTypesDeclaration(node)) {
           return;
         }
-        switch (node.value && node.value.type) {
-          case 'ObjectExpression':
-            checkForbidden(node.value.properties);
-            break;
-          case 'CallExpression':
-            if (
-              propWrapperFunctions.has(node.value.callee.name) &&
-              node.value.arguments && node.value.arguments[0]
-            ) {
-              checkForbidden(node.value.arguments[0].properties);
-            }
-            break;
-          default:
-            break;
-        }
+        checkNode(node.value);
       },
 
       MemberExpression: function(node) {
         if (!isPropTypesDeclaration(node.property)) {
           return;
         }
 
-        const right = node.parent.right;
-        switch (right && right.type) {
-          case 'ObjectExpression':
-            checkForbidden(right.properties);
-            break;
-          case 'CallExpression':
-            if (
-              propWrapperFunctions.has(right.callee.name) &&
-              right.arguments && right.arguments[0]
-            ) {
-              checkForbidden(right.arguments[0].properties);
-            }
-            break;
-          default:
-            break;
-        }
+        checkNode(node.parent.right);
       },
 
       ObjectExpression: function(node) {
@@ -163,7 +177,7 @@ module.exports = {
             return;
           }
           if (property.value.type === 'ObjectExpression') {
-            checkForbidden(property.value.properties);
+            checkProperties(property.value.properties);
           }
         });
       }

diff --git a/tests/lib/rules/forbid-prop-types.js b/tests/lib/rules/forbid-prop-types.js
@@ -398,15 +398,22 @@ ruleTester.run('forbid-prop-types', rule, {
     settings: {
       propWrapperFunctions: ['forbidExtraProps']
     }
+  }, {
+    code: [
+      'import { forbidExtraProps } from "airbnb-prop-types";',
+      'export const propTypes = {dpm: PropTypes.any};',
+      'export default function Component() {}',
+      'Component.propTypes = propTypes;'
+    ].join('\n'),
+    errors: [{message: ANY_ERROR_MESSAGE}]
   }, {
     code: [
       'import { forbidExtraProps } from "airbnb-prop-types";',
       'export const propTypes = {a: PropTypes.any};',
       'export default function Component() {}',
       'Component.propTypes = forbidExtraProps(propTypes);'
     ].join('\n'),
-    // errors: [{message: ANY_ERROR_MESSAGE}], // TODO: make this pass
-    errors: [],
+    errors: [{message: ANY_ERROR_MESSAGE}],
     settings: {
       propWrapperFunctions: ['forbidExtraProps']
     }