-
Notifications
You must be signed in to change notification settings - Fork 116
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
Bug: Flattening generic type also flattens all generic parameters #233
Merged
Merged
Changes from 2 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
495a6bf
add test
NyxCode 5daeed2
re-implement handeling of generics
NyxCode cab0908
Dependency::from_ty - remove generic params from name
NyxCode d764f5c
expand e2e test
NyxCode 29248b4
adjust failing test
NyxCode 4264c5f
fix optional features
NyxCode 83b9541
fix TS_RS_EXPORT_DIR
NyxCode 61c9692
add test for #70
NyxCode ebfa532
enable test for #214
NyxCode 129b323
remove TODO about race condition - we fixed that with a mutex.
NyxCode 0a68cef
remove TODO about #56 - already fixed
NyxCode 2b5b9a9
remove "limitations" section from readme - both #56 and #70 are fixed
NyxCode 5ab4aa9
Use type_params to simplify capture of Generics' identifiers
escritorio-gustavo 3e31a3a
Add ident method
escritorio-gustavo bae3df8
Add dummy type to allow exporting types that use ToString as a generi…
escritorio-gustavo a693b60
use DerivedTS::ts_name instead of rust_ty
escritorio-gustavo bdb06d4
use TS::ident
escritorio-gustavo b4f6b01
Prefer renaming Enum to using all variants
escritorio-gustavo 6d9a21f
Remove redundant clones
escritorio-gustavo 3354dc7
Remove unused Option
escritorio-gustavo 268742d
Fix inverted condition and separate the two checks
escritorio-gustavo 3606c48
Merge branch 'main' into bug/flatten-generic-enum
escritorio-gustavo 247276a
Remove redundant #[doc(hidden)]
escritorio-gustavo 01615fe
Replace if Some else None with bool::then
escritorio-gustavo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
#![macro_use] | ||
#![deny(unused)] | ||
//#![deny(unused)] | ||
|
||
use proc_macro2::{Ident, TokenStream}; | ||
use quote::{format_ident, quote}; | ||
|
@@ -9,6 +9,7 @@ use syn::{ | |
}; | ||
|
||
use crate::deps::Dependencies; | ||
use crate::utils::format_generics; | ||
|
||
#[macro_use] | ||
mod utils; | ||
|
@@ -17,10 +18,10 @@ mod deps; | |
mod types; | ||
|
||
struct DerivedTS { | ||
name: String, | ||
generics: Generics, | ||
ts_name: String, | ||
docs: String, | ||
inline: TokenStream, | ||
decl: TokenStream, | ||
inline_flattened: Option<TokenStream>, | ||
dependencies: Dependencies, | ||
|
||
|
@@ -29,29 +30,11 @@ struct DerivedTS { | |
} | ||
|
||
impl DerivedTS { | ||
fn generate_export_test(&self, rust_ty: &Ident, generics: &Generics) -> Option<TokenStream> { | ||
let test_fn = format_ident!("export_bindings_{}", &self.name.to_lowercase()); | ||
let generic_params = generics | ||
.params | ||
.iter() | ||
.filter(|param| matches!(param, GenericParam::Type(_))) | ||
.map(|_| quote! { () }); | ||
let ty = quote!(<#rust_ty<#(#generic_params),*> as ts_rs::TS>); | ||
|
||
Some(quote! { | ||
#[cfg(test)] | ||
#[test] | ||
fn #test_fn() { | ||
#ty::export().expect("could not export type"); | ||
} | ||
}) | ||
} | ||
|
||
fn into_impl(self, rust_ty: Ident, generics: Generics) -> TokenStream { | ||
fn into_impl(mut self, rust_ty: Ident, generics: Generics) -> TokenStream { | ||
let mut get_export_to = quote! {}; | ||
let export_to = match &self.export_to { | ||
Some(dirname) if dirname.ends_with('/') => { | ||
format!("{}{}.ts", dirname, self.name) | ||
format!("{}{}.ts", dirname, self.ts_name) | ||
} | ||
Some(filename) => filename.clone(), | ||
None => { | ||
|
@@ -60,7 +43,7 @@ impl DerivedTS { | |
ts_rs::__private::get_export_to_path::<Self>() | ||
} | ||
}; | ||
format!("bindings/{}.ts", self.name) | ||
format!("bindings/{}.ts", self.ts_name) | ||
} | ||
}; | ||
|
||
|
@@ -69,51 +52,26 @@ impl DerivedTS { | |
false => None, | ||
}; | ||
|
||
let DerivedTS { | ||
name, | ||
docs, | ||
inline, | ||
decl, | ||
inline_flattened, | ||
dependencies, | ||
.. | ||
} = self; | ||
|
||
let docs = match docs.is_empty() { | ||
true => None, | ||
false => { | ||
Some(quote!(const DOCS: Option<&'static str> = Some(#docs);)) | ||
} | ||
let docs = match &*self.docs { | ||
"" => None, | ||
docs => Some(quote!(const DOCS: Option<&'static str> = Some(#docs);)), | ||
}; | ||
|
||
let inline_flattened = inline_flattened | ||
.map(|t| { | ||
quote! { | ||
fn inline_flattened() -> String { | ||
#t | ||
} | ||
} | ||
}) | ||
.unwrap_or_else(TokenStream::new); | ||
let impl_start = generate_impl_block_header(&rust_ty, &generics); | ||
let name = self.generate_name_fn(); | ||
let inline = self.generate_inline_fn(); | ||
let decl = self.generate_decl_fn(&rust_ty); | ||
let dependencies = &self.dependencies; | ||
|
||
let impl_start = generate_impl(&rust_ty, &generics); | ||
quote! { | ||
#impl_start { | ||
const EXPORT_TO: Option<&'static str> = Some(#export_to); | ||
#get_export_to | ||
|
||
#get_export_to | ||
#docs | ||
|
||
fn decl() -> String { | ||
#decl | ||
} | ||
fn name() -> String { | ||
#name.to_owned() | ||
} | ||
fn inline() -> String { | ||
#inline | ||
} | ||
#inline_flattened | ||
#name | ||
#decl | ||
#inline | ||
|
||
#[allow(clippy::unused_unit)] | ||
fn dependency_types() -> impl ts_rs::typelist::TypeList | ||
|
@@ -131,10 +89,151 @@ impl DerivedTS { | |
#export | ||
} | ||
} | ||
|
||
/// Returns an expression which evaluates to the TypeScript name of the type, including generic | ||
/// parameters. | ||
fn name_with_generics(&self) -> TokenStream { | ||
let name = &self.ts_name; | ||
let mut generics_ts_names = self | ||
.generics | ||
.params | ||
.iter() | ||
.filter_map(|g| match g { | ||
GenericParam::Lifetime(_) => None, | ||
GenericParam::Type(ty) => Some(&ty.ident), | ||
GenericParam::Const(_) => None, | ||
}) | ||
.map(|generic| quote!(<#generic as ts_rs::TS>::name())) | ||
.peekable(); | ||
|
||
if generics_ts_names.peek().is_some() { | ||
quote! { | ||
format!("{}<{}>", #name, vec![#(#generics_ts_names),*].join(", ")) | ||
} | ||
} else { | ||
quote!(#name.to_owned()) | ||
} | ||
} | ||
|
||
/// Generate a dummy unit struct for every generic type parameter of this type. | ||
/// Example: | ||
/// ```ignore | ||
/// struct Generic<A, B, const C: usize> { /* ... */ } | ||
/// ``` | ||
/// has two generic type parameters, `A` and `B`. This function will therefor generate | ||
/// ```ignore | ||
/// struct A; | ||
/// impl ts_rs::TS for A { /* .. */ } | ||
/// | ||
/// struct B; | ||
/// impl ts_rs::TS for B { /* .. */ } | ||
/// ``` | ||
fn generate_generic_types(&self) -> TokenStream { | ||
let generics = self.generics.params.iter().filter_map(|g| match g { | ||
GenericParam::Lifetime(_) => None, | ||
GenericParam::Type(t) => Some(t.ident.clone()), | ||
GenericParam::Const(_) => None, | ||
}); | ||
|
||
quote! { | ||
#( | ||
#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)] | ||
struct #generics; | ||
impl std::fmt::Display for #generics { | ||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
write!(f, "{:?}", self) | ||
} | ||
} | ||
impl TS for #generics { | ||
fn name() -> String { stringify!(#generics).to_owned() } | ||
fn transparent() -> bool { false } | ||
} | ||
)* | ||
} | ||
} | ||
|
||
fn generate_export_test(&self, rust_ty: &Ident, generics: &Generics) -> Option<TokenStream> { | ||
let test_fn = format_ident!("export_bindings_{}", rust_ty.to_string().to_lowercase()); | ||
let generic_params = generics | ||
.params | ||
.iter() | ||
.filter(|param| matches!(param, GenericParam::Type(_))) | ||
.map(|_| quote! { () }); | ||
NyxCode marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let ty = quote!(<#rust_ty<#(#generic_params),*> as ts_rs::TS>); | ||
|
||
Some(quote! { | ||
#[cfg(test)] | ||
#[test] | ||
fn #test_fn() { | ||
#ty::export().expect("could not export type"); | ||
} | ||
}) | ||
} | ||
|
||
fn generate_name_fn(&self) -> TokenStream { | ||
let name = self.name_with_generics(); | ||
quote! { | ||
fn name() -> String { | ||
#name | ||
} | ||
} | ||
} | ||
|
||
fn generate_inline_fn(&self) -> TokenStream { | ||
let inline = &self.inline; | ||
|
||
let inline_flattened = self.inline_flattened.as_ref().map(|inline_flattened| { | ||
quote! { | ||
fn inline_flattened() -> String { | ||
#inline_flattened | ||
} | ||
} | ||
}); | ||
let inline = quote! { | ||
fn inline() -> String { | ||
#inline | ||
} | ||
}; | ||
quote! { | ||
#inline | ||
#inline_flattened | ||
} | ||
} | ||
|
||
/// Generates the `decl()` and `decl_concrete()` methods. | ||
/// `decl_concrete()` is simple, and simply defers to `inline()`. | ||
/// For `decl()`, however, we need to change out the generic parameters of the type, replacing | ||
/// them with the dummy types generated by `generate_generic_types()`. | ||
fn generate_decl_fn(&mut self, rust_ty: &Ident) -> TokenStream { | ||
let name = &self.ts_name; | ||
let generic_types = self.generate_generic_types(); | ||
let ts_generics = format_generics(&mut self.dependencies, &self.generics); | ||
// These are the generic parameters we'll be using. | ||
let generic_idents = self.generics.params.iter().filter_map(|p| match p { | ||
GenericParam::Lifetime(_) => None, | ||
// Since we named our dummy types the same as the generic parameters, we can just keep | ||
// the identifier of the generic parameter - its name is shadowed by the dummy struct. | ||
GenericParam::Type(TypeParam { ident, .. }) => Some(quote!(#ident)), | ||
// We keep const parameters as they are, since there's no sensible default value we can | ||
// use instead. This might be something to change in the future. | ||
GenericParam::Const(ConstParam { ident, .. }) => Some(quote!(#ident)), | ||
}); | ||
quote! { | ||
fn decl_concrete() -> String { | ||
format!("type {} = {};", #name, Self::inline()) | ||
} | ||
fn decl() -> String { | ||
#generic_types | ||
let inline = <#rust_ty<#(#generic_idents,)*> as ts_rs::TS>::inline(); | ||
let generics = #ts_generics; | ||
format!("type {}{generics} = {inline};", #name) | ||
} | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And finally, |
||
|
||
// generate start of the `impl TS for #ty` block, up to (excluding) the open brace | ||
fn generate_impl(ty: &Ident, generics: &Generics) -> TokenStream { | ||
fn generate_impl_block_header(ty: &Ident, generics: &Generics) -> TokenStream { | ||
use GenericParam::*; | ||
|
||
let bounds = generics.params.iter().map(|param| match param { | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, these dummy types are generated, named the same as the generic parameters.
I thought about only defining one dummy type, so e.g
#[docs(hidden)] pub struct Dummy<const NAME: &'static str>
.However, const-generics don't work with strings yet. Maybe something for the future to improve compile times.