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

AST: Allow jsonOptions to include Text field of Location struct #6213

Closed
anderseknert opened this issue Sep 6, 2023 · 3 comments · Fixed by #6234
Closed

AST: Allow jsonOptions to include Text field of Location struct #6213

anderseknert opened this issue Sep 6, 2023 · 3 comments · Fixed by #6234

Comments

@anderseknert
Copy link
Member

Similarly to what was previously done in order to include the location in the parsed AST, it would be useful for tools like Regal to be able to use the Text field of the Location struct. This is however currently lost in serialization.

If we could add a JSON option to have that optionally included in location data, it would be helpful to solve a certain class of problems in external tooling.

@charlieegan3
Copy link
Contributor

I've looked into this a bit this afternoon, we have an alias type Location in the ast package.

type Location = location.Location

This is the type that's used to for all the ast Term Location values, and forms the majority of Locations used in the ast Go API.

It aliases the location.Location type:

Row int `json:"row"` // The line in the source.

This is used more sparingly in ast & topdown.

I'm unsure of the history as to why we have the alias (#2219). However, it makes it harder to implement this feature as we can't add functions to the alias type required to alter the marshalling behaviour.

Open to other ideas 🙂

@charlieegan3
Copy link
Contributor

Update, #6234 should be a working fix for this issue. My comment above was based on a mistake I made in testing!

@anderseknert
Copy link
Member Author

Niiiiice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants