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

Validate Codegen Settings #11

Merged
merged 39 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
874e795
change interface
tadeohepperle Nov 21, 2023
ca78ad5
recursive derives
tadeohepperle Nov 22, 2023
4fe27fa
added test and comments
tadeohepperle Nov 22, 2023
8f3af3b
Merge branch 'master' into recursive-derives
tadeohepperle Nov 22, 2023
ce60070
readd test
tadeohepperle Nov 22, 2023
390cd15
formatting adjustment
tadeohepperle Nov 23, 2023
fcfb267
fix example and unused dependencies
tadeohepperle Nov 27, 2023
ffa5ea3
add documentation to everything.
tadeohepperle Nov 27, 2023
8ae1090
fix bugs in generic recursion of container types
tadeohepperle Nov 28, 2023
83bda09
clippy fix
tadeohepperle Nov 28, 2023
66302dc
fix dependencies
tadeohepperle Nov 28, 2023
f7fff3b
remove the generated rs code
tadeohepperle Nov 28, 2023
1be27b3
make scale_typegen directly accessible
tadeohepperle Nov 29, 2023
7ff03ad
Debug and Clone derives on relevant structs
tadeohepperle Nov 29, 2023
dcdeef5
fix comments in code
tadeohepperle Nov 29, 2023
4cf3e7b
add compact encoding differentiation
tadeohepperle Nov 29, 2023
361fa5b
fix pipeline error by hiding scale-typegen in description behind flag
tadeohepperle Nov 30, 2023
ddda6e0
add access to type registry
tadeohepperle Nov 30, 2023
9163658
give middleware access to transformer to resolve types
tadeohepperle Dec 4, 2023
619529c
nit: fix docs on recurse_policy
tadeohepperle Dec 4, 2023
89e4de0
add struct prefix, fix variant space bug
tadeohepperle Dec 5, 2023
1798db5
add cargo machete in CI/CD
tadeohepperle Dec 5, 2023
68cbc09
remove #![deny(unused_crate_dependencies)] in description crate
tadeohepperle Dec 5, 2023
68e2857
introduce validation, currently broken tests
tadeohepperle Dec 6, 2023
f9bad26
fix syn path issues
tadeohepperle Dec 7, 2023
f052257
add test for type suggestions
tadeohepperle Dec 7, 2023
8904669
introduce cache hit policy
tadeohepperle Dec 7, 2023
119bf73
fine grained control over cache hit behavior
tadeohepperle Dec 7, 2023
cfec37d
remove should_continue and other things
tadeohepperle Dec 7, 2023
588b7a9
Merge branch 'recursive-derives' into tadeohepperle/validate-codegen
tadeohepperle Dec 7, 2023
e0a4e68
remove smallvec
tadeohepperle Dec 7, 2023
47fae43
remove smallvec
tadeohepperle Dec 7, 2023
55f6464
generic arguments are working much better now
tadeohepperle Dec 7, 2023
ed27633
Merge branch 'recursive-derives' into tadeohepperle/validate-codegen
tadeohepperle Dec 7, 2023
be6186a
fix test
tadeohepperle Dec 7, 2023
0f283b0
merge
tadeohepperle Dec 8, 2023
2921dcd
restructure code
tadeohepperle Dec 8, 2023
1233641
validation error display
tadeohepperle Dec 8, 2023
08f60d9
remove panics from docs
tadeohepperle Dec 8, 2023
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
59 changes: 40 additions & 19 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ prettyplease = "0.2.15"
scale-decode = "0.9.0"
scale-encode = "0.5.0"
frame-metadata = { version = "16.0.0", default-features = false, features = ["current", "std"] }
bitvec = { version = "1", default-features = false }
bitvec = { version = "1", default-features = false, features = ["alloc"] }
pretty_assertions = "1.4.0"
anyhow = "1.0.75"
peekmore = "1.3.0"
Expand Down
118 changes: 117 additions & 1 deletion typegen/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ use syn::parse_quote;

use crate::{
tests::utils::{subxt_settings, Testgen},
typegen::settings::TypeGeneratorSettings,
typegen::{
error::SettingsValidationError,
settings::TypeGeneratorSettings,
validation::{
similar_type_paths_in_registry, validate_substitutes_and_derives_against_registry,
},
},
utils::ensure_unique_type_paths,
DerivesRegistry, TypeSubstitutes,
};
Expand Down Expand Up @@ -1260,3 +1266,113 @@ fn ensure_unique_type_paths_test() {
]
);
}

