fix: rust compile errors #869

YushiOMOTE · 2022-09-03T08:20:10Z

Description

Locally tried Rust generator with more complicated private examples and got some compile errors in the generate Rust crate.
The PR makes compile pass fixing the following issues:

Remove warnings in case the model name contains the dash + number (e.g. Abc_123)
Compile errors due to mismatch of some Option field definitions and constructors in struct
Fix broken generated file names
Fix invalid type name included in generated Union types
Fix the missing qualifer crate:: in some cases
Fix undefined error when schemaUid doesn't exist

The normal toPascalCase method separates number e.g. `abc_123` is converted to `Abc_123`. This causes compile warnings in Rust. The new method converts it to `Abc123`.

github-actions

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

leigh-johnson · 2022-09-05T04:01:29Z

👋 Hey @YushiOMOTE, thank you for the contributions! Would you feel comfortable adding tests to demonstrate how the code breaks, and verify your fixes? Feel free to ping me if you're having trouble getting the tests (unit or black-box) running locally.

An excellent first step would be to add a test for renderMacro revisions to: https://github.com/asyncapi/modelina/blob/next/test/generators/rust/RustGenerator.spec.ts

Can you share a schema with example data structures? That will make it easier to grok the behavior you're describing. Thanks!

The ConstrainedUnionModel isn't used because we use ConstrainedEnumModel to describe an enum of primitive type(s) and mixed/complex types ("union" in most languages).

Regarding imports, the decision to render unit types and use fully-qualified crate:: syntax was intentional. We follow this convention in the openapi-generator for data structures and fn signatures.
https://github.com/OpenAPITools/openapi-generator/blob/60dcf8613f6b75b606757c4a991fe018ad10ee99/samples/client/petstore/rust/reqwest/petstore/src/models/pet.rs#L26
https://github.com/OpenAPITools/openapi-generator/blob/master/samples/client/petstore/rust/reqwest/petstore/src/apis/pet_api.rs#L86

leigh-johnson · 2022-09-05T04:02:44Z

link against #847 for ref

YushiOMOTE · 2022-09-05T12:30:52Z

Thank you! Yes let me add tests. I cannot share the exact schema as it’s closed, but probably can share example schemas that can reproduce the issues.

There was mismatch of conditions for struct constructor and definition.

`modelName` contains `CargoToml` or `lib` instead of `Cargo.toml` and `lib.rs`. Fixed to use `model.name` that contains the actual file names.

The `type` field of union contains the type name with suffix number, which is non-existent type name. The actual name is stored in `ref` side.

Follow the actual Rust spec to generate PartialEq, PartialOrd, Eq, Ord derives.

YushiOMOTE · 2022-09-06T11:31:00Z

examples/rust-generate-crate/__snapshots__/index.spec.ts.snap

