-
Notifications
You must be signed in to change notification settings - Fork 63
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
Objects fields named Result generate code that (generally) won't compile #568
Labels
compile-fail
generated code doesn't compile
Comments
Thanks for the bug report. It sounds like we should fully qualify our use of |
anelson
added a commit
to anelson/typify
that referenced
this issue
Nov 30, 2024
I ran into oxidecomputer#568 when trying to use typify to generate Rust bindings for the [Model Context Protocol](https://spec.modelcontextprotocol.io). The Anthropic have published a (JSON Schema specification for this protocol)[https://github.com/modelcontextprotocol/specification/blob/main/schema/schema.json], but as soon as I tried to codegen Rust bindings for it, I ran into the aforementioned issue. I added the entire MCP JSON schema to `typify/tests/schemas/model-context-protocol.json`, which as expected immediately caused the `schemas` test to fail. Fixing this was a relatively simple matter of finding every place in the codegen that referred to the Rust `std::result::Result` type as simply `Result`. That was enough to make the MCP schema work right. But then I looked a bit closer and realized there were some other Rust built-in types that were being referenced in such a way as to cause either errors during codegen or generation of code that doesn't compile. So I made a diabolical test schema `rust-collisions.json` that is truly cursed. It defines a bunch of types that collide with Rust built-in types and keywords, and made the `schemas` test explode with such force that there may have been inquiries from the neighbors. Upon reassuring them that that's just how the Rust compiler expresses affection, I made some more codegen changes to resolve all of the issues that my cursed schema exposed. I've tried to make the changes minimal, and to my knowledge none of the changes I have made would be breaking from the perspective of any user of the generated Rust code. I had to modify several existing tests in `typify-impl` that broke because they were expecting the non-canonicalized versions of Rust standard types. Pay particular attention to the hack I put in `test_util::validate_output_impl` which is not something I'm particularly proud of but I think it retains the spirit of the test. Closes oxidecomputer#568
anelson
added a commit
to anelson/typify
that referenced
this issue
Dec 2, 2024
I ran into oxidecomputer#568 when trying to use typify to generate Rust bindings for the [Model Context Protocol](https://spec.modelcontextprotocol.io). The Anthropic have published a (JSON Schema specification for this protocol)[https://github.com/modelcontextprotocol/specification/blob/main/schema/schema.json], but as soon as I tried to codegen Rust bindings for it, I ran into the aforementioned issue. I added the entire MCP JSON schema to `typify/tests/schemas/model-context-protocol.json`, which as expected immediately caused the `schemas` test to fail. Fixing this was a relatively simple matter of finding every place in the codegen that referred to the Rust `std::result::Result` type as simply `Result`. That was enough to make the MCP schema work right. But then I looked a bit closer and realized there were some other Rust built-in types that were being referenced in such a way as to cause either errors during codegen or generation of code that doesn't compile. So I made a diabolical test schema `rust-collisions.json` that is truly cursed. It defines a bunch of types that collide with Rust built-in types and keywords, and made the `schemas` test explode with such force that there may have been inquiries from the neighbors. Upon reassuring them that that's just how the Rust compiler expresses affection, I made some more codegen changes to resolve all of the issues that my cursed schema exposed. I've tried to make the changes minimal, and to my knowledge none of the changes I have made would be breaking from the perspective of any user of the generated Rust code. I had to modify several existing tests in `typify-impl` that broke because they were expecting the non-canonicalized versions of Rust standard types. Pay particular attention to the hack I put in `test_util::validate_output_impl` which is not something I'm particularly proud of but I think it retains the spirit of the test. Closes oxidecomputer#568
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Having a
Result
definition seems to create code that won't compile, ifstd::result::Result
is needed.Reproduction
A simplified schema that shows the issue (note: if you don't have the enum, or something that needs a result, it won't give the error):
Gives:
Solve
I think an explicit Result, like #458, should hopefully solve it.
(meanwhile, I will probably change my field name...)
Thanks!
The text was updated successfully, but these errors were encountered: