Conversation
modify extend type signature to accept variable # of extendee tables
| - upwards navigation is rly useful; in this codemod, it'd be a lot easier / more performant to query for each function call (generally), update all those that are migrated accordingly (if we have upwards nav | ||
| we can get the statement by walking up tree from the call node). unfortunately, bc some of the migrations require the whole call statement (i.e local var = call), | ||
| we cannot do this and have to query for statements for those transforms that require the whole statement context, and separately query for calls for those transforms that only need the call node |
| - I'm not sure of a good way around this one, just a general performance concern, it's tricky to vary operations between node world / string world. | ||
| When operating on a node, it is not uncommon to want to re-use parts of that node in your transformed output; currently, this will require: | ||
| - for string transform: parsing the node into a string, and interpolating into your returned replacement | ||
| - for node transform: cloning node so it's mutable, updating accordingly (even more impractical, would never do this, just pointing out you CAN) |
There was a problem hiding this comment.
I think we should be biasing towards string transforms. Most codemods are going to be one-and-done, not multi-pass.
There was a problem hiding this comment.
I agree, writing the string transforms was much nicer for this. I guess my point here is that i'm curious if there's a better way to keep parts of existing nodes than using printer
…to e2e-query-tests
| end | ||
| end) | ||
| end | ||
| end) |
There was a problem hiding this comment.
Bumping this: I think you want fs.walk here.
This definitely looks like this can be abstracted as well.
hgoldstein
left a comment
hgoldstein
left a comment
There was a problem hiding this comment.
See comments, and I believe we should be using all lower case for the path names, so e2ecases and apisurfacechange instead of e2eCases and apiSurfaceChange respectively (yes I see there's a folder that's currently violating that).
tests/std/syntax/e2equery.test.luau
Outdated
| end) | ||
| end | ||
|
|
||
| local transformsDir = "./tests/std/syntax/e2eCases" |
There was a problem hiding this comment.
This feels a bit manual: is there any way for us to get this programmatically?
There was a problem hiding this comment.
Easiest way I know of is with the debug API but not sure if that's the best way. Do you know of any other solutions?
3 participants
No description provided.