@@ -50,7 +50,7 @@ impl Address {
 exports[`Should be able to render Rust Models and should log expected output to console 2`] = `
 Array [
  "// Members represents a union of types: String, f64, bool
-#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
+#[derive(Clone, Debug, Deserialize, PartialEq, PartialOrd, Serialize)]


Expected as f64 supports PartialOrd

YushiOMOTE · 2022-09-06T12:38:57Z

Updated/added tests. npm test passes locally

Test Suites: 148 passed, 148 total
Tests:       3 skipped, 977 passed, 980 total
Snapshots:   199 passed, 199 total
Time:        40.065 s
Ran all test suites.

Regarding imports, the decision to render unit types and use fully-qualified crate:: syntax was intentional. We follow this convention in the openapi-generator for data structures and fn signatures.

I have changed to use fully-qualified syntax instead of importing.

Can you share a schema with example data structures? That will make it easier to grok the behavior you're describing.

I think the updated snapshots in this PR describes most of the issues I faced. Except for this one:

Fix invalid type name included in generated Union types (78c44a2)

I'm trying to give the example swagger or add a test in PR to describe this case soon 👀

YushiOMOTE · 2022-09-06T11:32:24Z

test/generators/rust/__snapshots__/RustGenerator.spec.ts.snap

 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)]
-pub enum Things {
+pub enum Things123 {


Things_123 in schema is converted to Things123 now. Previously, it was Things_123 which caused compile warnings.

YushiOMOTE · 2022-09-06T11:33:13Z

test/generators/rust/presets/__snapshots__/CommonPreset.spec.ts.snap

@@ -2,18 +2,18 @@

 exports[`RUST_COMMON_PRESET Enum should render \`enum\` of union type with Default implementation of integer type 1`] = `
 "// Address represents a Address model.
-#[derive(Clone, Debug, Deserialize, Eq, Ord, PartialEq, PartialOrd, Serialize)]


Ord and PartialOrd are removed now as HashMap doesn't support those.

YushiOMOTE · 2022-09-06T11:34:30Z

test/generators/rust/presets/__snapshots__/CommonPreset.spec.ts.snap

 pub struct Address {
    #[serde(rename=\\"members\\")]
-    members: Box<crate::Members>,
+    members: crate::Members,


Expected. Since we take Members in fn new side without Box, the definition should be just Members as well.

I think we should keep references to models as pointer-heap memory by default. If you need the entire data structure to be loaded into stack memory, I think a preset would be the right mechanism for that behavior.

Got it! I fixed to use Box for reference. Now the constructor side is fixed to use Box: https://github.com/asyncapi/modelina/pull/869/files#diff-9f4739c9c2692512b77fc209d671487d26937caed76f432ff6adf62a343a3ebaR18

YushiOMOTE · 2022-09-06T11:37:14Z

test/generators/rust/presets/__snapshots__/CommonPreset.spec.ts.snap

    #[serde(rename=\\"optional_members\\", skip_serializing_if = \\"Option::is_none\\")]
    optional_members: Option<Box<crate::OptionalMembers>>,
    #[serde(rename=\\"additionalProperties\\", skip_serializing_if = \\"Option::is_none\\")]
    additional_properties: Option<std::collections::HashMap<String, String>>,
 }

 impl Address {
-    pub fn new(members: Members, optional_members: Option<crate::OptionalMembers>, additional_properties: Option<std::collections::HashMap<String, String>>) -> Address {
+    pub fn new(members: crate::Members, optional_members: Option<crate::OptionalMembers>, additional_properties: Option<std::collections::HashMap<String, String>>) -> Address {


crate:: is needed as Members and Address are defined in separate files when we use RustFileGenerator

src/processors/AsyncAPIInputProcessor.ts

sonarcloud · 2022-09-11T04:42:42Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
0.0% Duplication

YushiOMOTE · 2022-09-11T05:45:59Z

Fix invalid type name included in generated Union types > (78c44a2)

I'm trying to give the example swagger or add a test in PR to describe this case soon 👀

This spec generates the issue with union.
YushiOMOTE@6f47a2e

YushiOMOTE · 2022-09-11T04:52:40Z

test/generators/rust/presets/__snapshots__/CommonPreset.spec.ts.snap

+#[derive(Clone, Debug, Deserialize, Serialize)]
+pub enum ReservedUnion {
+    #[serde(rename=\\"Student0\\")]
+    Student0(crate::Student),


This snapshot is for the issue fixed by this revision

Before the fix, the line became Student0(crate::Student0), which caused compile error.

leigh-johnson · 2022-09-14T23:26:01Z

Sorry for the delay in responding! Thank you for sticking with this PR @YushiOMOTE 🙇‍♀️

The failing black-box tests are related to #847 , so don't worry about those.
https://github.com/asyncapi/modelina/actions/runs/3030534665/jobs/4877197283

Linter caught a couple of nits:
https://github.com/asyncapi/modelina/actions/runs/3030534665/jobs/4877197283

jonaslagoni · 2022-09-27T08:48:54Z

Cant wait for this to be merged 😄 Just encountered the same issues 👍

leigh-johnson

👋 I'd love to merge this @YushiOMOTE, just a few suggestions to pass the linter (added in review).

src/generators/rust/RustConstrainer.ts

src/generators/rust/presets/CommonPreset.ts

src/generators/rust/renderers/StructRenderer.ts

src/generators/rust/renderers/UnionRenderer.ts

src/helpers/FormatHelpers.ts

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

sonarcloud · 2022-11-05T03:50:49Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
0.0% Duplication

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

sonarcloud · 2022-11-05T03:51:06Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
0.0% Duplication

sonarcloud · 2022-11-05T03:51:48Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

leigh-johnson · 2022-11-07T01:37:08Z

@YushiOMOTE Looks like the last thing left is to update this snapshot 🎉 👏
https://github.com/asyncapi/modelina/actions/runs/3398564376/jobs/5652579555

leigh-johnson · 2022-11-16T18:04:16Z

@YushiOMOTE Just a heads up, I'm going to merge this PR into a new branch, cleanup-pr-869, and do the last bit of cleanup. Thank you for submitting these revisions!

jonaslagoni · 2022-11-16T19:57:35Z

@all-contributors
please add @YushiOMOTE for bug, code.
please add @leigh-johnson for review.

allcontributors · 2022-11-16T20:00:17Z

@jonaslagoni

I've put up a pull request to add @YushiOMOTE! 🎉

I've put up a pull request to add @leigh-johnson! 🎉

Add a new method to create pascal case merging numbers

c6523aa

The normal toPascalCase method separates number e.g. `abc_123` is converted to `Abc_123`. This causes compile warnings in Rust. The new method converts it to `Abc123`.

github-actions bot reviewed Sep 3, 2022

View reviewed changes

YushiOMOTE changed the title ~~Fix Rust compile errors~~ fix: Rust compile errors Sep 3, 2022

YushiOMOTE changed the title ~~fix: Rust compile errors~~ fix: rust compile errors Sep 3, 2022

YushiOMOTE marked this pull request as ready for review September 3, 2022 08:26

YushiOMOTE requested review from leigh-johnson, magicmatatjahu, jonaslagoni and asyncapi-bot-eve as code owners September 3, 2022 08:26

YushiOMOTE force-pushed the fix-rust-issues branch 2 times, most recently from 51e1ca3 to 1ab80a6 Compare September 6, 2022 11:19

YushiOMOTE added 9 commits September 6, 2022 20:27

Use toPascalCaseMergingNumbers to avoid compile warnings

d5d13f6

Match Rust struct constructor with definition

66c6539

There was mismatch of conditions for struct constructor and definition.

Fix Rust file names generated

ef2408a

`modelName` contains `CargoToml` or `lib` instead of `Cargo.toml` and `lib.rs`. Fixed to use `model.name` that contains the actual file names.

Fix Rust union item type names

78c44a2

The `type` field of union contains the type name with suffix number, which is non-existent type name. The actual name is stored in `ref` side.

Fix the condition to generate some derives

43dd94c

Follow the actual Rust spec to generate PartialEq, PartialOrd, Eq, Ord derives.

Fix undefined error when schemaUid isn't set

f0502fc

Fix renderMacro tests

e1762a4

Update snapshots

fe91c8f

Generate crate:: prefix for all the reference types

301cb29

YushiOMOTE force-pushed the fix-rust-issues branch from 1ab80a6 to 301cb29 Compare September 6, 2022 11:27

YushiOMOTE commented Sep 6, 2022

View reviewed changes

leigh-johnson reviewed Sep 9, 2022

View reviewed changes

src/processors/AsyncAPIInputProcessor.ts Show resolved Hide resolved

YushiOMOTE added 2 commits September 11, 2022 12:40

Add test to render reserved union

ee45773

Keep Box for references instead fix constructor

462c9bc

YushiOMOTE commented Sep 11, 2022

View reviewed changes

jonaslagoni mentioned this pull request Sep 27, 2022

feat: adds support for oneOf and anyOf as UnionModel #899

Merged

leigh-johnson requested changes Nov 4, 2022

View reviewed changes

YushiOMOTE and others added 9 commits November 5, 2022 12:49

Update src/generators/rust/RustConstrainer.ts

c5d22e8

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

Update src/generators/rust/RustConstrainer.ts

ab977a5

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

Update src/generators/rust/RustConstrainer.ts

c247c2a

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

Update src/generators/rust/RustConstrainer.ts

2eb4b44

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

Update src/generators/rust/presets/CommonPreset.ts

42e02e3

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

Update src/generators/rust/RustConstrainer.ts

8925f9c

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

Update src/generators/rust/presets/CommonPreset.ts

d58937c

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

Update src/helpers/FormatHelpers.ts

e677c45

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

Update src/generators/rust/renderers/UnionRenderer.ts

02aa8c4

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

Update src/generators/rust/renderers/StructRenderer.ts

15829a1

Co-authored-by: Leigh Johnson <hi@leighjohnson.me>

leigh-johnson changed the base branch from next to cleanup-pr-869 November 16, 2022 18:02

leigh-johnson merged commit 446ab17 into asyncapi:cleanup-pr-869 Nov 16, 2022

leigh-johnson mentioned this pull request Nov 16, 2022

fix: rust compile errors on enum members containing digits #994

Merged

This was referenced Nov 16, 2022

chore: add YushiOMOTE as a contributor for bug, and code #996

Merged

chore: add leigh-johnson as a contributor for review #997

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: rust compile errors #869

fix: rust compile errors #869

YushiOMOTE commented Sep 3, 2022 •

edited

Loading

github-actions bot left a comment

leigh-johnson commented Sep 5, 2022

leigh-johnson commented Sep 5, 2022

YushiOMOTE commented Sep 5, 2022

YushiOMOTE Sep 6, 2022

YushiOMOTE commented Sep 6, 2022

YushiOMOTE Sep 6, 2022

YushiOMOTE Sep 6, 2022

YushiOMOTE Sep 6, 2022

leigh-johnson Sep 9, 2022

YushiOMOTE Sep 11, 2022

YushiOMOTE Sep 6, 2022

sonarcloud bot commented Sep 11, 2022

YushiOMOTE commented Sep 11, 2022

YushiOMOTE Sep 11, 2022

leigh-johnson commented Sep 14, 2022 •

edited

Loading

jonaslagoni commented Sep 27, 2022

leigh-johnson left a comment

sonarcloud bot commented Nov 5, 2022

sonarcloud bot commented Nov 5, 2022

sonarcloud bot commented Nov 5, 2022

leigh-johnson commented Nov 7, 2022

leigh-johnson commented Nov 16, 2022

jonaslagoni commented Nov 16, 2022

allcontributors bot commented Nov 16, 2022

fix: rust compile errors #869

fix: rust compile errors #869

Conversation

YushiOMOTE commented Sep 3, 2022 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

leigh-johnson commented Sep 5, 2022

leigh-johnson commented Sep 5, 2022

YushiOMOTE commented Sep 5, 2022

Choose a reason for hiding this comment

YushiOMOTE commented Sep 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 11, 2022

YushiOMOTE commented Sep 11, 2022

Choose a reason for hiding this comment

leigh-johnson commented Sep 14, 2022 • edited Loading

jonaslagoni commented Sep 27, 2022

leigh-johnson left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Nov 5, 2022

sonarcloud bot commented Nov 5, 2022

sonarcloud bot commented Nov 5, 2022

leigh-johnson commented Nov 7, 2022

leigh-johnson commented Nov 16, 2022

jonaslagoni commented Nov 16, 2022

allcontributors bot commented Nov 16, 2022

YushiOMOTE commented Sep 3, 2022 •

edited

Loading

leigh-johnson commented Sep 14, 2022 •

edited

Loading