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
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
e163e57
integrate scale-typegen, remove types mod
tadeohepperle Nov 9, 2023
1760792
reintroduce default substitutes and derives
tadeohepperle Nov 10, 2023
b297fc8
support runtime_types only again
tadeohepperle Nov 13, 2023
7a2678a
generating polkadot.rs ok
tadeohepperle Nov 13, 2023
60c1e62
Merge branch 'master' into tadeohepperle/implement-scale-typegen-for-…
tadeohepperle Nov 13, 2023
297d2c6
update scale-typegen to discrete error types
tadeohepperle Nov 15, 2023
f47fd4e
scale-typegen-api-changes
tadeohepperle Nov 21, 2023
03591a6
add note about UncheckedExtrinsic in default substitutes
tadeohepperle Nov 21, 2023
9e56546
Merge branch 'master' into tadeohepperle/implement-scale-typegen-for-…
tadeohepperle Nov 21, 2023
50ed54e
add resursive attributes and derives
tadeohepperle Nov 28, 2023
dabc88c
adjust example where Clone bound recursive
tadeohepperle Nov 28, 2023
cd73d31
Merge branch 'master' into tadeohepperle/implement-scale-typegen-for-…
tadeohepperle Nov 29, 2023
fd718bf
move scale-typegen dependency to workspace
tadeohepperle Nov 29, 2023
985a46c
expose default typegen settings
tadeohepperle Nov 29, 2023
a6b580f
lightclient: Fix wasm socket closure called after being dropped (#1289)
lexnv Nov 29, 2023
44c42c3
workflows: Install rustup component for building substrate (#1295)
lexnv Nov 29, 2023
0724735
cli: Command to fetch chainSpec and optimise its size (#1278)
lexnv Nov 29, 2023
ee1e096
conflicts
tadeohepperle Nov 29, 2023
58b8fee
remove comments and unused args
tadeohepperle Nov 30, 2023
0c7d373
Update substrate- and signer-related dependencies (#1297)
tadeohepperle Nov 30, 2023
42643d8
fix lock file
tadeohepperle Nov 30, 2023
8196b63
Merge branch 'master' into tadeohepperle/implement-scale-typegen-for-…
tadeohepperle Nov 30, 2023
bbda16a
fix lock file again :|
tadeohepperle Nov 30, 2023
4259572
adjust to new interface in scale-typegen
tadeohepperle Dec 8, 2023
14775ab
use released scale typegen
tadeohepperle Jan 2, 2024
69af6d1
reintroduce type aliases
tadeohepperle Jan 2, 2024
f125b2b
introduce type aliases again using scale-typegen
tadeohepperle Jan 2, 2024
602d459
cargo fmt and clippy
tadeohepperle Jan 2, 2024
b18d472
Merge branch 'master' into tadeohepperle/implement-scale-typegen-for-…
tadeohepperle Jan 3, 2024
aaf9ded
reconcile changes with master branch
tadeohepperle Jan 3, 2024
05dbe3f
update polkadot.rs
tadeohepperle Jan 3, 2024
e54cb73
bump scale-typgen to fix substitution
tadeohepperle Jan 5, 2024
3a2a2a0
subxt macro, helpful error messages
tadeohepperle Jan 5, 2024
24cb3d7
Merge branch 'master' into tadeohepperle/implement-scale-typegen-for-…
tadeohepperle Jan 5, 2024
e6ed42f
Merge branch 'tadeohepperle/implement-scale-typegen-for-type-generati…
tadeohepperle Jan 5, 2024
58071f5
adjust ui tests
tadeohepperle Jan 8, 2024
a8d59a8
Merge branch 'master' into tadeohepperle/macro-better-error-messages
tadeohepperle Jan 8, 2024
cac64ee
Merge branch 'master' into tadeohepperle/macro-better-error-messages
tadeohepperle Jan 9, 2024
5ade376
fix lock file
tadeohepperle Jan 9, 2024
ad1fdd7
format
tadeohepperle Jan 10, 2024
50eeb0e
Merge branch 'master' into tadeohepperle/macro-better-error-messages
tadeohepperle Jan 19, 2024
c676c4f
Update macro/src/lib.rs
tadeohepperle Jan 19, 2024
96c0a38
incorperate nits
tadeohepperle Jan 22, 2024
56c43cd
Merge branch 'master' into tadeohepperle/macro-better-error-messages
tadeohepperle Jan 22, 2024
011e4b0
update Cargo.lock to avoid compatibility issues
tadeohepperle Jan 22, 2024
b884f64
Merge branch 'master' into tadeohepperle/macro-better-error-messages
tadeohepperle Jan 22, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion macro/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 })

quote = { workspace = true }
subxt-codegen = { workspace = true, features = ["fetch-metadata"] }
scale-typegen = { workspace = true }
107 changes: 78 additions & 29 deletions macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,16 @@ use codec::Decode;
use darling::{ast::NestedMeta, FromMeta};
use proc_macro::TokenStream;
use proc_macro_error::{abort_call_site, proc_macro_error};
use quote::ToTokens;
use scale_typegen::typegen::{
settings::substitutes::path_segments,
validation::{registry_contains_type_path, similar_type_paths_in_registry},
};
use subxt_codegen::{
fetch_metadata::{
fetch_metadata_from_file_blocking, fetch_metadata_from_url_blocking, MetadataVersion, Url,
},
CodegenBuilder, CodegenError,
CodegenBuilder, CodegenError, Metadata,
};
use syn::{parse_macro_input, punctuated::Punctuated};

Expand Down Expand Up @@ -83,17 +88,21 @@ struct SubstituteType {
#[proc_macro_attribute]
#[proc_macro_error]
pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
let attr_args = match NestedMeta::parse_meta_list(args.into()) {
Ok(v) => v,
Err(e) => {
return TokenStream::from(darling::Error::from(e).write_errors());
}
};
let item_mod = parse_macro_input!(input as syn::ItemMod);
let args = match RuntimeMetadataArgs::from_list(&attr_args) {
Ok(v) => v,
Err(e) => return TokenStream::from(e.write_errors()),
};
match _subxt(args, parse_macro_input!(input as syn::ItemMod)) {
Ok(e) => e,
Err(e) => e,
}
}

// 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 = ..)

let attr_args = NestedMeta::parse_meta_list(args.into())
.map_err(|e| TokenStream::from(darling::Error::from(e).write_errors()))?;
let args = RuntimeMetadataArgs::from_list(&attr_args)
.map_err(|e| TokenStream::from(e.write_errors()))?;

// Fetch metadata first, because we need it to validate some of the chosen codegen options.
let metadata = fetch_metadata(&args)?;

let mut codegen = CodegenBuilder::new();

Expand Down Expand Up @@ -127,6 +136,7 @@ pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
.collect(),
);
for d in args.derive_for_type {
validate_type_path(&d.path.path, &metadata);
codegen.add_derives_for_type(d.path, d.derive.into_iter(), d.recursive);
}

Expand All @@ -139,20 +149,65 @@ pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
.collect(),
);
for d in args.attributes_for_type {
validate_type_path(&d.path.path, &metadata);
codegen.add_attributes_for_type(d.path, d.attributes.into_iter().map(|a| a.0), d.recursive)
}

