Skip to content

Commit

Permalink
CLI: Return error on wrongly specified type paths (#1397)
Browse files Browse the repository at this point in the history
* add user errors and test cases

* Update cli/src/commands/codegen.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

---------

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
  • Loading branch information
tadeohepperle and niklasad1 authored Jan 26, 2024
1 parent 8dc62fc commit 4408a8c
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 5 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ codec = { package = "parity-scale-codec", workspace = true }
scale-info = { workspace = true }
scale-value = { workspace = true }
syn = { workspace = true }
quote = { workspace = true }
jsonrpsee = { workspace = true, features = ["async-client", "client-ws-transport-native-tls", "http-client"] }
tokio = { workspace = true, features = ["rt-multi-thread"] }
scale-typegen-description = { workspace = true }
Expand Down
99 changes: 95 additions & 4 deletions cli/src/commands/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ use crate::utils::{validate_url_security, FileOrUrl};
use clap::Parser as ClapParser;
use codec::Decode;
use color_eyre::eyre::eyre;
use scale_typegen_description::scale_typegen::typegen::{
settings::substitutes::path_segments,
validation::{registry_contains_type_path, similar_type_paths_in_registry},
};
use subxt_codegen::CodegenBuilder;
use subxt_metadata::Metadata;

/// Generate runtime API client code from metadata.
///
Expand Down Expand Up @@ -205,6 +210,9 @@ fn codegen(
codegen.no_docs()
}

let metadata = subxt_metadata::Metadata::decode(&mut &*metadata_bytes)
.map_err(|e| eyre!("Cannot decode the provided metadata: {e}"))?;

// Configure derives:
let global_derives = raw_derives
.iter()
Expand All @@ -219,6 +227,8 @@ fn codegen(
.map_err(|e| eyre!("Cannot parse derive for type {ty_str}: {e}"))?;
let derive = syn::parse_str(&d.trait_path)
.map_err(|e| eyre!("Cannot parse derive for type {ty_str}: {e}"))?;

validate_path_with_metadata(&ty.path, &metadata)?;
// Note: recursive derives and attributes not supported in the CLI => recursive: false
codegen.add_derives_for_type(ty, std::iter::once(derive), d.recursive);
}
Expand All @@ -234,10 +244,12 @@ fn codegen(

for a in attributes_for_type {
let ty_str = &a.type_path;
let ty = syn::parse_str(ty_str)
let ty: syn::TypePath = syn::parse_str(ty_str)
.map_err(|e| eyre!("Cannot parse attribute for type {ty_str}: {e}"))?;
let attribute: OuterAttribute = syn::parse_str(&a.attribute)
.map_err(|e| eyre!("Cannot parse attribute for type {ty_str}: {e}"))?;

validate_path_with_metadata(&ty.path, &metadata)?;
// Note: recursive derives and attributes not supported in the CLI => recursive: false
codegen.add_attributes_for_type(ty, std::iter::once(attribute.0), a.recursive);
}
Expand All @@ -248,11 +260,11 @@ fn codegen(
.map_err(|e| eyre!("Cannot parse type substitution for path {from_str}: {e}"))?;
let to: syn::Path = syn::parse_str(&to_str)
.map_err(|e| eyre!("Cannot parse type substitution for path {from_str}: {e}"))?;

validate_path_with_metadata(&from, &metadata)?;
codegen.set_type_substitute(from, to);
}

let metadata = subxt_metadata::Metadata::decode(&mut &*metadata_bytes)
.map_err(|e| eyre!("Cannot decode the provided metadata: {e}"))?;
let code = codegen
.generate(metadata)
.map_err(|e| eyre!("Cannot generate code: {e}"))?;
Expand All @@ -261,12 +273,50 @@ fn codegen(
Ok(())
}

/// Validates that the type path is part of the metadata.
fn validate_path_with_metadata(path: &syn::Path, metadata: &Metadata) -> color_eyre::Result<()> {
fn pretty_path(path: &syn::Path) -> String {
use quote::ToTokens;
path.to_token_stream().to_string().replace(' ', "")
}

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
};

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

Ok(())
}

#[cfg(test)]
mod tests {
use crate::commands::codegen::type_map_parser;

#[test]
fn parse_types() {
use crate::commands::codegen::type_map_parser;

assert_eq!(type_map_parser("Foo"), None);
assert_eq!(type_map_parser("Foo=Bar"), Some(("Foo", "Bar", false)));
assert_eq!(
Expand All @@ -276,4 +326,45 @@ mod tests {
assert_eq!(type_map_parser("Foo=Bar,a"), None);
assert_eq!(type_map_parser("Foo=Bar,a,b,c,recursive"), None);
}

async fn run(args_str: &str) -> color_eyre::Result<String> {
let mut args = vec![
"codegen",
"--file=../artifacts/polkadot_metadata_small.scale",
];
args.extend(args_str.split(' ').filter(|e| !e.is_empty()));
let opts: super::Opts = clap::Parser::try_parse_from(args)?;
let mut output: Vec<u8> = Vec::new();
let r = super::run(opts, &mut output)
.await
.map(|_| String::from_utf8(output).unwrap())?;
Ok(r)
}

#[tokio::test]
async fn invalid_type_paths() {
let valid_type = "sp_runtime::multiaddress::MultiAddress";
let invalid_type = "my_module::MultiAddress";

let valid_cases = [
format!("--derive-for-type {valid_type}=serde::Serialize"),
format!("--attributes-for-type {valid_type}=#[allow(clippy::all)]"),
format!("--substitute-type {valid_type}=::my_crate::MultiAddress"),
];
for case in valid_cases.iter() {
let output = run(case).await;
assert!(output.is_ok());
}

let invalid_cases = [
format!("--derive-for-type {invalid_type}=serde::Serialize"),
format!("--attributes-for-type {invalid_type}=#[allow(clippy::all)]"),
format!("--substitute-type {invalid_type}=my_module::MultiAddress"),
];
for case in invalid_cases.iter() {
let output = run(case).await;
// assert that we make suggestions pointing the user to the valid type
assert!(output.unwrap_err().to_string().contains(valid_type));
}
}
}
2 changes: 1 addition & 1 deletion macro/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ proc-macro = true
codec = { package = "parity-scale-codec", workspace = true }
darling = { workspace = true }
proc-macro-error = { workspace = true }
syn.workspace = { workspace = true }
syn = { workspace = true }
quote = { workspace = true }
subxt-codegen = { workspace = true, features = ["fetch-metadata"] }
scale-typegen = { workspace = true }

0 comments on commit 4408a8c

Please sign in to comment.