Merge pull request #4930 from microsoft/andrueastman/fixMethodOverride

Fix `ErrorMessageOverride` collision with classname
microsoft · Jul 3, 2024 · 77b80dd · 77b80dd
2 parents ef10468 + 62f5d4d
commit 77b80dd
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Ensures HashSet properties in `KiotaLock` maintain IgnoreCase comparer across runs [#4916](https://github.com/microsoft/kiota/issues/4916)
 - Dropped `client base url set to` message when generating plugins. [#4905](https://github.com/microsoft/kiota/issues/4905)
 - Emit `[GeneratedCode]` attribute for C# types. [#4907](https://github.com/microsoft/kiota/issues/4907)
+- Fixes error property disambiguation when the property has the same name as class [#4893](https://github.com/microsoft/kiota/issues/)
 - Fixes missing imports for `UntypedNode` for method parameter and return value scenarios. [#4925](https://github.com/microsoft/kiota/issues/4925)
 
 ## [1.15.0] - 2024-06-06

diff --git a/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs b/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs
@@ -294,7 +294,9 @@ protected static void ReplaceReservedExceptionPropertyNames(CodeElement current,
             provider,
             replacement,
             null,
-            static x => ((x is CodeProperty prop && prop.IsOfKind(CodePropertyKind.Custom)) || x is CodeMethod) && x.Parent is CodeClass parent && parent.IsOfKind(CodeClassKind.Model) && parent.IsErrorDefinition
+            x => (((x is CodeProperty prop && prop.IsOfKind(CodePropertyKind.Custom)) || x is CodeMethod) && x.Parent is CodeClass parent && parent.IsOfKind(CodeClassKind.Model) && parent.IsErrorDefinition) // rename properties or method of error classes matching the reserved names.
+                                                || (x is CodeClass codeClass && codeClass.IsOfKind(CodeClassKind.Model) && codeClass.IsErrorDefinition
+                                                    && codeClass.Properties.FirstOrDefault(classProp => provider.ReservedNames.Contains(classProp.Name)) is { } matchingProperty && matchingProperty.Name.Equals(codeClass.Name, StringComparison.OrdinalIgnoreCase)) // rename the a class if it has a matching property and the class has the same name as the property.
         );
     }
     protected static void ReplaceReservedNames(CodeElement current, IReservedNamesProvider provider, Func<string, string> replacement, HashSet<Type>? codeElementExceptions = null, Func<CodeElement, bool>? shouldReplaceCallback = null)
@@ -1507,6 +1509,14 @@ internal static void AddPrimaryErrorMessage(CodeElement currentElement, string n
         {
             if (asProperty)
             {
+                if (currentClass.Properties.FirstOrDefault(property => property.Name.Equals(name, StringComparison.OrdinalIgnoreCase)) is { } sameNameProperty)
+                {
+                    if (sameNameProperty.Type.Name.Equals(type().Name, StringComparison.OrdinalIgnoreCase))
+                        currentClass.RemoveChildElementByName(name); // type matches so just remove it for replacement
+                    else
+                        currentClass.RenameChildElement(name, $"{name}Escaped"); // type mismatch so just rename it
+                }
+
                 currentClass.AddProperty(new CodeProperty
                 {
                     Name = name,

diff --git a/tests/Kiota.Builder.Tests/Refiners/CSharpLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/CSharpLanguageRefinerTests.cs
@@ -446,6 +446,27 @@ public async Task KeepsCancellationParametersInRequestExecutors()
     }
     [Fact]
     public async Task ReplacesExceptionPropertiesNames()
+    {
+        var exception = root.AddClass(new CodeClass
+        {
+            Name = "error403",
+            Kind = CodeClassKind.Model,
+            IsErrorDefinition = true,
+        }).First();
+        var propToAdd = exception.AddProperty(new CodeProperty
+        {
+            Name = "stacktrace",
+            Type = new CodeType
+            {
+                Name = "string"
+            }
+        }).First();
+        await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.CSharp }, root);
+        Assert.Equal("stacktraceEscaped", propToAdd.Name, StringComparer.OrdinalIgnoreCase);
+        Assert.Equal("stacktrace", propToAdd.SerializationName, StringComparer.OrdinalIgnoreCase);
+    }
+    [Fact]
+    public async Task DoesNotRenamePrimaryErrorMessageIfMatchAlreadyExists()
     {
         var exception = root.AddClass(new CodeClass
         {
@@ -461,9 +482,59 @@ public async Task ReplacesExceptionPropertiesNames()
                 Name = "string"
             }
         }).First();
+        Assert.False(exception.Properties.First().IsOfKind(CodePropertyKind.ErrorMessageOverride));// property is NOT message override
         await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.CSharp }, root);
-        Assert.Equal("messageEscaped", propToAdd.Name, StringComparer.OrdinalIgnoreCase);
-        Assert.Equal("message", propToAdd.SerializationName, StringComparer.OrdinalIgnoreCase);
+        Assert.Equal("message", propToAdd.Name, StringComparer.OrdinalIgnoreCase);// property remains
+        Assert.Single(exception.Properties); // no new properties added
+        Assert.True(exception.Properties.First().IsOfKind(CodePropertyKind.ErrorMessageOverride));// property is now message override
+        Assert.Equal("message", exception.Properties.First().Name, StringComparer.OrdinalIgnoreCase); // name is expected.
+    }
+    [Fact]
+    public async Task RenamesExceptionClassWithReservedPropertyName()
+    {
+        var exception = root.AddClass(new CodeClass
+        {
+            Name = "message",
+            Kind = CodeClassKind.Model,
+            IsErrorDefinition = true,
+        }).First();
+        var propToAdd = exception.AddProperty(new CodeProperty
+        {
+            Name = "message",
+            Type = new CodeType
+            {
+                Name = "string"
+            }
+        }).First();
+        await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.CSharp }, root);
+        Assert.Equal("messageEscaped", exception.Name, StringComparer.OrdinalIgnoreCase);// class is renamed to avoid removing special overidden property
+        Assert.Equal("message", propToAdd.Name, StringComparer.OrdinalIgnoreCase); // property is unchanged
+        Assert.Single(exception.Properties); // no new properties added
+        Assert.Equal("message", exception.Properties.First().Name, StringComparer.OrdinalIgnoreCase);
+    }
+    [Fact]
+    public async Task RenamesExceptionClassWithReservedPropertyNameWhenPropertyIsInitiallyAbsent()
+    {
+        var exception = root.AddClass(new CodeClass
+        {
+            Name = "message",
+            Kind = CodeClassKind.Model,
+            IsErrorDefinition = true,
+        }).First();
+        var propToAdd = exception.AddProperty(new CodeProperty
+        {
+            Name = "something",
+            Type = new CodeType
+            {
+                Name = "string"
+            }
+        }).First();
+        await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.CSharp }, root);
+        Assert.Equal("messageEscaped", exception.Name, StringComparer.OrdinalIgnoreCase);// class is renamed to avoid removing special overidden property
+        Assert.Equal("something", propToAdd.Name, StringComparer.OrdinalIgnoreCase); // existing property remains
+        Assert.Equal(2, exception.Properties.Count()); // initial property plus primary message
+        Assert.Equal("message", exception.Properties.ToArray()[0].Name, StringComparer.OrdinalIgnoreCase); // primary error message is present
+        Assert.Equal("something", exception.Properties.ToArray()[1].Name, StringComparer.OrdinalIgnoreCase);// existing property remains
     }
     [Fact]
     public async Task DoesNotReplaceNonExceptionPropertiesNames()