Skip to content

Commit 1f3cac2

Browse files
committed
Make resolveToValue consider assignments to identifiers (fixes #58)
In the following example, resolveToValue wouldn't resolve to the value assigned to "React" because it only looks at the variable definition, not at assignments to the variable: ``` var React; React = require('react'); module.exports = React.createClass(...); ``` This diff fixes that. It will make a shallow traversal over the scope where the variable is defined and look for the last assignment to the variable. --- I'm also trying to improve tests by storing component definitions that are known to have been broken in `src/__tests__/fixtures/`. Adding more components over time will strengthen the test suit and help catch regressions.
1 parent 10af890 commit 1f3cac2

File tree

6 files changed

+91
-14
lines changed

6 files changed

+91
-14
lines changed

package.json

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,6 @@
5858
"bin",
5959
"src"
6060
],
61-
"testPathIgnorePatterns": [
62-
"/bin/__tests__/example/"
63-
]
61+
"testRegex": "/__tests__/.*-test\\.js$"
6462
}
6563
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
exports[`main fixtures processes component "component_1.js" without errors 1`] = `
2+
Object {
3+
"description": "The is a component to test the document generation",
4+
"displayName": "Component",
5+
"methods": Array []
6+
}
7+
`;

src/__tests__/fixtures/component_1.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
var Component, React;
2+
3+
React = require('react');
4+
5+
/**
6+
* The is a component to test the document generation
7+
*/
8+
Component = React.createClass({
9+
displayName: 'Component',
10+
});
11+
12+
module.exports = Component;

src/__tests__/main-test.js

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,15 @@
88
*
99
*/
1010

11-
/*global jest, describe, beforeEach, it, expect*/
11+
/*global describe, it, expect*/
1212

13-
jest.disableAutomock();
13+
import fs from 'fs';
14+
import path from 'path';
1415

15-
describe('main', () => {
16-
var docgen, ERROR_MISSING_DEFINITION;
16+
import * as docgen from '../main';
17+
import {ERROR_MISSING_DEFINITION} from '../parse';
1718

18-
beforeEach(() => {
19-
docgen = require('../main');
20-
({ERROR_MISSING_DEFINITION} = require('../parse'));
21-
});
19+
describe('main', () => {
2220

2321
function test(source) {
2422
it('parses with default resolver/handlers', () => {
@@ -225,4 +223,17 @@ describe('main', () => {
225223
expect(() => docgen.parse(source)).toThrowError(ERROR_MISSING_DEFINITION);
226224
});
227225
});
226+
227+
describe('fixtures', () => {
228+
const fixturePath = path.join(__dirname, 'fixtures');
229+
const fileNames = fs.readdirSync(fixturePath);
230+
for (let i = 0; i < fileNames.length; i++) {
231+
const fileContent = fs.readFileSync(path.join(fixturePath, fileNames[i]));
232+
233+
it(`processes component "${fileNames[i]}" without errors`, () => {
234+
expect(() => docgen.parse(fileContent)).not.toThrowError();
235+
expect(docgen.parse(fileContent)).toMatchSnapshot();
236+
});
237+
}
238+
});
228239
});

src/utils/__tests__/resolveToValue-test.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,18 @@ describe('resolveToValue', () => {
9191
.toBe('FunctionDeclaration');
9292
});
9393

94+
describe('assignments', () => {
95+
it('resolves to assigned values', () => {
96+
var path = parse([
97+
'var foo;',
98+
'foo = 42;',
99+
'foo;',
100+
].join('\n'));
101+
102+
expect(resolveToValue(path).node).toEqualASTNode(builders.literal(42));
103+
});
104+
});
105+
94106
describe('ImportDeclaration', () => {
95107

96108
it('resolves default import references to the import declaration', () => {

src/utils/resolveToValue.js

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
*/
1212

1313
import recast from 'recast';
14+
import {traverseShallow} from './traverse';
1415

1516
var {
1617
types: {
@@ -71,6 +72,36 @@ function findScopePath(paths: Array<NodePath>, path: NodePath): ?NodePath {
7172
}
7273
}
7374

75+
/**
76+
* Tries to find the last value assigned to `name` in the scope created by
77+
* `scope`. We are not descending into any statements (blocks).
78+
*/
79+
function findLastAssignedValue(scope, name) {
80+
let results = [];
81+
82+
traverseShallow(scope.path.node, {
83+
visitAssignmentExpression: function(path) {
84+
const node = path.node;
85+
// Skip anything that is not an assignment to a variable with the
86+
// passed name.
87+
if (
88+
!types.Identifier.check(node.left) ||
89+
node.left.name !== name ||
90+
node.operator !== '='
91+
) {
92+
return this.traverse(path);
93+
}
94+
results.push(path.get('right'));
95+
return false;
96+
},
97+
});
98+
99+
if (results.length === 0) {
100+
return null;
101+
}
102+
return resolveToValue(results.pop());
103+
}
104+
74105
/**
75106
* If the path is an identifier, it is resolved in the scope chain.
76107
* If it is an assignment expression, it resolves to the right hand side.
@@ -91,7 +122,7 @@ export default function resolveToValue(path: NodePath): NodePath {
91122
return path.parentPath;
92123
} else if (types.AssignmentExpression.check(node)) {
93124
if (node.operator === '=') {
94-
return resolveToValue(node.get('right'));
125+
return resolveToValue(path.get('right'));
95126
}
96127
} else if (types.Identifier.check(node)) {
97128
if ((types.ClassDeclaration.check(path.parentPath.node) ||
@@ -104,8 +135,14 @@ export default function resolveToValue(path: NodePath): NodePath {
104135
let scope = path.scope.lookup(node.name);
105136
let resolvedPath: ?NodePath;
106137
if (scope) {
107-
const bindings = scope.getBindings()[node.name];
108-
resolvedPath = findScopePath(bindings, path);
138+
// The variable may be assigned a different value after initialization.
139+
// We are first trying to find all assignments to the variable in the
140+
// block where it is defined (i.e. we are not traversing into statements)
141+
resolvedPath = findLastAssignedValue(scope, node.name);
142+
if (!resolvedPath) {
143+
const bindings = scope.getBindings()[node.name];
144+
resolvedPath = findScopePath(bindings, path);
145+
}
109146
} else {
110147
scope = path.scope.lookupType(node.name);
111148
if (scope) {

0 commit comments

Comments
 (0)