Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add path param to Transform::transform #10

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

freshtonic
Copy link
Contributor

Prior to this commit the only information about a node available to Transformer implementations was its type.

This made contextual transformations hard to implement. For example, transforming an Expr in a projection differently to an Expr in an ORDER BY clause.

The path parameter enables context-sensitive transformations of AST nodes by including the path from the root of the AST to the parent of the original_node.

Prior to this commit the only information about a node available to
`Transformer` implementations was its type.

This made contextual transformations hard to implement. For example,
transforming an `Expr` in a projection differently to an `Expr` in an
`ORDER BY` clause.

The context parameter enables context-sensitive transformations of AST
nodes by including the path from the root of the AST to the parent of
`original_node`.
@freshtonic freshtonic force-pushed the refactor/transformer-context branch from 7b99789 to cdf2e7c Compare March 20, 2025 03:49
@freshtonic freshtonic force-pushed the refactor/transformer-context branch from 01235fb to eb9174a Compare March 20, 2025 05:56
@freshtonic freshtonic force-pushed the refactor/transformer-context branch 7 times, most recently from b066999 to 6fe9f8c Compare April 4, 2025 04:48
`AsNodeKey` is the *only* way to contruct a `NodeKey` in a consuming
crate now.

`Semantic` was poorly thought out. It existed to prevent a `NodeKey` from
being created from a `Box<N: Visitable>` (because annotations should go on
the inner type).

It was really easy to forget to deref a `Box<N: Visitable>` when creating
a `NodeKey` - annotations end up on the Box instead of the N. Which is
annoying when you're trying to debug why annotations seem to be missing
later on.

So the impl of `AsNodeKey` for `Box<N: AsNodeKey>` dereferences to the
contained type.

AsNodeKey is implemented for `Option<N: 'static>` - returning a
`NodeKey` proxying the `Option` itself - because it might be `None`.
@freshtonic freshtonic force-pushed the refactor/transformer-context branch from 6fe9f8c to e16406f Compare April 4, 2025 04:52
Renamed `Context` -> `NodePath`.

Dropped `current_node` argument from `Transform::transform` and put
`node_path` before `new_node`.

It's nicer when contextual arguments come first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant