From 04f573f762c469af526728de129479b50b3eb8b9 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Wed, 12 Feb 2020 23:59:06 +0000 Subject: [PATCH] fix: avoid circular reference when resolving name from within assignment This minimal tweak to `resolveToValue` allows us, in particular, to follow some reassignments of the variable holding a component, e.g. when wrapping it with a HOC. --- .../__tests__/flowTypeHandler-test.js | 20 +++++++++++++++++++ src/utils/__tests__/resolveToValue-test.js | 20 +++++++++++++++++++ src/utils/resolveToValue.js | 16 ++++++++++++--- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/handlers/__tests__/flowTypeHandler-test.js b/src/handlers/__tests__/flowTypeHandler-test.js index c05167409a5..33c2f845cc8 100644 --- a/src/handlers/__tests__/flowTypeHandler-test.js +++ b/src/handlers/__tests__/flowTypeHandler-test.js @@ -368,5 +368,25 @@ describe('flowTypeHandler', () => { }, }); }); + + it('resolves when the function is rebound and not inline', () => { + const src = ` + import React from 'react'; + type Props = { foo: string }; + let Component = (props: Props, ref) =>
{props.foo}
; + Component = React.forwardRef(Component); + `; + flowTypeHandler( + documentation, + parse(src).get('body', 3, 'expression', 'right'), + ); + expect(documentation.descriptors).toEqual({ + foo: { + flowType: {}, + required: true, + description: '', + }, + }); + }); }); }); diff --git a/src/utils/__tests__/resolveToValue-test.js b/src/utils/__tests__/resolveToValue-test.js index a7f8fe1f528..7294eb299c3 100644 --- a/src/utils/__tests__/resolveToValue-test.js +++ b/src/utils/__tests__/resolveToValue-test.js @@ -109,6 +109,26 @@ describe('resolveToValue', () => { expect(resolveToValue(path)).toEqualASTNode(builders.literal(42)); }); + + it('resolves to other assigned value if ref is in an assignment lhs', () => { + const path = parsePath( + ['var foo;', 'foo = 42;', 'foo = wrap(foo);'].join('\n'), + ); + + expect(resolveToValue(path.get('left'))).toEqualASTNode( + builders.literal(42), + ); + }); + + it('resolves to other assigned value if ref is in an assignment rhs', () => { + const path = parsePath( + ['var foo;', 'foo = 42;', 'foo = wrap(foo);'].join('\n'), + ); + + expect(resolveToValue(path.get('right', 'arguments', 0))).toEqualASTNode( + builders.literal(42), + ); + }); }); describe('ImportDeclaration', () => { diff --git a/src/utils/resolveToValue.js b/src/utils/resolveToValue.js index 7200ec4d995..039092c08d7 100644 --- a/src/utils/resolveToValue.js +++ b/src/utils/resolveToValue.js @@ -72,22 +72,32 @@ function findScopePath(paths: Array, path: NodePath): ?NodePath { * Tries to find the last value assigned to `name` in the scope created by * `scope`. We are not descending into any statements (blocks). */ -function findLastAssignedValue(scope, name) { +function findLastAssignedValue(scope, idPath) { const results = []; + const name = idPath.node.name; traverseShallow(scope.path, { visitAssignmentExpression: function(path) { const node = path.node; // Skip anything that is not an assignment to a variable with the // passed name. + // Ensure the LHS isn't the reference we're trying to resolve. if ( !t.Identifier.check(node.left) || + node.left === idPath.node || node.left.name !== name || node.operator !== '=' ) { return this.traverse(path); } - results.push(path.get('right')); + // Ensure the RHS doesn't contain the reference we're trying to resolve. + const candidatePath = path.get('right'); + for (let p = idPath; p && p.node != null; p = p.parent) { + if (p.node === candidatePath.node) { + return this.traverse(path); + } + } + results.push(candidatePath); return false; }, }); @@ -159,7 +169,7 @@ export default function resolveToValue(path: NodePath): NodePath { // The variable may be assigned a different value after initialization. // We are first trying to find all assignments to the variable in the // block where it is defined (i.e. we are not traversing into statements) - resolvedPath = findLastAssignedValue(scope, node.name); + resolvedPath = findLastAssignedValue(scope, path); if (!resolvedPath) { const bindings = scope.getBindings()[node.name]; resolvedPath = findScopePath(bindings, path);