Merge pull request #5891 from microsoft/fix/implements-error-inline

fix: missing interface declaration for inherited error models
microsoft · Dec 11, 2024 · 5dbc92e · 5dbc92e
2 parents 0731517 + 2b088ef
commit 5dbc92e
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Fixed a bug in generation when a referenced schema in an allOf was a primitive [#5701](https://github.com/microsoft/kiota/issues/5701).
+- Fixed a bug where inherited error models would be missing interface declarations. [#5888](https://github.com/microsoft/kiota/issues/5888)
 
 ## [1.21.0] - 2024-12-05
 
@@ -1519,4 +1520,3 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - Initial GitHub release
-
diff --git a/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs b/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs
@@ -946,7 +946,7 @@ parent is CodeType parentType &&
                 foreach (var implement in currentParent
                              .StartBlock
                              .Implements
-                             .Where(pi => !currentClass.Usings.Any(ci => ci.Name.Equals(pi.Name, StringComparison.OrdinalIgnoreCase))))
+                             .Where(pi => !currentClass.Usings.Any(ci => !ci.IsExternal && ci.Name.Equals(pi.Name, StringComparison.OrdinalIgnoreCase))))
                 {
                     currentClass.StartBlock.AddImplements((CodeType)implement.Clone());
                 }

diff --git a/tests/Kiota.Builder.Tests/Refiners/CSharpLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/CSharpLanguageRefinerTests.cs
@@ -115,6 +115,11 @@ public async Task InlineParentOnErrorClassesWhichAlreadyInheritAsync()
         {
             Name = "otherNs",
         });
+        otherModel.StartBlock.AddImplements(new CodeType
+        {
+            Name = "IAdditionalDataHolder",
+            IsExternal = true
+        });
         var declaration = model.StartBlock;
         declaration.Inherits = new CodeType
         {
@@ -125,6 +130,7 @@ public async Task InlineParentOnErrorClassesWhichAlreadyInheritAsync()
         Assert.Contains(model.Properties, x => x.Name.Equals("OtherProp"));
         Assert.Contains(model.Methods, x => x.Name.Equals("otherMethod"));
         Assert.Contains(model.Usings, x => x.Name.Equals("otherNs"));
+        Assert.Contains(model.StartBlock.Implements, x => x.Name.Equals("IAdditionalDataHolder"));
     }
     [Fact]
     public async Task AddsUsingsForErrorTypesForRequestExecutorAsync()

diff --git a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs
@@ -578,6 +578,11 @@ public async Task InlineParentOnErrorClassesWhichAlreadyInheritAsync()
         {
             Name = "otherNs",
         });
+        otherModel.StartBlock.AddImplements(new CodeType
+        {
+            Name = "IAdditionalDataHolder",
+            IsExternal = true
+        });
         var declaration = model.StartBlock;
         declaration.Inherits = new CodeType
         {
@@ -588,6 +593,10 @@ public async Task InlineParentOnErrorClassesWhichAlreadyInheritAsync()
         Assert.Contains(model.Properties, x => x.Name.Equals("otherProp"));
         Assert.Contains(model.Methods, x => x.Name.Equals("otherMethod"));
         Assert.Contains(model.Usings, x => x.Name.Equals("otherNs"));
+
+        var modelInterface = root.FindChildByName<CodeInterface>("somemodelable");
+        Assert.NotNull(modelInterface);
+        Assert.Contains(modelInterface.StartBlock.Implements, x => x.Name.Equals("AdditionalDataHolder", StringComparison.OrdinalIgnoreCase));
     }
     [Fact]
     public async Task AddsUsingsForErrorTypesForRequestExecutorAsync()

diff --git a/tests/Kiota.Builder.Tests/Refiners/JavaLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/JavaLanguageRefinerTests.cs
@@ -84,6 +84,11 @@ public async Task InlineParentOnErrorClassesWhichAlreadyInheritAsync()
         {
             Name = "otherNs",
         });
