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

Macro error messages validating metadata and making suggestions #1339

Merged
merged 46 commits into from
Jan 23, 2024

Conversation

tadeohepperle
Copy link
Contributor

Fixes #1269

Branched from #1260 so #1260 should be merged first.

This PR introduces checks to all user defined type paths that are passed to the subxt macro. This could be a custom derive, attribute or a substitute for a type. The macro now makes sure that the specified path exists in the type registry. If this is not fulfilled it errors and gives the user a list of valid paths that belong to a Type with the same name. Here is an example:

image

tadeohepperle and others added 30 commits November 9, 2023 18:21
* lightclient: Close wasm socket while dropping from connecting state

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Construct one time only closures

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* testing: Enable console logs for lightclient WASM testing

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Separate wakes and check connectivity on poll_read

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Close the socket depending on internal state

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* Revert "lightclient: Separate wakes and check connectivity on poll_read"

This reverts commit 8660940.

* lightclient: Return pending if socket is opening from poll_read

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Close the socket on `poll_close`

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Reset closures on Drop to avoid recursive invokation

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Close the socket if not already closing

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
* cli: Add chainSpec command

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli/chainSpec: Move to dedicated module

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Compute the state root hash

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Remove code substitutes

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* artifacts: Update polkadot.json

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* scripts: Generate the chain spec

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Remove testing artifacts

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Fix clippy

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Apply rustfmt

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Introduce feature flag for smoldot dependency

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Rename chain-spec to chain-spec-pruning

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* scripts: Update chain-spec command

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
* update crypto dependencies, adjust keypair

* add scale_info::TypeInfo derive in some places

* add multi signature derive
@tadeohepperle tadeohepperle requested a review from a team as a code owner January 5, 2024 15:18
@tadeohepperle tadeohepperle changed the title Tadeohepperle/macro better error messages Macro error messages validating metadata and given user suggestions Jan 8, 2024
@tadeohepperle tadeohepperle changed the title Macro error messages validating metadata and given user suggestions Macro error messages validating metadata and making suggestions Jan 8, 2024
macro/src/lib.rs Outdated
}
}

/// Fetches metadata in a blocking manner, either from a url (not recommended) or from a file path.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why fetching metadata from an URL is not recommended? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not recommended for users because the compilation blocks while fetching the metadata, which is a bit slow and (maybe flaky) compared to using metadata file. But yeah, maybe the docs of this function don't need to mention this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can say its not recommended for production use-cases. Do remember we added the ability to fetch the metadata from URL as a feature request from the cargo-contract team, that wanted to fetch a fresh metadata for their testing

macro/src/lib.rs Outdated Show resolved Hide resolved
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
macro/src/lib.rs Outdated
}

// Note: just an additional function to make early returns easier.
fn _subxt(args: TokenStream, item_mod: syn::ItemMod) -> Result<TokenStream, TokenStream> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: or subxt_inner; the underscore leads me to wonder if we are going to use the function (similar to let _unused = ..)

macro/Cargo.toml Outdated
@@ -23,5 +23,7 @@ proc-macro = true
codec = { package = "parity-scale-codec", workspace = true }
darling = { workspace = true }
proc-macro-error = { workspace = true }
syn = { workspace = true }
syn.workspace = true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do wonder, did this happen as a result of some linting extensions from your IDE?

We could revert this to keep consistency with the dependencies of this file (syn = { workspace = true })

Copy link
Collaborator

@lexnv lexnv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! 👍

Just some changes wrt the documentation, and importing the dependencies with a different format

@tadeohepperle tadeohepperle merged commit 388ebac into master Jan 23, 2024
11 checks passed
@tadeohepperle tadeohepperle deleted the tadeohepperle/macro-better-error-messages branch January 23, 2024 10:44
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.

codegen: Compile error propagation for deriving at invalid paths
3 participants