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

GenericType resolution inspecificity #903

Closed
LockedThread opened this issue May 5, 2024 · 3 comments · Fixed by #904
Closed

GenericType resolution inspecificity #903

LockedThread opened this issue May 5, 2024 · 3 comments · Fixed by #904

Comments

@LockedThread
Copy link
Contributor

Recently, I was trying to integrate utoipa into my project and ran into an issue. See below for an example.

Example

Code

#[derive(Deserialize, Serialize, ToSchema)]
pub struct Location {
    pub map: Map,
}

#[derive(Deserialize, Serialize, ToSchema)]
pub struct Map {
  pub id: u32
}

Error

error: proc-macro derive panicked
  --> apps/api/src/http/models.rs:73:41
   |
73 | #[derive(Debug, Deserialize, Serialize, ToSchema)]
   |                                         ^^^^^^^^
   |
   = help: message: ComponentSchema Map type should have children

This error occurs because of the get_generic_type function using only the name of the type, not the actual type. See here:

fn get_generic_type(segment: &PathSegment) -> Option<GenericType> {
.

The most simple solution that would work for this specific edge case is to ensure that the segment passed into the get_generic_type function is actually for a generic type. However, if my Map struct was a generic type that fix would not work.

I looked into how we could do import introspection, but there are a lot of edge cases to consider when implementing that. This is my first time using the syn library, so I don't feel confident implementing that portion by myself.

If anyone has a better solution please feel free to contribute. In the mean time I am making a PR with my proposed half-baked solution.

@juhaku
Copy link
Owner

juhaku commented May 5, 2024

I don't have any better options for the fix. I believe that this is good enough fix just to check whether the type actually has arguments. Though just came to my mind that it would probably be good to check whether there actually is type arguments, e.g. ignoring the lifetimes and consts, but that can be another PR if there is need to. For now it can be as is IMO.

@LockedThread
Copy link
Contributor Author

I don't have any better options for the fix. I believe that this is good enough fix just to check whether the type actually has arguments. Though just came to my mind that it would probably be good to check whether there actually is type arguments, e.g. ignoring the lifetimes and consts, but that can be another PR if there is need to. For now it can be as is IMO.

Alright, sounds good. Do you want to keep this open for tracking future changes or do you think my PR solved it?

@juhaku
Copy link
Owner

juhaku commented May 5, 2024

Alright, sounds good. Do you want to keep this open for tracking future changes or do you think my PR solved it

This can be closed. In future if there is a need, one can open a new issue. (Need to get the list of open issues down quite a bit 😆 )

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 a pull request may close this issue.

2 participants