From 2f7fb70881f3be76e79e21253ab4c206092209ca Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Wed, 15 May 2024 16:49:40 +0300 Subject: [PATCH 1/2] Fixes collision in Typescript refiner --- CHANGELOG.md | 1 + src/Kiota.Builder/CodeDOM/CodeInterface.cs | 4 +-- .../Refiners/TypeScriptRefiner.cs | 28 ++++++++++++------- .../CodeDOM/CodeClassTests.cs | 3 +- .../TypeScriptLanguageRefinerTests.cs | 17 +++++++++++ .../Go/CodeInterfaceDeclarationWriterTests.cs | 3 +- .../Python/CodeClassDeclarationWriterTests.cs | 1 + .../TypeScript/CodeConstantWriterTests.cs | 4 ++- .../TypeScript/CodeFunctionWriterTests.cs | 6 ++++ .../CodeInterfaceDeclarationWriterTests .cs | 5 +++- .../TypeScript/CodePropertyWriterTests.cs | 3 +- .../TypeScript/CodeUsingWriterTests.cs | 3 +- 12 files changed, 60 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7765008e57..e8a62266ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed a bug where some allOf scenarios would be missing properties if type object wasn't set on the schema. [#4074](https://github.com/microsoft/kiota/issues/4074) - Fixed a bug where schema with multiple allOf entries would incorrectly get merged to inherit from the first entry [#4428] (https://github.com/microsoft/kiota/issues/4428) - Fixes constructor generation for nullable properties that are initialized as null in C#,Java and PHP +- Fixes a bug where name collisions would occur in the Typescript refiner if model name also exists with the `Interface` suffix [#4382](https://github.com/microsoft/kiota/issues/4382) ## [1.14.0] - 2024-05-02 diff --git a/src/Kiota.Builder/CodeDOM/CodeInterface.cs b/src/Kiota.Builder/CodeDOM/CodeInterface.cs index 08ce97203a..68ae5fdaee 100644 --- a/src/Kiota.Builder/CodeDOM/CodeInterface.cs +++ b/src/Kiota.Builder/CodeDOM/CodeInterface.cs @@ -13,9 +13,9 @@ public enum CodeInterfaceKind public class CodeInterface : ProprietableBlock, ITypeDefinition, IDeprecableElement { - public CodeClass? OriginalClass + public required CodeClass OriginalClass { - get; set; + get; init; } public DeprecationInformation? Deprecation { diff --git a/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs b/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs index 6ef8b678b6..48ba83cfdc 100644 --- a/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs +++ b/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs @@ -659,6 +659,7 @@ codeClass.Parent is CodeClass parentClass && Kind = CodeInterfaceKind.QueryParameters, Documentation = codeClass.Documentation, Deprecation = codeClass.Deprecation, + OriginalClass = codeClass }; parentClass.RemoveChildElement(codeClass); var codeInterface = targetNS.AddInterface(insertValue).First(); @@ -674,7 +675,7 @@ codeClass.Parent is CodeClass parentClass && } CrawlTree(currentElement, ReplaceRequestQueryParamsWithInterfaces); } - private const string TemporaryInterfaceNameSuffix = "Interface"; + private const string ModelSerializerPrefix = "serialize"; private const string ModelDeserializerPrefix = "deserializeInto"; @@ -737,9 +738,9 @@ private static void AddInterfaceParamToSerializer(CodeInterface modelInterface, method.AddParameter(new CodeParameter { - Name = ReturnFinalInterfaceName(modelInterface.Name), // remove the interface suffix + Name = ReturnFinalInterfaceName(modelInterface), DefaultValue = "{}", - Type = new CodeType { Name = ReturnFinalInterfaceName(modelInterface.Name), TypeDefinition = modelInterface }, + Type = new CodeType { Name = ReturnFinalInterfaceName(modelInterface), TypeDefinition = modelInterface }, Kind = CodeParameterKind.DeserializationTarget, }); @@ -750,7 +751,7 @@ private static void AddInterfaceParamToSerializer(CodeInterface modelInterface, Name = modelInterface.Parent.Name, Declaration = new CodeType { - Name = ReturnFinalInterfaceName(modelInterface.Name), + Name = ReturnFinalInterfaceName(modelInterface), TypeDefinition = modelInterface } }); @@ -781,7 +782,7 @@ private static void RenameModelInterfacesAndRemoveClasses(CodeElement currentEle { if (currentElement is CodeInterface modelInterface && modelInterface.IsOfKind(CodeInterfaceKind.Model) && modelInterface.Parent is CodeNamespace parentNS) { - var finalName = ReturnFinalInterfaceName(modelInterface.Name); + var finalName = ReturnFinalInterfaceName(modelInterface); if (!finalName.Equals(modelInterface.Name, StringComparison.Ordinal)) { if (parentNS.FindChildByName(finalName, false) is CodeClass existingClassToRemove) @@ -801,13 +802,13 @@ private static void RenameCodeInterfaceParamsInSerializers(CodeFunction codeFunc { if (codeFunction.OriginalLocalMethod.Parameters.FirstOrDefault(static x => x.Type is CodeType codeType && codeType.TypeDefinition is CodeInterface) is CodeParameter param && param.Type is CodeType codeType && codeType.TypeDefinition is CodeInterface paramInterface) { - param.Name = ReturnFinalInterfaceName(paramInterface.Name); + param.Name = ReturnFinalInterfaceName(paramInterface); } } - private static string ReturnFinalInterfaceName(string interfaceName) + private static string ReturnFinalInterfaceName(CodeInterface codeInterface) { - return interfaceName.EndsWith(TemporaryInterfaceNameSuffix, StringComparison.Ordinal) ? interfaceName[..^TemporaryInterfaceNameSuffix.Length] : interfaceName; + return codeInterface.OriginalClass.Name.ToFirstCharacterUpperCase(); } private static void GenerateModelInterfaces(CodeElement currentElement, Func interfaceNamingCallback) @@ -925,7 +926,7 @@ private static void SetTypeAsModelInterface(CodeInterface interfaceElement, Code Name = interfaceElement.Name, TypeDefinition = interfaceElement, }; - requestBuilder.RemoveUsingsByDeclarationName(interfaceElement.Name.Split(TemporaryInterfaceNameSuffix)[0]); + requestBuilder.RemoveUsingsByDeclarationName(ReturnFinalInterfaceName(interfaceElement)); if (!requestBuilder.Usings.Any(x => x.Declaration?.TypeDefinition == elemType.TypeDefinition)) { requestBuilder.AddUsing(new CodeUsing @@ -957,12 +958,19 @@ private static CodeInterface CreateModelInterface(CodeClass modelClass, Func(temporaryInterfaceName, false) is CodeInterface existing) return existing; + var i = 1; + while (namespaceOfModel.FindChildByName(temporaryInterfaceName, false) != null) + { + temporaryInterfaceName = $"{temporaryInterfaceName}{i++}"; + } + var insertValue = new CodeInterface { Name = temporaryInterfaceName, Kind = CodeInterfaceKind.Model, Documentation = modelClass.Documentation, Deprecation = modelClass.Deprecation, + OriginalClass = modelClass }; var modelInterface = modelClass.Parent is CodeClass modelParentClass ? @@ -987,7 +995,7 @@ private static void ProcessModelClassDeclaration(CodeClass modelClass, CodeInter var parentInterface = CreateModelInterface(baseClass, tempInterfaceNamingCallback); var codeType = new CodeType { - Name = ReturnFinalInterfaceName(parentInterface.Name), + Name = ReturnFinalInterfaceName(parentInterface), TypeDefinition = parentInterface, }; modelInterface.StartBlock.AddImplements(codeType); diff --git a/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs b/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs index de89de8fb5..41ccea91e3 100644 --- a/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs +++ b/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs @@ -193,7 +193,8 @@ public void AddsInnerInterface() }).First(); codeClass.AddInnerInterface(new CodeInterface { - Name = "subinterface" + Name = "subinterface", + OriginalClass = new CodeClass() { Name = "originalSubInterface" } }); Assert.Single(codeClass.GetChildElements(true).OfType()); Assert.Throws(() => diff --git a/tests/Kiota.Builder.Tests/Refiners/TypeScriptLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/TypeScriptLanguageRefinerTests.cs index e7704461fa..17e660a952 100644 --- a/tests/Kiota.Builder.Tests/Refiners/TypeScriptLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/TypeScriptLanguageRefinerTests.cs @@ -626,6 +626,23 @@ public async Task AddsModelInterfaceForAModelClass() Assert.Contains(codeFile.Interfaces, static x => "ModelA".Equals(x.Name, StringComparison.OrdinalIgnoreCase)); } + [Fact] + public async Task AddsModelInterfaceForAModelClassWithoutCollision() + { + var generationConfiguration = new GenerationConfiguration { Language = GenerationLanguage.TypeScript }; + TestHelper.CreateModelClassInModelsNamespace(generationConfiguration, root, "hostModel"); + TestHelper.CreateModelClassInModelsNamespace(generationConfiguration, root, "hostModelInterface");// a second model with the `Interface` suffix + + await ILanguageRefiner.Refine(generationConfiguration, root); + + var modelsNS = root.FindNamespaceByName(generationConfiguration.ModelsNamespaceName); + var codeFile = modelsNS.FindChildByName(IndexFileName, false); + Assert.NotNull(codeFile); + Assert.Equal(2,codeFile.Interfaces.Count()); + Assert.Contains(codeFile.Interfaces, static x => "hostModel".Equals(x.Name, StringComparison.OrdinalIgnoreCase)); + Assert.Contains(codeFile.Interfaces, static x => "hostModelInterface".Equals(x.Name, StringComparison.OrdinalIgnoreCase)); + } + [Fact] public async Task ReplaceRequestConfigsQueryParams() { diff --git a/tests/Kiota.Builder.Tests/Writers/Go/CodeInterfaceDeclarationWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Go/CodeInterfaceDeclarationWriterTests.cs index 4610fa5044..2941424878 100644 --- a/tests/Kiota.Builder.Tests/Writers/Go/CodeInterfaceDeclarationWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Go/CodeInterfaceDeclarationWriterTests.cs @@ -27,7 +27,8 @@ public CodeInterfaceDeclarationWriterTests() root = CodeNamespace.InitRootNamespace(); parentInterface = new() { - Name = "parentClass" + Name = "parentClass", + OriginalClass = new CodeClass() { Name = "parentClass" } }; root.AddInterface(parentInterface); } diff --git a/tests/Kiota.Builder.Tests/Writers/Python/CodeClassDeclarationWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Python/CodeClassDeclarationWriterTests.cs index 74f36a2bd1..3cdcaf3042 100644 --- a/tests/Kiota.Builder.Tests/Writers/Python/CodeClassDeclarationWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Python/CodeClassDeclarationWriterTests.cs @@ -99,6 +99,7 @@ public void WritesInheritance() var interfaceDef = new CodeInterface { Name = "SomeInterface", + OriginalClass = new CodeClass() { Name = "SomeInterface" } }; ns.AddInterface(interfaceDef); var nUsing = new CodeUsing diff --git a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeConstantWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeConstantWriterTests.cs index a8309a6076..759ea67dc8 100644 --- a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeConstantWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeConstantWriterTests.cs @@ -349,6 +349,7 @@ public void WritesRequestGeneratorBodyForParsable() { Name = "SomeComplexTypeForRequestBody", Kind = CodeInterfaceKind.Model, + OriginalClass = new CodeClass() { Name = "SomeComplexTypeForRequestBody" } }, }; generatorMethod.AcceptedResponseTypes.Add("application/json"); @@ -502,7 +503,8 @@ public void WritesIndexer() var parentInterface = new CodeInterface { Name = "parentClass", - Kind = CodeInterfaceKind.RequestBuilder + Kind = CodeInterfaceKind.RequestBuilder, + OriginalClass = new CodeClass() { Name = "parentClass" } }; method.AddParameter(new CodeParameter { diff --git a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeFunctionWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeFunctionWriterTests.cs index a22dec13c7..f057dc872c 100644 --- a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeFunctionWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeFunctionWriterTests.cs @@ -896,6 +896,7 @@ public void WritesReturnType() { Name = "SomeInterface", Kind = CodeInterfaceKind.Model, + OriginalClass = new CodeClass() { Name = "SomeInterface" } }).First(); var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName); method.Kind = CodeMethodKind.Serializer; @@ -930,6 +931,7 @@ public void DoesNotAddUndefinedOnNonNullableReturnType() { Name = "SomeInterface", Kind = CodeInterfaceKind.Model, + OriginalClass = new CodeClass() { Name = "SomeInterface" } }).First(); var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName); method.Kind = CodeMethodKind.Serializer; @@ -963,6 +965,7 @@ public void DoesNotAddAsyncInformationOnSyncMethods() { Name = "SomeInterface", Kind = CodeInterfaceKind.Model, + OriginalClass = new CodeClass() { Name = "SomeInterface" } }).First(); var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName); method.Kind = CodeMethodKind.Serializer; @@ -997,6 +1000,7 @@ public void WritesPublicMethodByDefault() { Name = "SomeInterface", Kind = CodeInterfaceKind.Model, + OriginalClass = new CodeClass() { Name = "SomeInterface" } }).First(); var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName); method.Kind = CodeMethodKind.Serializer; @@ -1029,6 +1033,7 @@ public void WritesPrivateMethod() { Name = "SomeInterface", Kind = CodeInterfaceKind.Model, + OriginalClass = new CodeClass() { Name = "SomeInterface" } }).First(); var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName); method.Kind = CodeMethodKind.Serializer; @@ -1062,6 +1067,7 @@ public void WritesProtectedMethod() { Name = "SomeInterface", Kind = CodeInterfaceKind.Model, + OriginalClass = new CodeClass() { Name = "SomeInterface" } }).First(); var method = TestHelper.CreateMethod(parentClass, MethodName, ReturnTypeName); method.Kind = CodeMethodKind.Serializer; diff --git a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeInterfaceDeclarationWriterTests .cs b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeInterfaceDeclarationWriterTests .cs index c5495dcac5..ace5c2ec9e 100644 --- a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeInterfaceDeclarationWriterTests .cs +++ b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeInterfaceDeclarationWriterTests .cs @@ -1,5 +1,6 @@ using System; using System.IO; +using System.Linq; using Kiota.Builder.CodeDOM; using Xunit; @@ -19,9 +20,11 @@ public CodeInterfaceDeclaraterWriterTests() writer.SetTextWriter(tw); var root = CodeNamespace.InitRootNamespace(); var ns = root.AddNamespace("graphtests.models"); + var originalClass = ns.AddClass(new CodeClass() { Name = "originalParentClass" }).First(); parentInterface = new CodeInterface() { - Name = "parent" + Name = "parent", + OriginalClass = originalClass }; ns.AddInterface(parentInterface); } diff --git a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodePropertyWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodePropertyWriterTests.cs index 00bf2a47d8..70d56bf989 100644 --- a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodePropertyWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodePropertyWriterTests.cs @@ -25,7 +25,8 @@ public CodePropertyWriterTests() var root = CodeNamespace.InitRootNamespace(); parentInterface = root.AddInterface(new CodeInterface { - Name = "parentClass" + Name = "parentClass", + OriginalClass = new CodeClass() { Name = "parentClass" } }).First(); property = new CodeProperty { diff --git a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeUsingWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeUsingWriterTests.cs index 0071f095f6..a196c8464f 100644 --- a/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeUsingWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/TypeScript/CodeUsingWriterTests.cs @@ -74,7 +74,8 @@ public void WritesImportTypeStatementForGeneratedInterfaces() var someInterface = new CodeInterface { Name = "Bar", - Kind = CodeInterfaceKind.Model + Kind = CodeInterfaceKind.Model, + OriginalClass = new CodeClass() { Name = "Bar" } }; root.AddInterface(someInterface); var us = new CodeUsing From 7882554330848b0bf2eb4dc4b3f625fec58f55d4 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Wed, 15 May 2024 17:04:03 +0300 Subject: [PATCH 2/2] Format and comment updates --- src/Kiota.Builder/Refiners/TypeScriptRefiner.cs | 2 +- .../Refiners/TypeScriptLanguageRefinerTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs b/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs index 42e47e5aa8..5e6c9673ca 100644 --- a/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs +++ b/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs @@ -960,7 +960,7 @@ private static CodeInterface CreateModelInterface(CodeClass modelClass, Func(temporaryInterfaceName, false) != null) - { + {// We already know an Interface doesn't exist with the name. Make sure we don't collide with an existing class name in the namespace. temporaryInterfaceName = $"{temporaryInterfaceName}{i++}"; } diff --git a/tests/Kiota.Builder.Tests/Refiners/TypeScriptLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/TypeScriptLanguageRefinerTests.cs index 17e660a952..0c7178dd7d 100644 --- a/tests/Kiota.Builder.Tests/Refiners/TypeScriptLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/TypeScriptLanguageRefinerTests.cs @@ -638,7 +638,7 @@ public async Task AddsModelInterfaceForAModelClassWithoutCollision() var modelsNS = root.FindNamespaceByName(generationConfiguration.ModelsNamespaceName); var codeFile = modelsNS.FindChildByName(IndexFileName, false); Assert.NotNull(codeFile); - Assert.Equal(2,codeFile.Interfaces.Count()); + Assert.Equal(2, codeFile.Interfaces.Count()); Assert.Contains(codeFile.Interfaces, static x => "hostModel".Equals(x.Name, StringComparison.OrdinalIgnoreCase)); Assert.Contains(codeFile.Interfaces, static x => "hostModelInterface".Equals(x.Name, StringComparison.OrdinalIgnoreCase)); }