-
-
Notifications
You must be signed in to change notification settings - Fork 7k
[fix][elixir] wrong typespec generation for all-of with single ref #21139
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
[fix][elixir] wrong typespec generation for all-of with single ref #21139
Conversation
2ef197c
to
b4f9a26
Compare
b4f9a26
to
23e74d7
Compare
typeMapping.put("UUID", "String"); | ||
typeMapping.put("URI", "String"); | ||
// primitive types | ||
typeMapping.put("string", "String.t"); |
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.
Now, we are truly mapping to Elixir built-in types.
"PID", | ||
// This is a workaround, since the DefaultCodeGen uses our elixir TypeSpec | ||
// datetype to evaluate the primitive | ||
"integer()", |
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.
This is more idiomatic and allows us to better identify what schemas may need a custom model doing something along the lines of languageSpecificPrimitives.contains(type)
} else if (!StringUtils.isEmpty(p.get$ref())) { | ||
switch (super.getTypeDeclaration(p)) { | ||
case "String": |
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.
With better type mappings, code like this one (which was incomplete and should have also included other types such as Map
, etc.) is no longer necessary.
@@ -784,24 +762,6 @@ public ExtendedCodegenOperation(CodegenOperation o) { | |||
this.operationIdCamelCase = o.operationIdCamelCase; | |||
} | |||
|
|||
private void translateBaseType(StringBuilder returnEntry, String baseType) { |
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.
This method wasn't used anywhere in the generator.
@@ -180,14 +234,14 @@ defmodule OpenapiPetstore.Api.Fake do | |||
|
|||
- `connection` (OpenapiPetstore.Connection): Connection to server | |||
- `opts` (keyword): Optional parameters | |||
- `:body` (float()): Input number as post body | |||
- `:body` (number()): Input number as post body |
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.
number()
is more accurate since we are not specifying any format in the OAS. number()
captures whole numbers and floating point numbers (see here).
I think this is quite a milestone 😄
Now, we can use the Elixir generation to build non-trivial clients (yeah, 5,606 files including model and API files). The above is the output I got when running this version on Cloudflare's official OAS (see here). This wasn't possible a few weeks ago 😄 |
nice, we will try to get it merged before the v7.13.0 release due this week |
Description
This PR addresses the issue reported in #21135 - ie. an issue with the generated typespecs that would prevent some generated clients from compiling; but it also introduces the following changes:
All in all, this should contribute to make the Elixir generator easier to extend and more maintainable.
As usual, curious to hear your thoughts @mrmstn, @wing328 🙂
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master
(upcoming7.x.0
minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)