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

Check all rust keywords, deal with recursive structs #15

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pscott31
Copy link

I was using this to generate an API for Square; and they had a api method called 'break' which upset the generator; so I've added in a crate that purports to have an exhaustive list.

Secondly, they have calls where youve got something like

struct Item{
   name: String
   related_item: Option<Item>
}

Which was causing the example generator to try and make an infinitely nested example. I had a go at refactoring to_rust_example_value to be iterative instead of recursive but gave up and instead pass around a set identifiers that we've already used in the tree to that point and put a None there instead of Some<Item>.

Might be nice to allow a couple one or two levels perhaps, we could change the HashSet into a HashMap with a depth - but thought I'd keep it simple.

@kurtbuilds
Copy link
Owner

kurtbuilds commented Dec 14, 2023

Can you add a regression test to this, or at least a minimal real-world OpenAPI spec that'd have such recursion. I'm not sure how you'd encounter this in an OpenAPI spec in the real world.

struct Item{
   name: String
   related_item: Option<Item>
}

is an illegal struct for the same reason (it'd have to be a Option<Box<Item>> or a Vec`).

The check_keyword is a great find, happy to merge that separately or when above question is addressed.

EDIT: Nevermind, saw that you brought this up via Issue #12

@kurtbuilds
Copy link
Owner

kurtbuilds commented Dec 14, 2023

Would you be able to trim down the vwphere spec to the offending endpoint or struct to facilitate adding a regression test? I think it's from ClusterComputeResourcePlaceVmRequest, but I'm not sure.

ceejbot added a commit to ceejbot/libninja that referenced this pull request Apr 4, 2024
This is the right way to do it.
ceejbot added a commit to ceejbot/libninja that referenced this pull request Apr 4, 2024
This is the right way to do it.
ceejbot added a commit to ceejbot/libninja that referenced this pull request Aug 6, 2024
This is the right way to do it.
ceejbot added a commit to ceejbot/libninja that referenced this pull request Oct 3, 2024
This is the right way to do it.
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 this pull request may close these issues.

2 participants