Expand API for tree manipulation

[creynders]

Maybe I'm missing something fundamental here, but it seems the API is very limited with regards to tree manipulation (or the documentation is lacking). E.g. I'd want to replace the arguments of a CallExpression, but the ArrayExpression node only provides methods to add nodes, not remove them. Obviously I can start hacking away and start doing stuff like methodCall.arguments.nodes[0].pop(), but that's verrrry yucky.

Am I missing the point of your lib, or is this planned, or did I just somehow miss this?

[creynders]

Also, what's the use of having the ArrayExpression push etc methods, if none of the node classes are exposed? Obviously we could go the require('ast-query/lib/nodes').ObjectExpression route, but that's hardly inviting :)

[SBoudrias]

Hey, there's still holes in the API. It is still a young project, and is not complete. Every PR adding new functionnality is welcome!

About the push method, you need to pass a code string representing the value. e.g.:

array.push("1"); // [ 1 ]
array.push("{ bar:  1 }"); // [{ bar: 1 }]
array.push("'text'"); // [ "text" ]

[creynders]

About the push method, you need to pass a code string representing the value.

Ah, yes of course, thanks.

I'd gladly help flesh out the API, but am still a bit unsure about the direction this is going, for instance: would you consider it useful to have node list iteration methods .next, .first, etc?

[SBoudrias]

Yes, I believe these could be useful.

[creynders]

Hi there, I haven't forgotten about this, just been swamped with work. I'll be pushing the node iteration later today normally. However, ATM I'm working on providing toString methods for all expressions, since I really need this for a project. Anyway, there's a problem with the arguments of a CallExpression. It returns an ArrayExpression instance, but this is incorrect as you can see in the Parser API it should be a plain Array. This is a problem, since I wanted to add the toString method to the Base, which seems to be working for all expressions, but chokes on the arguments of a callexpression, since it's not a "real" ArrayExpression. Ummm... hope that makes sense.

Is it ok I take a really close look at the entire codebase; which possibly might lead to some substantial changes?

[SBoudrias]

Hey @creynders, I'm okay with substantial changes, but try to keep them organized in their commits so they're easy to read through. That'll allow easier review and faster merge.

Of course, feel free to open issue to discuss some topic that could be separated - or just some changes you're not sure about.

[creynders]

So, thinking about all of this some more and digging through the code, I'm leaning towards a more versatile API (I hope), something like this: https://gist.github.com/creynders/a7e057922f20c7d188ea

[SBoudrias]

That mostly looks good to me, but how would you manage it when there's multiple nodes matching a query?

And I'm not sure about auto detecting the type of node that is queried. That seems edgy to me as you won't know if grunt.foo is suppose to be a method call or an assignment.

[creynders]

how would you manage it when there's multiple nodes matching a query?

That's already there: nodes(1).arguments(0); Would take the first argument of the second query result.

That seems edgy to me as you won't know if grunt.foo is suppose to be a method call or an assignment.

I'm not sure I get what you mean. As a user of ast-query you won't know what kind of result you're getting? As long as the results are properly typed, it's not really a problem IMO. The main benefit is that it's a lot more flexible: you can do reference counting, etc.
To me, the main query function should be as loose as possible, with a myriad of ways of getting results: using regex, deep object referencing, ... Then I'd provide a number of targeted functions: callExpression, assignment, ... to filter by type both for type safety and speed reasons. I.e. if you know exactly what you want you use the "specific" API, if you need a more loose approach you use the query function. This is really useful in reporting and static code analysis of circular dependencies for instance.

[SBoudrias]

I wasn't very clear about the multiple nodes. I meant more how it'd be handle in the case of toString.

BTW, I added you as collaborator on the repo.

[creynders]

I meant more how it'd be handle in the case of toString

That depends, I guess. As I see it now, which might change soon enough, there's basically the query function, the different type "classes" and some sort of collection "class" for the nodes, which will iterate through the collection of nodes to call methods directly e.g. toString

I meant more how it'd be handle in the case of toString

Wow, thanks. I'll create a new dedicated branch to explore where a possibly new API will take us.