Merge pull request #2862 from microsoft/hotfix/revert-2836

- reverts #2836 due to large number of side effects

CHANGELOG.md

@@ -28,7 +28,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Moved common RequestBuilder ((request_adapter, url_template and path_parameters)) and RequestConfiguration(headers, options) properties to respective base classes in Python.[2440](https://github.com/microsoft/kiota/issues/2440)
 - Fixed a bug where escaped package names would not be lowercased in Java.
 - Fix failing PHP integration tests [2378](https://github.com/microsoft/kiota/issues/2378)
-- Fixed generation of properties with identical names after symbol cleanup. 
 - Prevents method overloading for go getters and setters with different values. [#2719](https://github.com/microsoft/kiota/issues/2719)
 - Fixed PHP request executor methods that return enums.
 

src/Kiota.Builder/CodeDOM/CodeBlock.cs

@@ -30,7 +30,7 @@ public override IEnumerable<CodeElement> GetChildElements(bool innerOnly = false
             return InnerChildElements.Values;
         return new CodeElement[] { StartBlock, EndBlock }.Union(InnerChildElements.Values);
     }
-    public virtual void RenameChildElement(string oldName, string newName)
+    public void RenameChildElement(string oldName, string newName)
     {
         if (InnerChildElements.TryRemove(oldName, out var element))
         {
@@ -44,7 +44,7 @@ public void RemoveChildElement<T>(params T[] elements) where T : CodeElement
         if (elements == null) return;
         RemoveChildElementByName(elements.Select(static x => x.Name).ToArray());
     }
-    public virtual void RemoveChildElementByName(params string[] names)
+    public void RemoveChildElementByName(params string[] names)
     {
         if (names == null) return;
 
@@ -75,7 +75,6 @@ private T HandleDuplicatedExceptions<T>(T element, CodeElement returnedValue) wh
         if (returnedValue == element)
             return element;
         if (element is CodeMethod currentMethod)
-        {
             if (currentMethod.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility) &&
                 returnedValue is CodeProperty cProp &&
                 cProp.IsOfKind(CodePropertyKind.RequestBuilder) &&
@@ -100,19 +99,6 @@ returnedValue is CodeProperty cProp &&
                         return result2;
                 }
             }
-        }
-        else if (element is CodeProperty currentProperty &&
-            returnedValue is CodeProperty existingProperty &&
-            currentProperty.IsOfKind(CodePropertyKind.Custom, CodePropertyKind.QueryParameter) &&
-            existingProperty.IsOfKind(CodePropertyKind.Custom, CodePropertyKind.QueryParameter) &&
-            (existingProperty.IsNameEscaped || currentProperty.IsNameEscaped))
-        {
-            var nameToRemove = existingProperty.IsNameEscaped ? existingProperty.Name : currentProperty.Name;
-            existingProperty.Name = existingProperty.SerializationName;
-            currentProperty.Name = currentProperty.SerializationName;
-            if (InnerChildElements.TryRemove(nameToRemove, out _) && InnerChildElements.TryAdd(existingProperty.Name, existingProperty) && InnerChildElements.TryAdd(currentProperty.Name, currentProperty))
-                return existingProperty.IsNameEscaped ? (T)returnedValue : element;
-        }
 
         if (element.GetType() == returnedValue.GetType())
             return (T)returnedValue;

src/Kiota.Builder/CodeDOM/CodeClass.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Linq;
@@ -33,7 +32,6 @@ public enum CodeClassKind
 /// </summary>
 public class CodeClass : ProprietableBlock<CodeClassKind, ClassDeclaration>, ITypeDefinition, IDiscriminatorInformationHolder, IDeprecableElement
 {
-    protected ConcurrentDictionary<string, CodeProperty> PropertiesByWireName { get; private set; } = new(StringComparer.OrdinalIgnoreCase);
     public bool IsErrorDefinition
     {
         get; set;
@@ -67,74 +65,11 @@ public CodeIndexer? Indexer
     }
     public override IEnumerable<CodeProperty> AddProperty(params CodeProperty[] properties)
     {
-        if (properties == null || properties.Any(static x => x == null))
-            throw new ArgumentNullException(nameof(properties));
-        if (!properties.Any())
-            throw new ArgumentOutOfRangeException(nameof(properties));
-
-        foreach (var property in properties.OfKind(CodePropertyKind.Custom, CodePropertyKind.QueryParameter))
-        {
-            if (GetOriginalPropertyDefinedFromBaseType(property.WireName) is CodeProperty original)
-            {
-                // the property already exists in a parent type, use its name
-                property.Name = original.Name;
-                property.SerializationName = original.SerializationName;
-                property.OriginalPropertyFromBaseType = original;
-            }
-            else
-            {
-                var uniquePropertyName = ResolveUniquePropertyName(property.Name);
-                if (!uniquePropertyName.Equals(property.Name, StringComparison.OrdinalIgnoreCase) && string.IsNullOrEmpty(property.SerializationName))
-                    property.SerializationName = property.Name;
-                property.Name = uniquePropertyName;
-            }
-            PropertiesByWireName.AddOrUpdate(property.WireName, _ => property, (_, _) => property);
-        }
-        return base.AddProperty(properties);
-    }
-    public override void RenameChildElement(string oldName, string newName)
-    {
-        if (InnerChildElements.TryRemove(oldName, out var element))
-        {
-            if (element is CodeProperty removedProperty)
-            {
-                PropertiesByWireName.TryRemove(removedProperty.WireName, out _);
-            }
-            element.Name = newName;
-            AddRange(element);
-            if (element is CodeProperty propertyToAdd)
-            {
-                PropertiesByWireName.TryAdd(propertyToAdd.WireName, propertyToAdd);
-            }
-        }
-        else throw new InvalidOperationException($"The element {oldName} could not be found in the class {Name}");
-    }
-    public override void RemoveChildElementByName(params string[] names)
-    {
-        if (names == null) return;
-
-        foreach (var name in names)
-        {
-            if (InnerChildElements.TryRemove(name, out var removedElement))
-            {
-                if (removedElement is CodeProperty removedProperty)
-                    PropertiesByWireName.TryRemove(removedProperty.WireName, out _);
-            }
-            else throw new InvalidOperationException($"The element {name} could not be found in the class {Name}");
-        }
-    }
-    private string ResolveUniquePropertyName(string name)
-    {
-        if (FindPropertyByNameInTypeHierarchy(name) == null)
-            return name;
-        // the CodeClass.Name is not very useful as prefix for the property name, so keep the original name and add a number
-        var nameWithTypeName = Kind == CodeClassKind.QueryParameters ? name : Name + name.ToFirstCharacterUpperCase();
-        if (Kind != CodeClassKind.QueryParameters && FindPropertyByNameInTypeHierarchy(nameWithTypeName) == null)
-            return nameWithTypeName;
-        var i = 0;
-        while (FindPropertyByNameInTypeHierarchy(nameWithTypeName + i) != null)
-            i++;
-        return nameWithTypeName + i;
+        var result = base.AddProperty(properties);
+        foreach (var addedPropertyTuple in result.Select(x => new Tuple<CodeProperty, CodeProperty?>(x, StartBlock.GetOriginalPropertyDefinedFromBaseType(x.Name)))
+                                        .Where(static x => x.Item2 != null))
+            addedPropertyTuple.Item1.OriginalPropertyFromBaseType = addedPropertyTuple.Item2!;
+        return result;
     }
     private CodeProperty? FindPropertyByNameInTypeHierarchy(string propertyName)
     {
@@ -150,30 +85,6 @@ private string ResolveUniquePropertyName(string name)
         }
         return default;
     }
-    private CodeProperty? GetOriginalPropertyDefinedFromBaseType(string serializationName)
-    {
-        ArgumentException.ThrowIfNullOrEmpty(serializationName);
-
-        if (BaseClass is CodeClass currentParentClass)
-            if (currentParentClass.FindPropertyByWireName(serializationName) is CodeProperty currentProperty && !currentProperty.ExistsInBaseType)
-                return currentProperty;
-            else
-                return currentParentClass.GetOriginalPropertyDefinedFromBaseType(serializationName);
-        return default;
-    }
-    private CodeProperty? FindPropertyByWireName(string wireName)
-    {
-        if (!PropertiesByWireName.Any())
-            return default;
-
-        if (PropertiesByWireName.TryGetValue(wireName, out var result))
-            return result;
-        return InnerChildElements.Values.OfType<CodeClass>().Select(x => x.FindPropertyByWireName(wireName)).OfType<CodeProperty>().FirstOrDefault();
-    }
-    public bool ContainsPropertyWithWireName(string wireName)
-    {
-        return PropertiesByWireName.ContainsKey(wireName);
-    }
     public IEnumerable<CodeClass> AddInnerClass(params CodeClass[] codeClasses)
     {
         if (codeClasses == null || codeClasses.Any(x => x == null))
@@ -255,6 +166,18 @@ public CodeType? Inherits
             inherits = value;
         }
     }
+    public CodeProperty? GetOriginalPropertyDefinedFromBaseType(string propertyName)
+    {
+        ArgumentException.ThrowIfNullOrEmpty(propertyName);
+
+        if (inherits is CodeType currentInheritsType &&
+            currentInheritsType.TypeDefinition is CodeClass currentParentClass)
+            if (currentParentClass.FindChildByName<CodeProperty>(propertyName) is CodeProperty currentProperty && !currentProperty.ExistsInBaseType)
+                return currentProperty;
+            else
+                return currentParentClass.StartBlock.GetOriginalPropertyDefinedFromBaseType(propertyName);
+        return default;
+    }
     public bool InheritsFrom(CodeClass candidate)
     {
         ArgumentNullException.ThrowIfNull(candidate);

src/Kiota.Builder/KiotaBuilder.cs

@@ -2008,8 +2008,9 @@ internal static void AddSerializationMembers(CodeClass model, bool includeAdditi
                                     operation.Summary).CleanupDescription(),
                 },
             }).First();
