refactor

Shopify · Apr 12, 2019 · c3469da · c3469da
1 parent 1935564
commit c3469da
Showing 1 changed file with 108 additions and 103 deletions.
diff --git a/lib/rules/hooks-strict-return.js b/lib/rules/hooks-strict-return.js
@@ -1,4 +1,5 @@
 const {docsUrl} = require('../utilities');
+
 const MAX_RETURN_ELEMENTS = 2;
 
 module.exports = {
@@ -8,79 +9,16 @@ module.exports = {
       category: 'Best Practices',
       recommended: true,
       uri: docsUrl('hooks-strict-return'),
-      messages: {
-        hooksStrictReturn:
-          'Hooks must return a tuple of two or fewer values or a single object.',
-      },
+    },
+    messages: {
+      hooksStrictReturn:
+        'Hooks must return a tuple of two or fewer values or a single object.',
     },
   },
+
   create(context) {
     let inHook = false;
 
-    function exceedsMaxReturnElements(node, max) {
-      if (node.argument.type === 'ArrayExpression') {
-        return node.argument && node.argument.elements.length > max;
-      }
-
-      if (node.argument.type === 'ObjectExpression') {
-        const elements = node.argument.properties.reduce((acc, val) => {
-          const prop = isSpreadElement(val) ? gatherProperties(val) : val;
-
-          return [...acc, prop];
-        }, []);
-
-        return elements.length > max;
-      }
-
-      if (node.argument.type === 'Identifier') {
-        const {references} = getVariableByName(
-          context.getScope(),
-          node.argument.name,
-        );
-        const elements =
-          references &&
-          references.reduce((acc, ref) => {
-            if (
-              ref.identifier &&
-              ref.identifier.parent &&
-              ref.identifier.parent.init &&
-              ref.identifier.parent.init.elements
-            ) {
-              return [...acc, ...ref.identifier.parent.init.elements];
-            }
-            return acc;
-          }, []);
-
-        return elements.length > max;
-      }
-
-      return gatherProperties(node).length > max;
-    }
-
-    function gatherProperties(val) {
-      const {references} =
-        getVariableByName(context.getScope(), val.argument.name) || {};
-
-      return (
-        references &&
-        references
-          .map((ref) => {
-            if (
-              ref.identifier &&
-              ref.identifier.parent &&
-              ref.identifier.parent.init &&
-              ref.identifier.parent.init.properties
-            ) {
-              return ref.identifier.parent.init.properties;
-            }
-
-            return null;
-          })
-          .filter((el) => el)
-          .flat()
-      );
-    }
-
     return {
       FunctionDeclaration(node) {
         if (!isHookDeclaration(node)) {
@@ -89,29 +27,86 @@ module.exports = {
 
         inHook = true;
       },
-      ReturnStatement(node) {
-        if (!inHook) {
-          return;
-        }
-        if (!exceedsMaxReturnElements(node, MAX_RETURN_ELEMENTS)) {
+      'FunctionDeclaration:exit': function(node) {
+        if (!isHookDeclaration(node)) {
           return;
         }
 
-        context.report({message: 'noooo', node});
+        inHook = false;
       },
-      'FunctionDeclaration:exit': function(node) {
-        if (!isHookDeclaration(node)) {
+      ReturnStatement(node) {
+        if (!inHook) {
+          return;
+        }
+        if (
+          !exceedsMaxReturnProperties(
+            node,
+            context.getScope(),
+            MAX_RETURN_ELEMENTS,
+          )
+        ) {
           return;
         }
 
-        inHook = false;
+        context.report({
+          messageId: 'hooksStrictReturn',
+          node,
+        });
       },
     };
   },
 };
 
+function exceedsMaxReturnProperties(node, scope, max) {
+  let props;
+  switch (node.argument.type) {
+    case 'ArrayExpression': {
+      props = node.argument && node.argument.elements;
+      break;
+    }
+    case 'ObjectExpression': {
+      props = getPropertiesForObject(node);
+      break;
+    }
+
+    case 'Identifier': {
+      const {references} = getVariableByName(scope, node.argument.name);
+      props = getPropertiesForIdentifier(references);
+      break;
+    }
+    default: {
+      props = gatherProperties(node, scope);
+    }
+  }
+
+  return props && props.length > max;
+}
+
+function gatherProperties(val, scope) {
+  const {references} = getVariableByName(scope, val.argument.name) || {};
+
+  return (
+    references &&
+    references
+      .map((ref) => {
+        if (
+          ref.identifier &&
+          ref.identifier.parent &&
+          ref.identifier.parent.init &&
+          ref.identifier.parent.init.properties
+        ) {
+          return ref.identifier.parent.init.properties;
+        }
+
+        return null;
+      })
+      .filter((el) => el)
+      .flat()
+  );
+}
+
 function isHookDeclaration(node) {
-  return node.id.name.startsWith('use');
+  return /^use[A-Z0-9].*$/.test(node.id.name);
 }
 
 function isSpreadElement(node) {
@@ -135,32 +130,42 @@ function getVariableByName(initScope, name) {
 
   return null;
 }
+function getPropertiesForObject(node) {
+  node.argument.properties.reduce((acc, val) => {
+    const prop = isSpreadElement(val) ? gatherProperties(val) : val;
 
-/**
- * Catch all identifiers that begin with "use" followed by an uppercase Latin
- * character to exclude identifiers like "user".
- */
-
-function isHookName(s) {
-  return /^use[A-Z0-9].*$/.test(s);
+    return [...acc, prop];
+  }, []);
 }
 
-/**
- * We consider hooks to be a hook name identifier or a member expression
- * containing a hook name.
- */
-
-function isHook(node) {
-  if (node.type === 'Identifier') {
-    return isHookName(node.name);
-  } else if (
-    node.type === 'MemberExpression' &&
-    !node.computed &&
-    isHook(node.property)
-  ) {
-    const obj = node.object;
-    return obj.type === 'Identifier' && obj.name === 'React';
-  } else {
-    return false;
-  }
+function getPropertiesForIdentifier(references) {
+  return (
+    references &&
+    references.reduce((acc, ref) => {
+      if (
+        ref.identifier &&
+        ref.identifier.parent &&
+        ref.identifier.parent.init &&
+        ref.identifier.parent.init.elements
+      ) {
+        return [...acc, ...ref.identifier.parent.init.elements];
+      }
+      return acc;
+    }, [])
+  );
 }
+
+// function isHook(node) {
+//   if (node.type === 'Identifier') {
+//     return isHookName(node.name);
+//   } else if (
+//     node.type === 'MemberExpression' &&
+//     !node.computed &&
+//     isHook(node.property)
+//   ) {
+//     const obj = node.object;
+//     return obj.type === 'Identifier' && obj.name === 'React';
+//   } else {
+//     return false;
+//   }
+// }