Merge pull request #4667 from microsoft/andrueastman/deduplicateInter…

…faces Fixes name collisions in TS refiner between models and interfaces.
microsoft · May 15, 2024 · 596e783 · 596e783
2 parents 088c318 + 7882554
commit 596e783
Show file tree

Hide file tree

Showing 12 changed files with 60 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - 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
 - Fixed a bug where the hash alias in typescript wasn't being generated uniformly for similar interfaces [#84](https://github.com/microsoftgraph/msgraph-beta-sdk-typescript/issues/84)
+- 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
@@ -13,9 +13,9 @@ public enum CodeInterfaceKind
 
 public class CodeInterface : ProprietableBlock<CodeInterfaceKind, InterfaceDeclaration>, 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
@@ -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<CodeClass>(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<CodeClass, string> 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<Cod
         if (namespaceOfModel.FindChildByName<CodeInterface>(temporaryInterfaceName, false) is CodeInterface existing)
             return existing;
 
+        var i = 1;
+        while (namespaceOfModel.FindChildByName<CodeClass>(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++}";
+        }
+
         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
@@ -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<CodeInterface>());
         Assert.Throws<ArgumentNullException>(() =>

diff --git 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<CodeFile>(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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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