+        otherModel.StartBlock.AddImplements(new CodeType
+        {
+            Name = "IAdditionalDataHolder",
+            IsExternal = true
+        });
         var declaration = model.StartBlock;
         declaration.Inherits = new CodeType
         {
@@ -95,6 +100,7 @@ public async Task InlineParentOnErrorClassesWhichAlreadyInheritAsync()
         Assert.Contains(model.Methods, x => x.Name.Equals("otherMethod"));
         Assert.Contains(model.Usings, x => x.Name.Equals("otherNs"));
         Assert.Equal("ApiException", model.StartBlock.Inherits.Name);
+        Assert.Contains(model.StartBlock.Implements, x => x.Name.Equals("AdditionalDataHolder", StringComparison.OrdinalIgnoreCase));
     }
     [Fact]
     public async Task AddsUsingsForErrorTypesForRequestExecutorAsync()

diff --git a/tests/Kiota.Builder.Tests/Refiners/PythonLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/PythonLanguageRefinerTests.cs
@@ -160,6 +160,11 @@ public async Task InlineParentOnErrorClassesWhichAlreadyInheritAsync()
         {
             Name = "otherNs",
         });
+        otherModel.StartBlock.AddImplements(new CodeType
+        {
+            Name = "IAdditionalDataHolder",
+            IsExternal = true
+        });
         var declaration = model.StartBlock;
         declaration.Inherits = new CodeType
         {
@@ -170,6 +175,7 @@ public async Task InlineParentOnErrorClassesWhichAlreadyInheritAsync()
         Assert.Contains(model.Properties, x => x.Name.Equals("other_prop"));
         Assert.Contains(model.Methods, x => x.Name.Equals("other_method"));
         Assert.Contains(model.Usings, x => x.Name.Equals("otherNs"));
+        Assert.Contains(model.StartBlock.Implements, x => x.Name.Equals("AdditionalDataHolder", StringComparison.OrdinalIgnoreCase));
     }
     [Fact]
     public async Task AddsUsingsForErrorTypesForRequestExecutorAsync()

diff --git a/tests/Kiota.Builder.Tests/Refiners/TypeScriptLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/TypeScriptLanguageRefinerTests.cs
@@ -177,16 +177,32 @@ public async Task InlineParentOnErrorClassesWhichAlreadyInheritAsync()
             new CodeUsing
             {
                 Name = "otherNs",
+            },
+            new CodeUsing
+            {
+                Name = "AdditionalDataHolder",
             });
+        otherModel.StartBlock.AddImplements(new CodeType
+        {
+            Name = "AdditionalDataHolder",
+            IsExternal = true
+        });
         model.StartBlock.Inherits = new CodeType
         {
             TypeDefinition = otherModel
         };
         await ILanguageRefiner.RefineAsync(new GenerationConfiguration { Language = GenerationLanguage.TypeScript }, root);
 
-        Assert.DoesNotContain("otherProp", model.Properties.Select(static x => x.Name), StringComparer.OrdinalIgnoreCase); // we're not inlining since base error for TS is an interface
-        Assert.DoesNotContain("otherMethod", model.Methods.Select(static x => x.Name), StringComparer.OrdinalIgnoreCase);
-        Assert.DoesNotContain("otherNs", model.Usings.Select(static x => x.Name), StringComparer.OrdinalIgnoreCase);
+        var interfaceModel = root.FindChildByName<CodeInterface>("somemodel");
+        Assert.NotNull(interfaceModel);
+
+        Assert.DoesNotContain("otherProp", interfaceModel.Properties.Select(static x => x.Name), StringComparer.OrdinalIgnoreCase); // we're not inlining since base error for TS is an interface
+        Assert.DoesNotContain("otherMethod", interfaceModel.Methods.Select(static x => x.Name), StringComparer.OrdinalIgnoreCase);
+        Assert.DoesNotContain("otherNs", interfaceModel.Usings.Select(static x => x.Name), StringComparer.OrdinalIgnoreCase);
+
+        var interfaceOtherModel = root.FindChildByName<CodeInterface>("otherModel");
+        Assert.NotNull(interfaceOtherModel);
+        Assert.Contains(interfaceOtherModel.StartBlock.Implements, x => x.Name.Equals("AdditionalDataHolder", StringComparison.OrdinalIgnoreCase));
     }
     [Fact]
     public async Task AddsUsingsForErrorTypesForRequestExecutorAsync()