// Insert type substitutions:
for sub in args.substitute_type.into_iter() {
validate_type_path(&sub.path, &metadata);
codegen.set_type_substitute(sub.path, sub.with);
}

let code = codegen
.generate(metadata)
.map_err(|e| e.into_compile_error())?;

Ok(code.into())
}

/// Checks that a type is present in the type registry. If it is not found, abort with a
/// helpful error message, showing the user alternative types, that have the same name, but are at different locations in the metadata.
fn validate_type_path(path: &syn::Path, metadata: &Metadata) {
let path_segments = path_segments(path);
let ident = &path
.segments
.last()
.expect("Empty path should be filtered out before already")
.ident;
if !registry_contains_type_path(metadata.types(), &path_segments) {
let alternatives = similar_type_paths_in_registry(metadata.types(), path);
let alternatives: String = if alternatives.is_empty() {
format!("There is no Type with name `{ident}` in the provided metadata.")
} else {
let mut s = "A type with the same name is present at: ".to_owned();
for p in alternatives {
s.push('\n');
s.push_str(&pretty_path(&p));
}
s
};

abort_call_site!(
"Type `{}` does not exist at path `{}`\n\n{}",
ident.to_string(),
pretty_path(path),
alternatives
);
}

fn pretty_path(path: &syn::Path) -> String {
path.to_token_stream().to_string().replace(' ', "")
}
}

/// 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

