From 2cb820fb85b4041ab35f62669d82b7659660bc5e Mon Sep 17 00:00:00 2001 From: Jonas Lagoni Date: Wed, 8 Mar 2023 11:38:11 +0100 Subject: [PATCH] fix!: additionalItems being applied for regular arrays (#1140) --- docs/migrations/version-1-to-2.md | 25 ++++++++++ .../__snapshots__/index.spec.ts.snap | 2 +- .../__snapshots__/index.spec.ts.snap | 4 +- src/helpers/CommonModelToMetaModel.ts | 25 ++++------ .../CSharpGenerator.spec.ts.snap | 4 +- .../go/__snapshots__/GoGenerator.spec.ts.snap | 2 +- .../__snapshots__/CommonPreset.spec.ts.snap | 50 +++++++++---------- .../rust/presets/CommonPreset.spec.ts | 3 +- .../__snapshots__/CommonPreset.spec.ts.snap | 18 +------ .../TypeScriptGenerator.spec.ts.snap | 14 +++--- test/helpers/CommonModelToMetaModel.spec.ts | 8 ++- 11 files changed, 82 insertions(+), 73 deletions(-) diff --git a/docs/migrations/version-1-to-2.md b/docs/migrations/version-1-to-2.md index 44c455b4ad..68a006847d 100644 --- a/docs/migrations/version-1-to-2.md +++ b/docs/migrations/version-1-to-2.md @@ -50,6 +50,31 @@ components: The _type_ property in the _CloudEvent_ schema will in this case have an _anonymous_schema_ id. If another schema in the _allOf_ list has the same property and an id other than _anonymous_schema_, it will now use that id. Meaning, in this example, it will be _DogType_ and _CatType_. +## Accurate array types + +For JSON Schema inputs (indirectly for AsyncAPI and OpenAPI), `additionalItems` was applied to regular array types, when it should only be applied for tuples. + +This means that a schema such as: +``` + "tags": { + "type": "array", + "items": { + "$ref": "http://asyncapi.com/definitions/2.6.0/tag.json" + }, + "additionalItems": true + }, +``` + +Would generate a type such as, in TypeScript: +``` +private _tags?: (Tag | any)[]; +``` + +Where it now generates: +``` +private _tags?: Tag[]; +``` + ## Creates union type for operation message oneOf In the example above, where `operation.message.oneOf` is set, Modelina will now generate a union type for it. Previously, Modelina ignored this union type, and only generated models for the content of `operation.message.oneOf`. In the example above, that meant models for `Dog` and `Cat`. Now, Modelina will generate a union type of `Pet` in addition to `Dog` and `Cat`. diff --git a/examples/csharp-auto-implemented-properties/__snapshots__/index.spec.ts.snap b/examples/csharp-auto-implemented-properties/__snapshots__/index.spec.ts.snap index 0ffc62e3a2..a6154eb8e7 100644 --- a/examples/csharp-auto-implemented-properties/__snapshots__/index.spec.ts.snap +++ b/examples/csharp-auto-implemented-properties/__snapshots__/index.spec.ts.snap @@ -4,7 +4,7 @@ exports[`Should be able to render auto-implemented properties in CSharp and shou Array [ "public class Root { - public dynamic[]? Emails { get; set; } + public string[]? Emails { get; set; } public string? StringProp { get; set; } public double? NumberProp { get; set; } }", diff --git a/examples/csharp-change-collection-type/__snapshots__/index.spec.ts.snap b/examples/csharp-change-collection-type/__snapshots__/index.spec.ts.snap index ad8bad831d..6072b1ae29 100644 --- a/examples/csharp-change-collection-type/__snapshots__/index.spec.ts.snap +++ b/examples/csharp-change-collection-type/__snapshots__/index.spec.ts.snap @@ -4,9 +4,9 @@ exports[`Should be able to render collections in C# as IEnumerable and should lo Array [ "public class Root { - private IEnumerable? email; + private IEnumerable? email; - public IEnumerable? Email + public IEnumerable? Email { get { return email; } set { email = value; } diff --git a/src/helpers/CommonModelToMetaModel.ts b/src/helpers/CommonModelToMetaModel.ts index 9b08baa446..662dae4e1a 100644 --- a/src/helpers/CommonModelToMetaModel.ts +++ b/src/helpers/CommonModelToMetaModel.ts @@ -445,15 +445,9 @@ export function convertToArrayModel( return undefined; } - const isNormalArray = - !Array.isArray(jsonSchemaModel.items) && - jsonSchemaModel.additionalItems === undefined && - jsonSchemaModel.items !== undefined; - //item multiple types + additionalItems sat = both count, as normal array - //item single type + additionalItems sat = contradicting, only items count, as normal array - //item not sat + additionalItems sat = anything is allowed, as normal array - //item single type + additionalItems not sat = normal array - //item not sat + additionalItems not sat = normal array, any type + const isNormalArray = !Array.isArray(jsonSchemaModel.items); + //items single type = normal array + //items not sat = normal array, any type if (isNormalArray) { const placeholderModel = new AnyModel('', undefined); const metaModel = new ArrayModel( @@ -462,12 +456,13 @@ export function convertToArrayModel( placeholderModel ); alreadySeenModels.set(jsonSchemaModel, metaModel); - - const valueModel = convertToMetaModel( - jsonSchemaModel.items as CommonModel, - alreadySeenModels - ); - metaModel.valueModel = valueModel; + if (jsonSchemaModel.items !== undefined) { + const valueModel = convertToMetaModel( + jsonSchemaModel.items as CommonModel, + alreadySeenModels + ); + metaModel.valueModel = valueModel; + } return metaModel; } diff --git a/test/generators/csharp/__snapshots__/CSharpGenerator.spec.ts.snap b/test/generators/csharp/__snapshots__/CSharpGenerator.spec.ts.snap index c24555fe87..ade4298869 100644 --- a/test/generators/csharp/__snapshots__/CSharpGenerator.spec.ts.snap +++ b/test/generators/csharp/__snapshots__/CSharpGenerator.spec.ts.snap @@ -42,7 +42,7 @@ exports[`CSharpGenerator should render \`class\` type 1`] = ` private bool? marriage; private dynamic? members; private dynamic[]? tupleType; - private dynamic[] arrayType; + private string[] arrayType; private Dictionary? additionalProperties; public string StreetName @@ -87,7 +87,7 @@ exports[`CSharpGenerator should render \`class\` type 1`] = ` set { tupleType = value; } } - public dynamic[] ArrayType + public string[] ArrayType { get { return arrayType; } set { arrayType = value; } diff --git a/test/generators/go/__snapshots__/GoGenerator.spec.ts.snap b/test/generators/go/__snapshots__/GoGenerator.spec.ts.snap index 636dbec177..71cdd2436c 100644 --- a/test/generators/go/__snapshots__/GoGenerator.spec.ts.snap +++ b/test/generators/go/__snapshots__/GoGenerator.spec.ts.snap @@ -103,7 +103,7 @@ type Address struct { Marriage bool Members interface{} TupleType []interface{} - ArrayType []interface{} + ArrayType []string AdditionalProperties map[string]interface{} }" `; diff --git a/test/generators/java/presets/__snapshots__/CommonPreset.spec.ts.snap b/test/generators/java/presets/__snapshots__/CommonPreset.spec.ts.snap index 229ca33199..19e686a2ea 100644 --- a/test/generators/java/presets/__snapshots__/CommonPreset.spec.ts.snap +++ b/test/generators/java/presets/__snapshots__/CommonPreset.spec.ts.snap @@ -6,7 +6,7 @@ exports[`JAVA_COMMON_PRESET should render accurately when there is no additional private String stringProp; private Double numberProp; private Boolean booleanProp; - private Object[] arrayProp; + private String[] arrayProp; public Boolean getRequiredProp() { return this.requiredProp; } public void setRequiredProp(Boolean requiredProp) { this.requiredProp = requiredProp; } @@ -20,8 +20,8 @@ exports[`JAVA_COMMON_PRESET should render accurately when there is no additional public Boolean getBooleanProp() { return this.booleanProp; } public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; } - public Object[] getArrayProp() { return this.arrayProp; } - public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; } + public String[] getArrayProp() { return this.arrayProp; } + public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; } @Override public boolean equals(Object o) { @@ -75,7 +75,7 @@ exports[`JAVA_COMMON_PRESET should render common function in class by common pre private String stringProp; private Double numberProp; private Boolean booleanProp; - private Object[] arrayProp; + private String[] arrayProp; private Map additionalProperties; public Boolean getRequiredProp() { return this.requiredProp; } @@ -90,8 +90,8 @@ exports[`JAVA_COMMON_PRESET should render common function in class by common pre public Boolean getBooleanProp() { return this.booleanProp; } public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; } - public Object[] getArrayProp() { return this.arrayProp; } - public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; } + public String[] getArrayProp() { return this.arrayProp; } + public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; } public Map getAdditionalProperties() { return this.additionalProperties; } public void setAdditionalProperties(Map additionalProperties) { this.additionalProperties = additionalProperties; } @@ -150,7 +150,7 @@ exports[`JAVA_COMMON_PRESET with option should not render any functions when all private String stringProp; private Double numberProp; private Boolean booleanProp; - private Object[] arrayProp; + private String[] arrayProp; private Map additionalProperties; public Boolean getRequiredProp() { return this.requiredProp; } @@ -165,8 +165,8 @@ exports[`JAVA_COMMON_PRESET with option should not render any functions when all public Boolean getBooleanProp() { return this.booleanProp; } public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; } - public Object[] getArrayProp() { return this.arrayProp; } - public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; } + public String[] getArrayProp() { return this.arrayProp; } + public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; } public Map getAdditionalProperties() { return this.additionalProperties; } public void setAdditionalProperties(Map additionalProperties) { this.additionalProperties = additionalProperties; } @@ -179,7 +179,7 @@ exports[`JAVA_COMMON_PRESET with option should render all functions 1`] = ` private String stringProp; private Double numberProp; private Boolean booleanProp; - private Object[] arrayProp; + private String[] arrayProp; private Map additionalProperties; public Boolean getRequiredProp() { return this.requiredProp; } @@ -194,8 +194,8 @@ exports[`JAVA_COMMON_PRESET with option should render all functions 1`] = ` public Boolean getBooleanProp() { return this.booleanProp; } public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; } - public Object[] getArrayProp() { return this.arrayProp; } - public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; } + public String[] getArrayProp() { return this.arrayProp; } + public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; } public Map getAdditionalProperties() { return this.additionalProperties; } public void setAdditionalProperties(Map additionalProperties) { this.additionalProperties = additionalProperties; } @@ -254,7 +254,7 @@ exports[`JAVA_COMMON_PRESET with option should render classToString 1`] = ` private String stringProp; private Double numberProp; private Boolean booleanProp; - private Object[] arrayProp; + private String[] arrayProp; private Map additionalProperties; public Boolean getRequiredProp() { return this.requiredProp; } @@ -269,8 +269,8 @@ exports[`JAVA_COMMON_PRESET with option should render classToString 1`] = ` public Boolean getBooleanProp() { return this.booleanProp; } public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; } - public Object[] getArrayProp() { return this.arrayProp; } - public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; } + public String[] getArrayProp() { return this.arrayProp; } + public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; } public Map getAdditionalProperties() { return this.additionalProperties; } public void setAdditionalProperties(Map additionalProperties) { this.additionalProperties = additionalProperties; } @@ -306,7 +306,7 @@ exports[`JAVA_COMMON_PRESET with option should render equals 1`] = ` private String stringProp; private Double numberProp; private Boolean booleanProp; - private Object[] arrayProp; + private String[] arrayProp; private Map additionalProperties; public Boolean getRequiredProp() { return this.requiredProp; } @@ -321,8 +321,8 @@ exports[`JAVA_COMMON_PRESET with option should render equals 1`] = ` public Boolean getBooleanProp() { return this.booleanProp; } public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; } - public Object[] getArrayProp() { return this.arrayProp; } - public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; } + public String[] getArrayProp() { return this.arrayProp; } + public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; } public Map getAdditionalProperties() { return this.additionalProperties; } public void setAdditionalProperties(Map additionalProperties) { this.additionalProperties = additionalProperties; } @@ -353,7 +353,7 @@ exports[`JAVA_COMMON_PRESET with option should render hashCode 1`] = ` private String stringProp; private Double numberProp; private Boolean booleanProp; - private Object[] arrayProp; + private String[] arrayProp; private Map additionalProperties; public Boolean getRequiredProp() { return this.requiredProp; } @@ -368,8 +368,8 @@ exports[`JAVA_COMMON_PRESET with option should render hashCode 1`] = ` public Boolean getBooleanProp() { return this.booleanProp; } public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; } - public Object[] getArrayProp() { return this.arrayProp; } - public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; } + public String[] getArrayProp() { return this.arrayProp; } + public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; } public Map getAdditionalProperties() { return this.additionalProperties; } public void setAdditionalProperties(Map additionalProperties) { this.additionalProperties = additionalProperties; } @@ -387,7 +387,7 @@ exports[`JAVA_COMMON_PRESET with option should render un/marshal 1`] = ` private String stringProp; private Double numberProp; private Boolean booleanProp; - private Object[] arrayProp; + private String[] arrayProp; private Map additionalProperties; public Boolean getRequiredProp() { return this.requiredProp; } @@ -402,8 +402,8 @@ exports[`JAVA_COMMON_PRESET with option should render un/marshal 1`] = ` public Boolean getBooleanProp() { return this.booleanProp; } public void setBooleanProp(Boolean booleanProp) { this.booleanProp = booleanProp; } - public Object[] getArrayProp() { return this.arrayProp; } - public void setArrayProp(Object[] arrayProp) { this.arrayProp = arrayProp; } + public String[] getArrayProp() { return this.arrayProp; } + public void setArrayProp(String[] arrayProp) { this.arrayProp = arrayProp; } public Map getAdditionalProperties() { return this.additionalProperties; } public void setAdditionalProperties(Map additionalProperties) { this.additionalProperties = additionalProperties; } @@ -447,7 +447,7 @@ exports[`JAVA_COMMON_PRESET with option should render un/marshal 1`] = ` result.setBooleanProp(jsonObject.getBoolean(\\"booleanProp\\")); } if(jsonObject.has(\\"arrayProp\\")) { - result.setArrayProp(jsonObject.getObject[](\\"arrayProp\\")); + result.setArrayProp(jsonObject.getString[](\\"arrayProp\\")); } if(jsonObject.has(\\"additionalProperties\\")) { result.setAdditionalProperties(jsonObject.getMap(\\"additionalProperties\\")); diff --git a/test/generators/rust/presets/CommonPreset.spec.ts b/test/generators/rust/presets/CommonPreset.spec.ts index 238bbdd564..5e77ae659c 100644 --- a/test/generators/rust/presets/CommonPreset.spec.ts +++ b/test/generators/rust/presets/CommonPreset.spec.ts @@ -143,10 +143,9 @@ describe('RUST_COMMON_PRESET', () => { }; const models = await generator.generate(doc); - expect(models).toHaveLength(3); + expect(models).toHaveLength(2); expect(models[0].result).toMatchSnapshot(); expect(models[1].result).toMatchSnapshot(); - expect(models[2].result).toMatchSnapshot(); }); }); diff --git a/test/generators/rust/presets/__snapshots__/CommonPreset.spec.ts.snap b/test/generators/rust/presets/__snapshots__/CommonPreset.spec.ts.snap index a171cefa0b..8029807fb2 100644 --- a/test/generators/rust/presets/__snapshots__/CommonPreset.spec.ts.snap +++ b/test/generators/rust/presets/__snapshots__/CommonPreset.spec.ts.snap @@ -197,13 +197,13 @@ exports[`RUST_COMMON_PRESET Enum should render reserved union for dict array 1`] #[derive(Clone, Debug, Deserialize, Serialize)] pub struct Class { #[serde(rename=\\"students\\")] - pub students: Vec, + pub students: Vec, #[serde(rename=\\"additionalProperties\\", skip_serializing_if = \\"Option::is_none\\")] pub additional_properties: Option>, } impl Class { - pub fn new(students: Vec, additional_properties: Option>) -> Class { + pub fn new(students: Vec, additional_properties: Option>) -> Class { Class { students, additional_properties, @@ -214,20 +214,6 @@ impl Class { `; exports[`RUST_COMMON_PRESET Enum should render reserved union for dict array 2`] = ` -"// Union represents a union of types: Student, serde_json::Value -#[derive(Clone, Debug, Deserialize, Serialize)] -#[serde(untagged)] -pub enum Union { - #[serde(rename=\\"Student\\")] - Student(crate::Student), - #[serde(rename=\\"Undefined\\")] - Undefined(serde_json::Value), -} - -" -`; - -exports[`RUST_COMMON_PRESET Enum should render reserved union for dict array 3`] = ` "// Student represents a Student model. #[derive(Clone, Debug, Deserialize, Serialize)] pub struct Student { diff --git a/test/generators/typescript/__snapshots__/TypeScriptGenerator.spec.ts.snap b/test/generators/typescript/__snapshots__/TypeScriptGenerator.spec.ts.snap index b878c808a0..e6224762af 100644 --- a/test/generators/typescript/__snapshots__/TypeScriptGenerator.spec.ts.snap +++ b/test/generators/typescript/__snapshots__/TypeScriptGenerator.spec.ts.snap @@ -368,7 +368,7 @@ exports[`TypeScriptGenerator should render \`class\` type 1`] = ` private _members?: string | number | boolean; private _tupleType?: [string, number]; private _tupleTypeWithAdditionalItems?: (string | number | any)[]; - private _arrayType: (string | any)[]; + private _arrayType: string[]; private _additionalProperties?: Map; constructor(input: { @@ -380,7 +380,7 @@ exports[`TypeScriptGenerator should render \`class\` type 1`] = ` members?: string | number | boolean, tupleType?: [string, number], tupleTypeWithAdditionalItems?: (string | number | any)[], - arrayType: (string | any)[], + arrayType: string[], additionalProperties?: Map, }) { this._streetName = input.streetName; @@ -419,8 +419,8 @@ exports[`TypeScriptGenerator should render \`class\` type 1`] = ` get tupleTypeWithAdditionalItems(): (string | number | any)[] | undefined { return this._tupleTypeWithAdditionalItems; } set tupleTypeWithAdditionalItems(tupleTypeWithAdditionalItems: (string | number | any)[] | undefined) { this._tupleTypeWithAdditionalItems = tupleTypeWithAdditionalItems; } - get arrayType(): (string | any)[] { return this._arrayType; } - set arrayType(arrayType: (string | any)[]) { this._arrayType = arrayType; } + get arrayType(): string[] { return this._arrayType; } + set arrayType(arrayType: string[]) { this._arrayType = arrayType; } get additionalProperties(): Map | undefined { return this._additionalProperties; } set additionalProperties(additionalProperties: Map | undefined) { this._additionalProperties = additionalProperties; } @@ -447,14 +447,14 @@ exports[`TypeScriptGenerator should render \`interface\` type 1`] = ` members?: string | number | boolean; tupleType?: [string, number]; tupleTypeWithAdditionalItems?: (string | number | any)[]; - arrayType: (string | any)[]; + arrayType: string[]; additionalProperties?: Map; }" `; -exports[`TypeScriptGenerator should render \`type\` type - array of primitive type 1`] = `"type TypeArray = (string | any)[];"`; +exports[`TypeScriptGenerator should render \`type\` type - array of primitive type 1`] = `"type TypeArray = string[];"`; -exports[`TypeScriptGenerator should render \`type\` type - array of union type 1`] = `"type TypeArray = (string | number | boolean | any)[];"`; +exports[`TypeScriptGenerator should render \`type\` type - array of union type 1`] = `"type TypeArray = (string | number | boolean)[];"`; exports[`TypeScriptGenerator should render \`type\` type - enum 1`] = ` "enum TypeEnum { diff --git a/test/helpers/CommonModelToMetaModel.spec.ts b/test/helpers/CommonModelToMetaModel.spec.ts index 0a02fd3c34..c7e7de1959 100644 --- a/test/helpers/CommonModelToMetaModel.spec.ts +++ b/test/helpers/CommonModelToMetaModel.spec.ts @@ -134,6 +134,7 @@ describe('CommonModelToMetaModel', () => { expect(model).not.toBeUndefined(); expect(model instanceof ArrayModel).toEqual(true); + expect((model as ArrayModel).valueModel instanceof AnyModel).toEqual(true); }); test('should convert to object model', () => { const spm = new CommonModel(); @@ -273,8 +274,11 @@ describe('CommonModelToMetaModel', () => { expect(model).not.toBeUndefined(); expect(model instanceof ArrayModel).toEqual(true); + expect((model as ArrayModel).valueModel instanceof StringModel).toEqual( + true + ); }); - test('should convert array with additional items to array model as union type', () => { + test('should not convert array with additional items to array model as union type', () => { const spm = new CommonModel(); spm.type = 'string'; const cm = new CommonModel(); @@ -287,7 +291,7 @@ describe('CommonModelToMetaModel', () => { expect(model).not.toBeUndefined(); expect(model instanceof ArrayModel).toEqual(true); - expect((model as ArrayModel).valueModel instanceof UnionModel).toEqual( + expect((model as ArrayModel).valueModel instanceof StringModel).toEqual( true ); });