Error while replacing identifier of a PropertyAccessExpression

[gregjacobs]

Hey @dsherret, long time no talk! How's it going?

I'm back doing ts transforms (yay!), but ran into an issue while trying to replace the object name (expression) in a PropertyAccessExpression.

I'm basically trying to convert that.something to this.something, but if the there are 4 chained PropertyAccessExpressions, I get an error. Try running this code:

import Project, { Identifier, PropertyAccessExpression, SyntaxKind, TypeGuards } from 'ts-simple-ast';

const project = new Project();
const sourceFile = project.createSourceFile( 'my-file.ts', `
	that.something.something2.something3;   // <-- too nested!
` );


const thatDotSomething: PropertyAccessExpression = sourceFile
	.getDescendantsOfKind( SyntaxKind.PropertyAccessExpression )
	.find( propAccess => TypeGuards.isIdentifier( propAccess.getExpression() ) )!;

const thatIdentifier = thatDotSomething.getExpression() as Identifier;
thatIdentifier.replaceWithText( 'this' );

console.log( sourceFile.getFullText() );

I get the error: Error: Error replacing tree: Should not have more children left over.

Oddly enough, when I remove one property and change that.something.something2.something3 to that.something.something2, the code works fine.

Can you take a look into this? Thanks!

[cancerberoSgx]

I think is an issue indeed. First of all I suggest you use rename() instead o replaceWithText(). Is safer, wont make the nodes obsolete and also it will rename all the references to that symbol in the whole project (just like TypeScript rename feature). Nevertheless rename() has the same problem.

Fortunately you can workaround it by renaming the Identifier definition node:

play with the code

import Project, { Identifier, PropertyAccessExpression, SyntaxKind, TypeGuards, Node, DeclarationNamedNode } from 'ts-simple-ast'

function canBeRenamed(node: Node): node is Node & { rename: (name: string)=> Node; } {
  return typeof (node as any).rename === 'function'
}

const project = new Project({useVirtualFileSystem: true});
const sourceFile = project.createSourceFile( 'my-file2.ts', `
let a, x
a.b.c.d = 3;   // <-- too nested!
x=1
` );

const aInExpression = sourceFile.getDescendants().find(d => TypeGuards.isIdentifier(d) && 
  TypeGuards.isExpression(d.getParent()) && d.getText()==='a') as Identifier

// renaming the id in the expression doesn't work: 
// aInExpression.rename('RENAMED1'); 

// however renaming the definitino node does:
// aInExpression.getDefinitionNodes().filter(canBeRenamed).forEach(n => n.rename('RENAMED2')) // n is VariableDeclaration

// also renaming the id of the variable declaration (and not the variable declaration itseof) works:
sourceFile.getDescendants().filter(d => TypeGuards.isIdentifier(d) && 
  TypeGuards.isVariableDeclaration(d.getParent()) && d.getText()==='a').
  filter(canBeRenamed).forEach(n=>n.rename('RENAMED3')); 
log(sourceFile.getFullText());

[gregjacobs]

Interesting, I didn't realize there was a rename method. Thanks for that!

Although an interesting point in this case might be that I'm "renaming" from an identifier node to a ThisKeyword node. Will that still work? (I haven't checked the code to see if there's a special case for the string "this")

And of course as you pointed out, the renaming still doesn't work on its own with this level of nested property accesses :/

[cancerberoSgx]

I think is weird to anIdentifier.rename('this') conceptually since what you really want to do is replace the Identifier for a ThisKeyword node - (not renaming an Id). Perhaps replaceText would be more appropriate but I think the ideal tool for what you want to accomplish is ts.transformations. In the playground you have three transformation examples in typescript native compiler API. The library doesn't have that API wrapped so you will need to do it in the compilerNode (sourceFile.compilerNode) save the file and reload the ts-simple-ast project to get the changes. My two cents

[dsherret]

Hey @gregjacobs! I've been doing well thanks. I hope you have been too. Nice to see you back, but sorry that it's because of a bug!

Thanks for reporting this. I've fixed it now in 12.6.1 (available now). Just use the return value of replaceWithText to get the new ThisKeyword as the previous Identifier will be forgotten.

const thatIdentifier = ...;
const thisKeyword = thatIdentifier.replaceWithText("this");

thatIdentifier.getText(); // throws because node was forgotten since the kind changed
thisKeyword.getText();    // ok

[gregjacobs]

@dsherret Thanks for the quick fix! Definitely good to be back - I really love this stuff and I missed it :)

Very good to know about the return value from replaceWithText() - didn't realize that it did that. Many thanks, and keep up the great work!