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 a Visitor implementation to Marker #159

Closed
xFrednet opened this issue Jun 28, 2023 · 1 comment · Fixed by #232
Closed

Add a Visitor implementation to Marker #159

xFrednet opened this issue Jun 28, 2023 · 1 comment · Fixed by #232
Labels
A-marker-utils Area: Utility functions usually provided in `marker_utils` or `marker_api` C-enhancement Category: New feature or request
Milestone

Comments

@xFrednet
Copy link
Member

The basic job of a Visitor, is to visit everything inside a given scope, in our case, a part of the AST. This definition still gives a lot of room for interpretation or better say flexibility.

The granularity of the visit can vary. I'm thinking of potentially supporting different types of granularity:

  1. Only visit items and nested items
  2. Visit items and expressions

A different factor is how bodies are handled. Bodies can contain further items and expressions. This can be relevant, depending on the goal of the visitor. Most lints in Clippy, only visit one body and don't resolve inner bodies.

Another thing to consider, is how the implementor code will be called. Rustc's implementation calls a trait Visitor while Clippy often uses a visitor calling a closure: for_each_expr.

This is way more complicated than I thought at first. This will be fun.


Once this is implementation, I would like to use the visitor in marker_adapter to ensure that everything is visited correctly (Currently it skips quite a few nodes 😅 )

@xFrednet xFrednet added C-enhancement Category: New feature or request A-marker-utils Area: Utility functions usually provided in `marker_utils` or `marker_api` labels Jun 28, 2023
@xFrednet xFrednet added this to the v0.1.0 milestone Jun 28, 2023
@xFrednet xFrednet self-assigned this Jun 28, 2023
bors bot added a commit that referenced this issue Jul 9, 2023
161: Utils: Setup `marker_utils` project and add a `Visitor` (First clean dogfood) r=xFrednet a=xFrednet

This PR is an important milestone. It adds the new `marker_utils` crate with a new `Visitor` trait. This trait is then directly used to traverse the entire AST in the `marker_adapter` crate. And finally, dogfood, or better say a clean dogfood. It just passes, able to translate everything. I don't quite know how to feel about this, tbh.

The number of changes sadly exploded, sorry if someone actually looks at this 😅

---

cc: #159 I think the design still needs some small adjustments, like being able to limit the scope of what is being visited etc. 


Co-authored-by: xFrednet <xFrednet@gmail.com>
@xFrednet xFrednet removed this from the v0.1.0 milestone Jul 16, 2023
@xFrednet xFrednet removed their assignment Jul 25, 2023
@xFrednet
Copy link
Member Author

xFrednet commented Jul 25, 2023

The basic implementation is done, but I would like to add more options, to allow the user to specify if bodies should be visited. This could be done with constant generics or a simple function in the Visitor trait.

It would also be nice to have a closure-based visitor, for simple use cases, like the one Clippy provides: clippy_utils::visitor::for_each_expr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-marker-utils Area: Utility functions usually provided in `marker_utils` or `marker_api` C-enhancement Category: New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant