Skip to content

Commit

Permalink
Fix renameTo not taking property shorthands into account (#211)
Browse files Browse the repository at this point in the history
* Fix `renameTo` not taking property shorthands into account

This fixes the `renameTo` helper method not properly renaming
object properties that are shorthands.

The previous version of the code would rename the property's
`value` correctly, but since `shorthand` was still set as `true`,
the output result would still be the old variable as a shorthand
property.

Now, if an identifier to be renamed is of type `Property` and
is a shorthand (but not a method), we flip the `shorthand` value
to be `false`.

* babel -> getParser()

* Add comment that explains why this works
  • Loading branch information
vitorbal authored and fkling committed Mar 7, 2018
1 parent e60662b commit 99aaae5
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
15 changes: 15 additions & 0 deletions src/collections/VariableDeclarator.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,21 @@ const transformMethods = {
scope = scope.parent;
}
if (scope) { // identifier must refer to declared variable

// It may look like we filtered out properties,
// but the filter only ignored property "keys", not "value"s
// In shorthand properties, "key" and "value" both have an
// Identifier with the same structure.
const parent = path.parent.node;
if (
types.Property.check(parent) &&
parent.shorthand &&
!parent.method
) {

path.parent.get('shorthand').replace(false);
}

path.get('name').replace(newName);
}
});
Expand Down
31 changes: 31 additions & 0 deletions src/collections/__tests__/VariableDeclarator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,37 @@ describe('VariableDeclarators', function() {

expect(identifiers.length).toBe(1);
});

it('properly renames a shorthand property that was using the old variable name', function() {
nodes = [recast.parse([
'var foo = 42;',
'var obj2 = {',
' foo,',
'};',
].join('\n'), {parser: getParser()}).program];

// Outputs:
// var newFoo = 42;
// var obj2 = {
// foo: newFoo,
// };
Collection.fromNodes(nodes)
.findVariableDeclarators('foo').renameTo('newFoo');

expect(
Collection.fromNodes(nodes).find(types.Identifier, { name: 'newFoo' }).length
).toBe(2);
expect(
Collection.fromNodes(nodes).find(types.Identifier, { name: 'foo' }).length
).toBe(1);

expect(
Collection.fromNodes(nodes).find(types.Property).filter(prop => !prop.value.shorthand).length
).toBe(1);
expect(
Collection.fromNodes(nodes).find(types.Property).filter(prop => prop.value.shorthand).length
).toBe(0);
});

it('does not rename React component prop name', function () {
const declarators = Collection.fromNodes(nodes)
Expand Down

0 comments on commit 99aaae5

Please sign in to comment.