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

Consider removing support for paths in abigen! in favour of Rust's include_str! #94

Closed
mitchmindtree opened this issue Feb 10, 2022 · 6 comments
Labels
abigen tech-debt Improves code quality or safety

Comments

@mitchmindtree
Copy link
Contributor

mitchmindtree commented Feb 10, 2022

We might be able to simplify the implementation of abigen! a little and add a little flexibility by only accepting expressions that evaluate to a str in the right-hand side argument.

For example, as an alternative to:

abigen!(MyContract, "./my-contract-abi.json");

which involves some custom path and file handling, we could potentially accept an Expr in the right-hand side argument (instead of LitStr) and offload the path and file-handling work to the Rust include_str macro from std:

abigen!(MyContract, include_str!("./my-contract-abi.json"));

Currently, even though we technically accept full contract ABI JSON strings in the right argument, we are unable to use macros that evaluate to a str like above as the parser for the right hand side argument expects a LitStr (string literal) rather than an Expr (expression). By taking an Expr, I believe users would be free to use both literal strings and macros that expand to a str, e.g. include_str, or an alternative to include_str that accepted absolute paths rather than relative, etc.


Motivation

I ran into this while moving an example out of The Sway Book into a standalone project within the sway repo examples directory as a part of solving FuelLabs/sway#544.

I noticed that currently the path accepted in the right-hand side of abigen! seems to be relative to the root of the current workspace (rather than the project). This makes the invocation of abigen! within the test harnesses of the examples that are nested within the sway repo a little awkward as they must specify the path relative to the sway workspace root, rather than the example's own project root. E.g. in examples/hello_world/tests/harness.rs currently we have to do

abigen!(MyContract, "./examples/hello_world/my-contract-abi.json");

rather than ideally

abigen!(MyContract, "./my-contract-abi.json");

or

abigen!(MyContract, include_str!("../my-contract-abi.json"));

Ideally we'd be able to write one of the latter as we'd like to include these examples into the book verbatim with

{{#include ../../../examples/hello_world/tests/harness.rs}}

Addressing this issue should allow for the latter.

@digorithm
Copy link
Member

Love this idea, let's do it!

@mitchmindtree
Copy link
Contributor Author

This may also solve #93 as a side effect.

@digorithm
Copy link
Member

True, include_str! should work on multiple platforms... Very nice side effect 😁

@JoshuaBatty
Copy link
Member

Just been taking a look into this issue with @mitchmindtree. Unfortunately, I don't think rust will let us expand an expression given to the second argument into a string at the time of procedural macro expansion. grrrrr shakes fist at rust

Perhaps another solution is to find the logic that include_str! is doing and copy that over as a function instead. I'll see if I can track down what code is actually being called internally.

@mitchmindtree
Copy link
Contributor Author

Yeah 🤦

Unfortunately it looks like we can't evaluate an expression given to the second argument to a string that we can actually use to generate the Contract type during proc macro expansion. I imagine that might be why abigen! is the way it is now!

There's some writing on this limitation on expanding macros in argument position of other macros here.

Barring that, I think @JoshuaBatty's suggestion of modelling our "path" source behaviour after include_str! is a good idea. I'll close this and open another dedicated issue.

@digorithm
Copy link
Member

Damn you Rust!

image

But yeah, fair enough, #97 sounds like a good option too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abigen tech-debt Improves code quality or safety
Projects
Archived in project
Development

No branches or pull requests

3 participants