Fix resolving of more than two Flow Utility types (#345)

* Refactor code to do unwrapping of flow utility types in one place

* Simplify unwrapping of utility types

* Restore old functionality of returning falsey value

src/handlers/__tests__/__snapshots__/flowTypeHandler-test.js.snap

@@ -0,0 +1,11 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`flowTypeHandler does support utility types inline 1`] = `
+Object {
+  "foo": Object {
+    "description": "",
+    "flowType": Object {},
+    "required": true,
+  },
+}
+`;

src/handlers/__tests__/flowTypeHandler-test.js

@@ -248,6 +248,20 @@ describe('flowTypeHandler', () => {
     });
   });
 
+  it('does support utility types inline', () => {
+    const definition = statement(`
+      (props: $ReadOnly<Props>) => <div />;
+
+      var React = require('React');
+
+      type Props = { foo: 'fooValue' };
+    `).get('expression');
+
+    expect(() => flowTypeHandler(documentation, definition)).not.toThrow();
+
+    expect(documentation.descriptors).toMatchSnapshot();
+  });
+
   it('does not support union proptypes', () => {
     const definition = statement(`
       (props: Props) => <div />;

src/handlers/flowTypeHandler.js

@@ -20,20 +20,14 @@ import getFlowTypeFromReactComponent, {
 } from '../utils/getFlowTypeFromReactComponent';
 import resolveToValue from '../utils/resolveToValue';
 import setPropDescription from '../utils/setPropDescription';
-import {
-  isSupportedUtilityType,
-  unwrapUtilityType,
-} from '../utils/flowUtilityTypes';
+import { unwrapUtilityType } from '../utils/flowUtilityTypes';
 
 const {
   types: { namedTypes: types },
 } = recast;
 function setPropDescriptor(documentation: Documentation, path: NodePath): void {
   if (types.ObjectTypeSpreadProperty.check(path.node)) {
-    let argument = path.get('argument');
-    while (isSupportedUtilityType(argument)) {
-      argument = unwrapUtilityType(argument);
-    }
+    const argument = unwrapUtilityType(path.get('argument'));
 
     if (types.ObjectTypeAnnotation.check(argument.node)) {
       applyToFlowTypeProperties(argument, propertyPath => {

src/utils/__tests__/flowUtilityTypes-test.js

@@ -0,0 +1,84 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+/*global describe, it, expect*/
+
+import { unwrapUtilityType, isSupportedUtilityType } from '../flowUtilityTypes';
+
+import { statement } from '../../../tests/utils';
+
+describe('flowTypeUtilities', () => {
+  describe('unwrapUtilityType', () => {
+    it('correctly unwraps', () => {
+      const def = statement(`
+        type A = $ReadOnly<{ foo: string }>
+      `);
+
+      expect(unwrapUtilityType(def.get('right'))).toBe(
+        def.get('right', 'typeParameters', 'params', 0),
+      );
+    });
+
+    it('correctly unwraps double', () => {
+      const def = statement(`
+        type A = $ReadOnly<$ReadOnly<{ foo: string }>>
+      `);
+
+      expect(unwrapUtilityType(def.get('right'))).toBe(
+        def.get(
+          'right',
+          'typeParameters',
+          'params',
+          0,
+          'typeParameters',
+          'params',
+          0,
+        ),
+      );
+    });
+
+    it('correctly unwraps triple', () => {
+      const def = statement(`
+        type A = $ReadOnly<$ReadOnly<$ReadOnly<{ foo: string }>>>
+      `);
+
+      expect(unwrapUtilityType(def.get('right'))).toBe(
+        def.get(
+          'right',
+          'typeParameters',
+          'params',
+          0,
+          'typeParameters',
+          'params',
+          0,
+          'typeParameters',
+          'params',
+          0,
+        ),
+      );
+    });
+  });
+
+  describe('isSupportedUtilityType', () => {
+    it('correctly returns true for valid type', () => {
+      const def = statement(`
+        type A = $Exact<{ foo: string }>
+      `);
+
+      expect(isSupportedUtilityType(def.get('right'))).toBe(true);
+    });
+
+    it('correctly returns false for invalid type', () => {
+      const def = statement(`
+        type A = $Homer<{ foo: string }>
+      `);
+
+      expect(isSupportedUtilityType(def.get('right'))).toBe(false);
+    });
+  });
+});

src/utils/flowUtilityTypes.js

@@ -24,7 +24,7 @@ const supportedUtilityTypes = new Set(['$Exact', '$ReadOnly']);
 export function isSupportedUtilityType(path: NodePath): boolean {
   if (types.GenericTypeAnnotation.check(path.node)) {
     const idPath = path.get('id');
-    return Boolean(idPath) && supportedUtilityTypes.has(idPath.node.name);
+    return !!idPath && supportedUtilityTypes.has(idPath.node.name);
   }
   return false;
 }
@@ -35,5 +35,9 @@ export function isSupportedUtilityType(path: NodePath): boolean {
  *   $ReadOnly<T> => T
  */
 export function unwrapUtilityType(path: NodePath): NodePath {
-  return path.get('typeParameters', 'params', 0);
+  while (isSupportedUtilityType(path)) {
+    path = path.get('typeParameters', 'params', 0);
+  }
+
+  return path;
 }

src/utils/getFlowTypeFromReactComponent.js

@@ -14,19 +14,8 @@ import getTypeAnnotation from '../utils/getTypeAnnotation';
 import getMemberValuePath from '../utils/getMemberValuePath';
 import isReactComponentClass from '../utils/isReactComponentClass';
 import isStatelessComponent from '../utils/isStatelessComponent';
-import isUnreachableFlowType from '../utils/isUnreachableFlowType';
-import recast from 'recast';
-import resolveToValue from '../utils/resolveToValue';
-import {
-  isSupportedUtilityType,
-  unwrapUtilityType,
-} from '../utils/flowUtilityTypes';
 import resolveGenericTypeAnnotation from '../utils/resolveGenericTypeAnnotation';
 
-const {
-  types: { namedTypes: types },
-} = recast;
-
 /**
  * Given an React component (stateless or class) tries to find the
  * flow type for the props. If not found or not one of the supported
@@ -59,21 +48,6 @@ export default (path: NodePath): ?NodePath => {
     typePath = getTypeAnnotation(param);
   }
 
-  if (typePath && isSupportedUtilityType(typePath)) {
-    typePath = unwrapUtilityType(typePath);
-  } else if (typePath && types.GenericTypeAnnotation.check(typePath.node)) {
-    typePath = resolveToValue(typePath.get('id'));
-    if (
-      !typePath ||
-      types.Identifier.check(typePath.node) ||
-      isUnreachableFlowType(typePath)
-    ) {
-      return;
-    }
-
-    typePath = typePath.get('right');
-  }
-
   return typePath;
 };
 

src/utils/resolveGenericTypeAnnotation.js

@@ -13,32 +13,40 @@
 import isUnreachableFlowType from '../utils/isUnreachableFlowType';
 import recast from 'recast';
 import resolveToValue from '../utils/resolveToValue';
-import { isSupportedUtilityType, unwrapUtilityType } from './flowUtilityTypes';
+import { unwrapUtilityType } from './flowUtilityTypes';
 
 const {
   types: { namedTypes: types },
 } = recast;
 
+function tryResolveGenericTypeAnnotation(path: NodePath): ?NodePath {
+  let typePath = unwrapUtilityType(path);
+
+  if (types.GenericTypeAnnotation.check(typePath.node)) {
+    typePath = resolveToValue(typePath.get('id'));
+    if (isUnreachableFlowType(typePath)) {
+      return;
+    }
+
+    return tryResolveGenericTypeAnnotation(typePath.get('right'));
+  }
+
+  return typePath;
+}
+
 /**
  * Given an React component (stateless or class) tries to find the
  * flow type for the props. If not found or not one of the supported
- * component types returns null.
+ * component types returns undefined.
  */
 export default function resolveGenericTypeAnnotation(
   path: NodePath,
 ): ?NodePath {
-  // If the node doesn't have types or properties, try to get the type.
-  let typePath: ?NodePath;
-  if (path && isSupportedUtilityType(path)) {
-    typePath = unwrapUtilityType(path);
-  } else if (path && types.GenericTypeAnnotation.check(path.node)) {
-    typePath = resolveToValue(path.get('id'));
-    if (isUnreachableFlowType(typePath)) {
-      return;
-    }
+  if (!path) return;
 
-    typePath = typePath.get('right');
-  }
+  const typePath = tryResolveGenericTypeAnnotation(path);
+
+  if (!typePath || typePath === path) return;
 
   return typePath;
 }