-            foreach (var parameter in parameters)
-                AddPropertyForQueryParameter(parameter, parameterClass);
+            if (!parameterClass.Properties.Any())
+                foreach (var parameter in parameters)
+                    AddPropertyForQueryParameter(parameter, parameterClass);
 
             return parameterClass;
         }
@@ -2043,7 +2044,7 @@ private void AddPropertyForQueryParameter(OpenApiParameter parameter, CodeClass
             prop.SerializationName = parameter.Name.SanitizeParameterNameForUrlTemplate();
         }
 
-        if (!parameterClass.ContainsPropertyWithWireName(prop.WireName))
+        if (!parameterClass.ContainsMember(prop.Name))
         {
             parameterClass.AddProperty(prop);
         }

tests/Kiota.Builder.Tests/KiotaBuilderTests.cs

@@ -6016,144 +6016,4 @@ public async Task SkipsInvalidItemsProperties()
         Assert.Empty(resultClass.Properties.Where(x => x.IsOfKind(CodePropertyKind.Custom) && keysToCheck.Contains(x.Name)));
         Assert.Single(resultClass.Properties.Where(x => x.IsOfKind(CodePropertyKind.Custom) && x.Name.Equals("id", StringComparison.OrdinalIgnoreCase)));
     }
-    [Fact]
-    public async Task CleanupSymbolNameDoesNotCauseNameConflicts()
-    {
-        var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
-        await using var fs = await GetDocumentStream(@"openapi: 3.0.1
-info:
-  title: Example
-  description: Example
-  version: 1.0.1
-servers:
-  - url: https://example.org
-paths:
-  /directoryObject:
-    get:
-      responses:
-        '200':
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/entity'
-components:
-  schemas:
-    entity:
-      title: entity
-      type: object
-      required: ['type', '@type']
-      properties:
-        type:
-          type: string
-        '@type':
-          type: integer");
-        var mockLogger = new Mock<ILogger<KiotaBuilder>>();
-        var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath, IncludeAdditionalData = false }, _httpClient);
-        var document = await builder.CreateOpenApiDocumentAsync(fs);
-        var node = builder.CreateUriSpace(document);
-        var codeModel = builder.CreateSourceModel(node);
-        var resultClass = codeModel.FindChildByName<CodeClass>("Entity");
-        Assert.NotNull(resultClass);
-        Assert.Equal(2, resultClass.Properties.Select(static x => x.Name).Distinct(StringComparer.OrdinalIgnoreCase).Count());
-    }
-    [Fact]
-    public async Task CleanupSymbolNameDoesNotCauseNameConflictsWithSuperType()
-    {
-        var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
-        await using var fs = await GetDocumentStream(@"openapi: 3.0.1
-info:
-  title: Example
-  description: Example
-  version: 1.0.1
-servers:
-  - url: https://example.org
-paths:
-  /directoryObject:
-    get:
-      responses:
-        '200':
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/subtype'
-components:
-  schemas:
-    entity:
-      title: entity
-      type: object
-      required: ['@type']
-      properties:
-        '@type':
-          type: integer
-      discriminator:
-        propertyName: '@type'
-        mapping:
-          'subtype': '#/components/schemas/subtype'
-    subtype:
-      allOf:
-        - $ref: '#/components/schemas/entity'
-        - title: subtype
-          type: object
-          required: ['type', '@type']
-          properties:
-            'type':
-              type: string");
-        var mockLogger = new Mock<ILogger<KiotaBuilder>>();
-        var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath, IncludeAdditionalData = false }, _httpClient);
-        var document = await builder.CreateOpenApiDocumentAsync(fs);
-        var node = builder.CreateUriSpace(document);
-        var codeModel = builder.CreateSourceModel(node);
-        var entityClass = codeModel.FindChildByName<CodeClass>("Entity");
-        Assert.NotNull(entityClass);
-        var atType = entityClass.FindChildByName<CodeProperty>("Type");
-        Assert.Equal("@type", atType.WireName);
-        var subtypeClass = codeModel.FindChildByName<CodeClass>("Subtype");
-        Assert.NotNull(subtypeClass);
-        var type = subtypeClass.FindChildByName<CodeProperty>("SubtypeType");
-        Assert.Equal("type", type.WireName);
-        Assert.Equal("SubtypeType", type.Name);
-    }
-    [Fact]
-    public async Task CleanupSymbolNameDoesNotCauseNameConflictsInQueryParameters()
-    {
-        var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());
-        await using var fs = await GetDocumentStream(@"openapi: 3.0.1
-info:
-  title: Example
-  description: Example
-  version: 1.0.1
-servers:
-  - url: https://example.org
-paths:
-  /directoryObject:
-    get:
-      parameters:
-        - name: $select
-          in: query
-          schema:
-            type: string
-        - name: select
-          in: query
-          schema:
-            type: int64
-      responses:
-        '200':
-          content:
-            application/json:
-              schema:
-                type: string");
-        var mockLogger = new Mock<ILogger<KiotaBuilder>>();
-        var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath, IncludeAdditionalData = false }, _httpClient);
-        var document = await builder.CreateOpenApiDocumentAsync(fs);
-        var node = builder.CreateUriSpace(document);
-        var codeModel = builder.CreateSourceModel(node);
-        var parametersClass = codeModel.FindChildByName<CodeClass>("directoryObjectRequestBuilderGetQueryParameters");
-        Assert.NotNull(parametersClass);
-        var dollarSelect = parametersClass.FindChildByName<CodeProperty>("Select");
-        Assert.Equal("%24select", dollarSelect.WireName);
-        Assert.Equal("string", dollarSelect.Type.Name);
-        var select = parametersClass.FindChildByName<CodeProperty>("select0");
-        Assert.Equal("select", select.WireName);
-        Assert.Equal("int64", select.Type.Name);
-    }
 }