#[test]
fn validation_errors() {
#[allow(unused)]
#[derive(TypeInfo)]
struct S;

let registry = Testgen::new().with::<S>().into_portable_registry();
let mut settings = TypeGeneratorSettings::new();
settings.derives = {
let mut d = DerivesRegistry::new();
d.add_derives_for(
parse_quote!(scale_typegen::tests::S),
[parse_quote!(Clone), parse_quote!(Reflect)],
true,
);

d.add_attributes_for(
parse_quote!(scale_typegen::tests::S),
[parse_quote!(#[nice])],
true,
);

d.add_derives_for(
parse_quote!(scale_typegen::tests::T),
[parse_quote!(Clone), parse_quote!(Reflect)],
true,
);

d.add_attributes_for(
parse_quote!(scale_typegen::tests::T),
[parse_quote!(#[nice])],
true,
);
d
};
settings = settings
.substitute(
parse_quote!(scale_typegen::tests::S),
parse_quote!(::hello::S),
)
.substitute(
parse_quote!(scale_typegen::tests::T),
parse_quote!(::hello::T),
);

let err = validate_substitutes_and_derives_against_registry(
&settings.substitutes,
&settings.derives,
&registry,
)
.unwrap_err();

// we expect only errors for type T because S is in the registry, T is not.
let expected_err = SettingsValidationError {
derives_for_unknown_types: vec![(
parse_quote!(scale_typegen::tests::T),
[parse_quote!(Clone), parse_quote!(Reflect)].into(),
)],
attributes_for_unknown_types: vec![(
parse_quote!(scale_typegen::tests::T),
[parse_quote!(#[nice])].into(),
)],
substitutes_for_unknown_types: vec![(
parse_quote!(scale_typegen::tests::T),
parse_quote!(::hello::T),
)],
};

assert_eq!(err, expected_err);
}

#[test]
fn find_similar_type_paths() {
#[allow(unused)]
#[derive(TypeInfo)]
struct S;
mod types {
#[allow(unused)]
#[derive(scale_info::TypeInfo)]
pub struct T;

#[allow(unused)]
#[derive(scale_info::TypeInfo)]
pub struct S;

pub mod abc {
#[allow(unused)]
#[derive(scale_info::TypeInfo)]
pub struct S;
}
}

let registry = Testgen::new()
.with::<S>()
.with::<types::S>()
.with::<types::T>()
.with::<types::abc::S>()
.into_portable_registry();

// user gives the wrong type location types::xyz::S => show them suggestions for correct paths.
let similar = similar_type_paths_in_registry(&registry, &parse_quote!(types::xyz::S));

let expected: Vec<syn::Path> = vec![
parse_quote!(scale_typegen::tests::S),
parse_quote!(scale_typegen::tests::types::S),
parse_quote!(scale_typegen::tests::types::abc::S),
];
assert_eq!(similar, expected);
}
83 changes: 83 additions & 0 deletions typegen/src/typegen/error.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::collections::HashSet;

use proc_macro2::Span;
use quote::ToTokens;

/// Error for when something went wrong during type generation.
#[derive(Debug, thiserror::Error)]
Expand All @@ -25,6 +28,9 @@ pub enum TypegenError {
/// Type substitution error.
#[error("Type substitution error: {0}")]
InvalidSubstitute(#[from] TypeSubstitutionError),
/// The settings do not fit the given type registry.
#[error("Settings do not fit the given type registry: {0}")]
SettingsValidation(SettingsValidationError),
}

/// Error attempting to do type substitution.
Expand Down Expand Up @@ -68,3 +74,80 @@ pub enum TypeSubstitutionErrorKind {
#[error("Cannot find matching param on 'from' type.")]
NoMatchingFromType,
}

/// Error attempting to do type substitution.
#[derive(Debug, thiserror::Error, Default, PartialEq, Eq)]
pub struct SettingsValidationError {
/// Cannot add derive for type that is not present in the registry.
pub derives_for_unknown_types: Vec<(syn::Path, HashSet<syn::Path>)>,
/// Cannot add attribute for type that is not present in the registry.
pub attributes_for_unknown_types: Vec<(syn::Path, HashSet<syn::Attribute>)>,
/// Invalid path to replace: the type getting substituted is not present in the registry.
pub substitutes_for_unknown_types: Vec<(syn::Path, syn::Path)>,
}

impl std::fmt::Display for SettingsValidationError {
Copy link

Choose a reason for hiding this comment

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

nit; It would be interesting to see if we could provide alternative types. Since we cannot find type a::b::Ty, we could inspect the type registry for Ty and return all valid paths.

Which for an xcm MultiLocation it would return both v2 and v3 entries, which might make things easier during development. Tho, this could probably be a follow-up

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a feeling that Subxt won't be able to use the function that produces this error, so it may well be that it's not that useful for us actually, and instead we'll lean on those other validation.rs functions, one of which does provide a list of similar paths

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the similar_type_paths_in_registry function can do that, but I have not put it into the SettingsValidationError, to not bloat it too much. We should definitely do this though from the subxt macro side.

fn fmt(
&self,
f: &mut scale_info::prelude::fmt::Formatter<'_>,
) -> scale_info::prelude::fmt::Result {
fn display_one(e: &impl ToTokens) -> String {
e.to_token_stream().to_string().replace(' ', "")
}

fn display_many(set: &HashSet<impl ToTokens>) -> String {
set.iter()
.map(|e| display_one(e))
.collect::<Vec<_>>()
.join(", ")
}

writeln!(f, "Settings validation error:")?;

if !self.derives_for_unknown_types.is_empty() {
writeln!(f, " Derives for unknown types:")?;
for (path, derives) in &self.derives_for_unknown_types {
writeln!(
f,
" {} (Derives: {})",
display_one(path),
display_many(derives)
)?;
}
}

if !self.attributes_for_unknown_types.is_empty() {
writeln!(f, " Attributes for unknown types:")?;
for (path, attributes) in &self.attributes_for_unknown_types {
writeln!(
f,
" {} (Attributes: {})",
display_one(path),
display_many(attributes)
)?;
}
}

if !self.substitutes_for_unknown_types.is_empty() {
writeln!(f, " Substitutes for unknown types:")?;
for (original, substitute) in &self.substitutes_for_unknown_types {
writeln!(
f,
" {} (Substitute: {})",
display_one(original),
display_one(substitute),
)?;
}
}

Ok(())
}
}

impl SettingsValidationError {
pub(crate) fn is_empty(&self) -> bool {
self.derives_for_unknown_types.is_empty()
&& self.attributes_for_unknown_types.is_empty()
&& self.substitutes_for_unknown_types.is_empty()
}
}
Loading
Loading