fn fetch_metadata(args: &RuntimeMetadataArgs) -> Result<subxt_codegen::Metadata, TokenStream> {
// Do we want to fetch unstable metadata? This only works if fetching from a URL.
let unstable_metadata = args.unstable_metadata.is_present();

match (
args.runtime_metadata_path,
args.runtime_metadata_insecure_url,
let metadata = match (
&args.runtime_metadata_path,
&args.runtime_metadata_insecure_url,
) {
(Some(rest_of_path), None) => {
if unstable_metadata {
Expand All @@ -164,16 +219,12 @@ pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
let root = std::env::var("CARGO_MANIFEST_DIR").unwrap_or_else(|_| ".".into());
let root_path = std::path::Path::new(&root);
let path = root_path.join(rest_of_path);
let generated_code = fetch_metadata_from_file_blocking(&path)
.map_err(CodegenError::from)
fetch_metadata_from_file_blocking(&path)
.and_then(|b| subxt_codegen::Metadata::decode(&mut &*b).map_err(Into::into))
.and_then(|m| codegen.generate(m).map_err(Into::into))
.unwrap_or_else(|e| e.into_compile_error());

generated_code.into()
.map_err(|e| CodegenError::from(e).into_compile_error())?
}
(None, Some(url_string)) => {
let url = Url::parse(&url_string).unwrap_or_else(|_| {
let url = Url::parse(url_string).unwrap_or_else(|_| {
abort_call_site!("Cannot download metadata; invalid url: {}", url_string)
});

Expand All @@ -182,13 +233,10 @@ pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
false => MetadataVersion::Latest,
};

let generated_code = fetch_metadata_from_url_blocking(url, version)
fetch_metadata_from_url_blocking(url, version)
.map_err(CodegenError::from)
.and_then(|b| subxt_codegen::Metadata::decode(&mut &*b).map_err(Into::into))
.and_then(|m| codegen.generate(m).map_err(Into::into))
.unwrap_or_else(|e| e.into_compile_error());

generated_code.into()
.map_err(|e| e.into_compile_error())?
}
(None, None) => {
abort_call_site!(
Expand All @@ -200,5 +248,6 @@ pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
"Only one of 'runtime_metadata_path' or 'runtime_metadata_insecure_url' can be provided"
)
}
}
};
Ok(metadata)
}
10 changes: 10 additions & 0 deletions testing/ui-tests/src/incorrect/substitute_at_wrong_path.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#[subxt::subxt(
runtime_metadata_path = "../../../../artifacts/polkadot_metadata_small.scale",
substitute_type(
path = "sp_runtime::multiaddress::Event",
with = "crate::MyEvent"
)
)]
pub mod node_runtime {}

fn main() {}
18 changes: 18 additions & 0 deletions testing/ui-tests/src/incorrect/substitute_at_wrong_path.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: Type `Event` does not exist at path `sp_runtime::multiaddress::Event`

A type with the same name is present at:
frame_system::pallet::Event
pallet_balances::pallet::Event
pallet_multisig::pallet::Event
--> src/incorrect/substitute_at_wrong_path.rs:1:1
|
1 | / #[subxt::subxt(
2 | | runtime_metadata_path = "../../../../artifacts/polkadot_metadata_small.scale",
3 | | substitute_type(
4 | | path = "sp_runtime::multiaddress::Event",
5 | | with = "crate::MyEvent"
6 | | )
7 | | )]
| |__^
|
= note: this error originates in the attribute macro `subxt::subxt` (in Nightly builds, run with -Z macro-backtrace for more info)
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#[subxt::subxt(
runtime_metadata_path = "../../../../artifacts/polkadot_metadata_tiny.scale",
runtime_metadata_path = "../../../../artifacts/polkadot_metadata_small.scale",
substitute_type(
path = "sp_arithmetic::per_things::Perbill",
with = "sp_runtime::Perbill"
path = "frame_support::dispatch::DispatchInfo",
with = "my_mod::DispatchInfo"
)
)]
pub mod node_runtime {}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: Type Generation failed: Type substitution error: `substitute_type(with = <path>)` must be a path prefixed with 'crate::' or '::'
--> src/incorrect/substitute_path_not_absolute.rs:5:16
|
5 | with = "sp_runtime::Perbill"
| ^^^^^^^^^^^^^^^^^^^^^
5 | with = "my_mod::DispatchInfo"
| ^^^^^^^^^^^^^^^^^^^^^^
Loading