From df988e80f53852e901a099fa4bd14aa67bb9f3e0 Mon Sep 17 00:00:00 2001 From: "peter.wurzinger" Date: Wed, 29 May 2024 16:41:42 +0200 Subject: [PATCH 1/8] Fix 4475 by introducing fully qualified names for implementations of ITypeDefinition --- .../Writers/CSharp/CSharpConventionService.cs | 72 ++----------------- .../Writers/CSharp/CodeMethodWriter.cs | 16 +++-- .../CSharp/TypeDefinitionExtensions.cs | 34 +++++++++ 3 files changed, 48 insertions(+), 74 deletions(-) create mode 100644 src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs diff --git a/src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs b/src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs index 5a5c1309e8..c975d79c0e 100644 --- a/src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs +++ b/src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs @@ -177,7 +177,7 @@ public string GetTypeString(CodeTypeBase code, CodeElement targetElement, bool i throw new InvalidOperationException($"CSharp does not support union types, the union type {code.Name} should have been filtered out by the refiner"); if (code is CodeType currentType) { - var typeName = TranslateTypeAndAvoidUsingNamespaceSegmentNames(currentType, targetElement); + var typeName = TranslateType(currentType); var nullableSuffix = ShouldTypeHaveNullableMarker(code, typeName) && includeNullableInformation ? NullableMarkerAsString : string.Empty; var collectionPrefix = currentType.CollectionKind == CodeTypeCollectionKind.Complex && includeCollectionInformation ? "List<" : string.Empty; var collectionSuffix = currentType.CollectionKind switch @@ -196,76 +196,14 @@ public string GetTypeString(CodeTypeBase code, CodeElement targetElement, bool i throw new InvalidOperationException($"type of type {code?.GetType()} is unknown"); } - private string TranslateTypeAndAvoidUsingNamespaceSegmentNames(CodeType currentType, CodeElement targetElement) - { - var parentElementsHash = targetElement.Parent is CodeClass parentClass ? - parentClass.Methods.Select(static x => x.Name) - .Union(parentClass.Properties.Select(static x => x.Name)) - .Distinct(StringComparer.OrdinalIgnoreCase) - .ToHashSet(StringComparer.OrdinalIgnoreCase) : - new HashSet(0, StringComparer.OrdinalIgnoreCase); - - var typeName = TranslateType(currentType); - var areElementsInSameNamesSpace = DoesTypeExistsInSameNamesSpaceAsTarget(currentType, targetElement); - if (currentType.TypeDefinition != null && - ( - GetNamesInUseByNamespaceSegments(targetElement).Contains(typeName) && !areElementsInSameNamesSpace // match if elements are not in the same namespace and the type name is used in the namespace segments - || parentElementsHash.Contains(typeName) // match if type name is used in the parent elements segments - || !areElementsInSameNamesSpace && DoesTypeExistsInTargetAncestorNamespace(currentType, targetElement) // match if elements are not in the same namespace and the type exists in target ancestor namespace - || !areElementsInSameNamesSpace && DoesTypeExistsInOtherImportedNamespaces(currentType, targetElement) // match if elements is not imported already by another namespace. - ) - ) - return $"{currentType.TypeDefinition.GetImmediateParentOfType().Name}.{typeName}"; - return typeName; - } - - private static bool DoesTypeExistsInSameNamesSpaceAsTarget(CodeType currentType, CodeElement targetElement) - { - return currentType?.TypeDefinition?.GetImmediateParentOfType()?.Name.Equals(targetElement?.GetImmediateParentOfType()?.Name, StringComparison.OrdinalIgnoreCase) ?? false; - } - - private static bool DoesTypeExistsInTargetAncestorNamespace(CodeType currentType, CodeElement targetElement) - { - // Avoid type ambiguity on similarly named classes. Currently, if we have namespaces A and A.B where both namespaces have type T, - // Trying to use type A.B.T in namespace A without using a qualified name will break the build. - // Similarly, if we have type A.B.C.D.T1 that needs to be used within type A.B.C.T2, but there's also a type - // A.B.T1, using T1 in T2 will resolve A.B.T1 even if you have a using statement with A.B.C.D. - var hasChildWithName = false; - if (currentType != null && currentType.TypeDefinition != null && !currentType.IsExternal && targetElement != null) - { - var typeName = currentType.TypeDefinition.Name; - var ns = targetElement.GetImmediateParentOfType(); - var rootNs = ns?.GetRootNamespace(); - while (ns is not null && ns != rootNs && !hasChildWithName) - { - hasChildWithName = ns.GetChildElements(true).OfType().Any(c => c.Name?.Equals(typeName, StringComparison.OrdinalIgnoreCase) == true); - ns = ns.Parent is CodeNamespace n ? n : (ns.GetImmediateParentOfType()); - } - } - return hasChildWithName; - } - - private static bool DoesTypeExistsInOtherImportedNamespaces(CodeType currentType, CodeElement targetElement) - { - if (currentType.TypeDefinition is CodeClass { Parent: CodeNamespace currentTypeNamespace } codeClass) - { - var targetClass = targetElement.GetImmediateParentOfType(); - var importedNamespaces = targetClass.StartBlock.Usings - .Where(codeUsing => !codeUsing.IsExternal // 1. Are defined during generation(not external) - && codeUsing.Declaration?.TypeDefinition != null - && !codeUsing.Name.Equals(currentTypeNamespace.Name, StringComparison.OrdinalIgnoreCase)) // 2. Do not match the namespace of the current type - .Select(static codeUsing => codeUsing.Declaration!.TypeDefinition!.GetImmediateParentOfType()) - .DistinctBy(static declaredNamespace => declaredNamespace.Name); - - return importedNamespaces.Any(importedNamespace => (importedNamespace.FindChildByName(codeClass.Name, false) != null) - || (importedNamespace.FindChildByName(codeClass.Name, false) != null)); - } - return false; - } public override string TranslateType(CodeType type) { ArgumentNullException.ThrowIfNull(type); + + if (type.TypeDefinition is ITypeDefinition typeDefinition) + return typeDefinition.GetFullyQualifiedName(); + return type.Name switch { "integer" => "int", diff --git a/src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs index 66d29a759f..341e5eda25 100644 --- a/src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs +++ b/src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs @@ -105,7 +105,9 @@ private void WriteRawUrlBuilderBody(CodeClass parentClass, CodeMethod codeElemen { var rawUrlParameter = codeElement.Parameters.OfKind(CodeParameterKind.RawUrl) ?? throw new InvalidOperationException("RawUrlBuilder method should have a RawUrl parameter"); var requestAdapterProperty = parentClass.GetPropertyOfKind(CodePropertyKind.RequestAdapter) ?? throw new InvalidOperationException("RawUrlBuilder method should have a RequestAdapter property"); - writer.WriteLine($"return new {parentClass.Name.ToFirstCharacterUpperCase()}({rawUrlParameter.Name.ToFirstCharacterLowerCase()}, {requestAdapterProperty.Name.ToFirstCharacterUpperCase()});"); + + var fullName = parentClass.GetFullyQualifiedName(); + writer.WriteLine($"return new {fullName}({rawUrlParameter.Name.ToFirstCharacterLowerCase()}, {requestAdapterProperty.Name.ToFirstCharacterUpperCase()});"); } private static readonly CodePropertyTypeComparer CodePropertyTypeForwardComparer = new(); private static readonly CodePropertyTypeComparer CodePropertyTypeBackwardComparer = new(true); @@ -117,13 +119,13 @@ private void WriteFactoryMethodBodyForInheritedModel(CodeMethod codeElement, Cod { writer.WriteLine($"\"{mappedType.Key}\" => new {conventions.GetTypeString(mappedType.Value.AllTypes.First(), codeElement)}(),"); } - writer.WriteLine($"_ => new {parentClass.Name.ToFirstCharacterUpperCase()}(),"); + writer.WriteLine($"_ => new {parentClass.GetFullyQualifiedName()}(),"); writer.CloseBlock("};"); } private const string ResultVarName = "result"; private void WriteFactoryMethodBodyForUnionModel(CodeMethod codeElement, CodeClass parentClass, CodeParameter parseNodeParameter, LanguageWriter writer) { - writer.WriteLine($"var {ResultVarName} = new {parentClass.Name.ToFirstCharacterUpperCase()}();"); + writer.WriteLine($"var {ResultVarName} = new {parentClass.GetFullyQualifiedName()}();"); var includeElse = false; foreach (var property in parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) .OrderBy(static x => x, CodePropertyTypeForwardComparer) @@ -150,7 +152,7 @@ private void WriteFactoryMethodBodyForUnionModel(CodeMethod codeElement, CodeCla } private void WriteFactoryMethodBodyForIntersectionModel(CodeMethod codeElement, CodeClass parentClass, CodeParameter parseNodeParameter, LanguageWriter writer) { - writer.WriteLine($"var {ResultVarName} = new {parentClass.Name.ToFirstCharacterUpperCase()}();"); + writer.WriteLine($"var {ResultVarName} = new {parentClass.GetFullyQualifiedName()}();"); var includeElse = false; foreach (var property in parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) .Where(static x => x.Type is not CodeType propertyType || propertyType.IsCollection || propertyType.TypeDefinition is not CodeClass) @@ -202,7 +204,7 @@ private void WriteFactoryMethodBody(CodeMethod codeElement, CodeClass parentClas else if (parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForIntersectionType) WriteFactoryMethodBodyForIntersectionModel(codeElement, parentClass, parseNodeParameter, writer); else - writer.WriteLine($"return new {parentClass.Name.ToFirstCharacterUpperCase()}();"); + writer.WriteLine($"return new {parentClass.GetFullyQualifiedName()}();"); } private void WriteRequestBuilderBody(CodeClass parentClass, CodeMethod codeElement, LanguageWriter writer) { @@ -357,7 +359,7 @@ private string GetDeserializationMethodName(CodeTypeBase propType, CodeMethod me return $"GetCollectionOfObjectValues<{propertyType}>({propertyType}.CreateFromDiscriminatorValue){collectionMethod}"; } else if (currentType.TypeDefinition is CodeEnum enumType) - return $"GetEnumValue<{enumType.Name.ToFirstCharacterUpperCase()}>()"; + return $"GetEnumValue<{enumType.GetFullyQualifiedName()}>()"; } return propertyType switch { @@ -662,7 +664,7 @@ private string GetSerializationMethodName(CodeTypeBase propType, CodeMethod meth else return $"WriteCollectionOfObjectValues<{propertyType}>"; else if (currentType.TypeDefinition is CodeEnum enumType) - return $"WriteEnumValue<{enumType.Name.ToFirstCharacterUpperCase()}>"; + return $"WriteEnumValue<{enumType.GetFullyQualifiedName()}>"; } return propertyType switch diff --git a/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs b/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs new file mode 100644 index 0000000000..91e7c58e61 --- /dev/null +++ b/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs @@ -0,0 +1,34 @@ +using System; +using System.Text; +using Kiota.Builder.CodeDOM; +using Kiota.Builder.Extensions; + +namespace Kiota.Builder.Writers.CSharp; + +internal static class TypeDefinitionExtensions +{ + public static string GetFullyQualifiedName(this ITypeDefinition typeDefinition) + { + ArgumentNullException.ThrowIfNull(typeDefinition); + + var fullNameBuilder = new StringBuilder(); + return GetFullyQualifiedName(typeDefinition, fullNameBuilder).ToString(); + } + + private static StringBuilder GetFullyQualifiedName(ITypeDefinition codeClass, StringBuilder fullNameBuilder) + { + fullNameBuilder.Insert(0, codeClass.Name.ToFirstCharacterUpperCase()); + if (codeClass.Parent is CodeClass parentClass) + { + fullNameBuilder.Insert(0, '.'); + return GetFullyQualifiedName(parentClass, fullNameBuilder); + } + if (codeClass.Parent is CodeNamespace ns && !string.IsNullOrEmpty(ns.Name)) + { + fullNameBuilder.Insert(0, '.'); + fullNameBuilder.Insert(0, ns.Name); + } + + return fullNameBuilder; + } +} From f793ece83ce50d8d7597a67de5c4d37dcdbd6fa3 Mon Sep 17 00:00:00 2001 From: "peter.wurzinger" Date: Wed, 29 May 2024 17:05:17 +0200 Subject: [PATCH 2/8] Fix existing tests --- .../Writers/CLI/CliCodeMethodWriterTests.cs | 20 +++++++++---------- .../Writers/CSharp/CodeMethodWriterTests.cs | 6 +++--- .../Writers/CSharp/CodePropertyWriterTests.cs | 8 ++++---- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/Kiota.Builder.Tests/Writers/CLI/CliCodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/CLI/CliCodeMethodWriterTests.cs index ab5d455c24..1755ff0519 100644 --- a/tests/Kiota.Builder.Tests/Writers/CLI/CliCodeMethodWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/CLI/CliCodeMethodWriterTests.cs @@ -241,7 +241,7 @@ public void WritesIndexerCommands() writer.Write(method); var result = tw.ToString(); - Assert.Contains("var builder = new TestClass", result); + Assert.Contains("var builder = new Test.Name.Sub.TestClass", result); Assert.Contains("var commands = new List();", result); Assert.Contains("commands.Add(builder.BuildTestMethod1());", result); Assert.Contains("commands.AddRange(builder.BuildTestMethod2());", result); @@ -325,7 +325,7 @@ public void WritesMatchingIndexerCommandsIntoExecutableCommand() writer.Write(method); var result = tw.ToString(); - Assert.Contains("var testItemIdx = new TestItemRequestBuilder();", result); + Assert.Contains("var testItemIdx = new Test.Name.Sub.TestItemRequestBuilder();", result); Assert.Contains("var command = testItemIdx.BuildTestMethod1();", result); Assert.Contains("var cmds = testItemIdx.BuildTestMethod2();", result); Assert.DoesNotContain("execCommands.AddRange(cmds.Item1)", result); @@ -418,12 +418,12 @@ public void WritesMatchingIndexerCommandsIntoContainerCommand() writer.Write(method); var result = tw.ToString(); - Assert.Contains("var testItemIndexer = new TestIndexItemRequestBuilder();", result); + Assert.Contains("var testItemIndexer = new Test.Name.Sub.TestIndexItemRequestBuilder();", result); Assert.Contains("var command = testItemIndexer.BuildTestMethod1();", result); Assert.Contains("var cmds = testItemIndexer.BuildTestMethod2();", result); Assert.DoesNotContain("execCommands.AddRange(cmds.Item1);", result); Assert.Contains("nonExecCommands.AddRange(cmds.Item2);", result); - Assert.Contains("var builder = new TestNavItemRequestBuilder", result); + Assert.Contains("var builder = new Test.TestNavItemRequestBuilder", result); Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod11());", result); Assert.Contains("return command;", result); Assert.DoesNotContain("nonExecCommands.Add(builder.BuildTestMethod3());", result); @@ -553,7 +553,7 @@ public void WritesNavCommandThatSkipsReusedNavCommandInstance() var result = tw.ToString(); Assert.Contains("var command = new Command(\"user\");", result); - Assert.Contains("var builder = new TestNavItemRequestBuilder();", result); + Assert.Contains("var builder = new Test.TestNavItemRequestBuilder();", result); Assert.Contains("execCommands.Add(builder.BuildExecutableTestMethod());", result); Assert.Contains("return command;", result); Assert.DoesNotContain("BuildNavTestMethod", result); @@ -596,7 +596,7 @@ public void WritesContainerCommands() var result = tw.ToString(); Assert.Contains("var command = new Command(\"user\");", result); - Assert.Contains("var builder = new TestClass1", result); + Assert.Contains("var builder = new Test.Name.Sub1.Sub2.TestClass1", result); Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod1());", result); Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod2());", result); Assert.Contains("return command;", result); @@ -637,7 +637,7 @@ public void WritesRequestBuilderWithParametersCommands() var result = tw.ToString(); Assert.Contains("var command = new Command(\"user\");", result); - Assert.Contains("var builder = new TestClass1", result); + Assert.Contains("var builder = new Test.Name.Sub1.Sub2.TestClass1", result); Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod1());", result); Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod2());", result); Assert.Contains("return command;", result); @@ -1085,7 +1085,7 @@ public void WritesExecutableCommandForPostRequestWithModelBody() Assert.Contains("command.AddOption(bodyOption);", result); Assert.Contains("var body = invocationContext.ParseResult.GetValueForOption(bodyOption) ?? string.Empty;", result); Assert.Contains("using var stream = new MemoryStream(Encoding.UTF8.GetBytes(body));", result); - Assert.Contains("var model = parseNode.GetObjectValue(Content.CreateFromDiscriminatorValue);", result); + Assert.Contains("var model = parseNode.GetObjectValue(Test.Content.CreateFromDiscriminatorValue);", result); Assert.Contains("if (model is null)", result); Assert.Contains("Console.Error.WriteLine(\"No model data to send.\")", result); Assert.Contains("var requestInfo = CreatePostRequestInformation", result); @@ -1171,7 +1171,7 @@ public void WritesExecutableCommandForPostRequestWithModelBodyAndContentType() Assert.Contains("var body = invocationContext.ParseResult.GetValueForOption(bodyOption) ?? string.Empty;", result); Assert.Contains("var contentType = invocationContext.ParseResult.GetValueForOption(contentTypeOption);", result); Assert.Contains("using var stream = new MemoryStream(Encoding.UTF8.GetBytes(body));", result); - Assert.Contains("var model = parseNode.GetObjectValue(Content.CreateFromDiscriminatorValue);", result); + Assert.Contains("var model = parseNode.GetObjectValue(Test.Content.CreateFromDiscriminatorValue);", result); Assert.Contains("if (model is null)", result); Assert.Contains("Console.Error.WriteLine(\"No model data to send.\")", result); Assert.Contains("var requestInfo = CreatePostRequestInformation(model, contentType", result); @@ -1242,7 +1242,7 @@ public void WritesExecutableCommandForPostRequestWithCollectionModel() Assert.Contains("command.AddOption(bodyOption);", result); Assert.Contains("var body = invocationContext.ParseResult.GetValueForOption(bodyOption) ?? string.Empty;", result); Assert.Contains("using var stream = new MemoryStream(Encoding.UTF8.GetBytes(body));", result); - Assert.Contains("var model = parseNode.GetCollectionOfObjectValues(Content.CreateFromDiscriminatorValue)?.ToList();", result); + Assert.Contains("var model = parseNode.GetCollectionOfObjectValues(Test.Content.CreateFromDiscriminatorValue)?.ToList();", result); Assert.Contains("if (model is null)", result); Assert.Contains("Console.Error.WriteLine(\"No model data to send.\")", result); Assert.Contains("var requestInfo = CreatePostRequestInformation", result); diff --git a/tests/Kiota.Builder.Tests/Writers/CSharp/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/CSharp/CodeMethodWriterTests.cs index ae87bd46ba..2fe8182ed3 100644 --- a/tests/Kiota.Builder.Tests/Writers/CSharp/CodeMethodWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/CSharp/CodeMethodWriterTests.cs @@ -738,7 +738,7 @@ public void WritesModelFactoryBodyAndDisambiguateAmbiguousImportedTypes() Assert.Contains("var mappingValue = parseNode.GetChildNode(\"@odata.type\")?.GetStringValue()", result); Assert.Contains("return mappingValue switch", result); Assert.Contains("\"namespaceLevelOne.ConflictingModel\" => new namespaceLevelOne.ConflictingModel(),", result); //Assert the disambiguation happens due to the enum imported - Assert.Contains("_ => new ConflictingModelBaseClass()", result); + Assert.Contains("_ => new models.ConflictingModelBaseClass()", result); AssertExtensions.CurlyBracesAreClosed(result); } [Fact] @@ -1532,7 +1532,7 @@ public void WritesConstructorWithEnumValue() writer.Write(method); var result = tw.ToString(); Assert.Contains(parentClass.Name.ToFirstCharacterUpperCase(), result); - Assert.Contains($"{propName.ToFirstCharacterUpperCase()} = {codeEnum.Name.ToFirstCharacterUpperCase()}.{defaultValue.CleanupSymbolName()}", result);//ensure symbol is cleaned up + Assert.Contains($"{propName.ToFirstCharacterUpperCase()} = {modelsNamespace.Name}.{codeEnum.Name.ToFirstCharacterUpperCase()}.{defaultValue.CleanupSymbolName()}", result);//ensure symbol is cleaned up } [Fact] public void WritesConstructorAndIncludesSanitizedEnumValue() @@ -1557,7 +1557,7 @@ public void WritesConstructorAndIncludesSanitizedEnumValue() var result = tw.ToString(); Assert.Contains(parentClass.Name.ToFirstCharacterUpperCase(), result); Assert.Contains("PictureSize.Slash", result);//ensure symbol is cleaned up - Assert.Contains($"{propName.ToFirstCharacterUpperCase()} = {codeEnum.Name.ToFirstCharacterUpperCase()}.{defaultValue.CleanupSymbolName()}", result);//ensure symbol is cleaned up + Assert.Contains($"{propName.ToFirstCharacterUpperCase()} = {modelsNamespace.Name}.{codeEnum.Name.ToFirstCharacterUpperCase()}.{defaultValue.CleanupSymbolName()}", result);//ensure symbol is cleaned up } [Fact] public void WritesConstructorAndDisambiguatesEnumType() diff --git a/tests/Kiota.Builder.Tests/Writers/CSharp/CodePropertyWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/CSharp/CodePropertyWriterTests.cs index a12cd43520..96524a9e7f 100644 --- a/tests/Kiota.Builder.Tests/Writers/CSharp/CodePropertyWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/CSharp/CodePropertyWriterTests.cs @@ -74,7 +74,7 @@ public void WritesRequestBuilder() writer.Write(property); var result = tw.ToString(); Assert.Contains("get =>", result); - Assert.Contains($"new {TypeName}", result); + Assert.Contains($"new {rootNamespace.Name}.{TypeName}", result); Assert.Contains("RequestAdapter", result); Assert.Contains("PathParameters", result); } @@ -103,7 +103,7 @@ public void MapsCustomPropertiesToBackingStore() property.Kind = CodePropertyKind.Custom; writer.Write(property); var result = tw.ToString(); - Assert.Contains("get { return BackingStore?.Get(\"propertyName\"); }", result); + Assert.Contains("get { return BackingStore?.Get<" + rootNamespace.Name + ".SomeCustomClass>(\"propertyName\"); }", result); Assert.Contains("set { BackingStore?.Set(\"propertyName\", value);", result); } [Fact] @@ -113,7 +113,7 @@ public void MapsAdditionalDataPropertiesToBackingStore() property.Kind = CodePropertyKind.AdditionalData; writer.Write(property); var result = tw.ToString(); - Assert.Contains("get { return BackingStore.Get(\"propertyName\") ?? new Dictionary(); }", result); + Assert.Contains("get { return BackingStore.Get<" + rootNamespace.Name + ".SomeCustomClass>(\"propertyName\") ?? new Dictionary(); }", result); Assert.Contains("set { BackingStore.Set(\"propertyName\", value);", result); } [Fact] @@ -207,7 +207,7 @@ public void DisambiguateAmbiguousImportedTypes() // Assert: properties types are disambiguated. Assert.Contains("namespaceLevelOne.SomeCustomClass", result); - Assert.Contains("defaultNamespace.SomeCustomClass", result); + Assert.Contains($"{rootNamespace.Name}.SomeCustomClass", result); } [Fact] public void WritesDeprecationInformation() From 9de4bda2a5d2cbeadb60c6a502c91ed33220d380 Mon Sep 17 00:00:00 2001 From: Peter Wurzinger Date: Fri, 31 May 2024 22:00:10 +0200 Subject: [PATCH 3/8] Rename GetFullyQualifiedName to GetFullName and refine some edge cases --- .../Writers/CSharp/CSharpConventionService.cs | 2 +- .../Writers/CSharp/CodeMethodWriter.cs | 14 ++++---- .../CSharp/TypeDefinitionExtensions.cs | 32 ++++++++++++------- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs b/src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs index c975d79c0e..9522fa678e 100644 --- a/src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs +++ b/src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs @@ -202,7 +202,7 @@ public override string TranslateType(CodeType type) ArgumentNullException.ThrowIfNull(type); if (type.TypeDefinition is ITypeDefinition typeDefinition) - return typeDefinition.GetFullyQualifiedName(); + return typeDefinition.GetFullName(); return type.Name switch { diff --git a/src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs index 341e5eda25..796e6a91ac 100644 --- a/src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs +++ b/src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs @@ -106,7 +106,7 @@ private void WriteRawUrlBuilderBody(CodeClass parentClass, CodeMethod codeElemen var rawUrlParameter = codeElement.Parameters.OfKind(CodeParameterKind.RawUrl) ?? throw new InvalidOperationException("RawUrlBuilder method should have a RawUrl parameter"); var requestAdapterProperty = parentClass.GetPropertyOfKind(CodePropertyKind.RequestAdapter) ?? throw new InvalidOperationException("RawUrlBuilder method should have a RequestAdapter property"); - var fullName = parentClass.GetFullyQualifiedName(); + var fullName = parentClass.GetFullName(); writer.WriteLine($"return new {fullName}({rawUrlParameter.Name.ToFirstCharacterLowerCase()}, {requestAdapterProperty.Name.ToFirstCharacterUpperCase()});"); } private static readonly CodePropertyTypeComparer CodePropertyTypeForwardComparer = new(); @@ -119,13 +119,13 @@ private void WriteFactoryMethodBodyForInheritedModel(CodeMethod codeElement, Cod { writer.WriteLine($"\"{mappedType.Key}\" => new {conventions.GetTypeString(mappedType.Value.AllTypes.First(), codeElement)}(),"); } - writer.WriteLine($"_ => new {parentClass.GetFullyQualifiedName()}(),"); + writer.WriteLine($"_ => new {parentClass.GetFullName()}(),"); writer.CloseBlock("};"); } private const string ResultVarName = "result"; private void WriteFactoryMethodBodyForUnionModel(CodeMethod codeElement, CodeClass parentClass, CodeParameter parseNodeParameter, LanguageWriter writer) { - writer.WriteLine($"var {ResultVarName} = new {parentClass.GetFullyQualifiedName()}();"); + writer.WriteLine($"var {ResultVarName} = new {parentClass.GetFullName()}();"); var includeElse = false; foreach (var property in parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) .OrderBy(static x => x, CodePropertyTypeForwardComparer) @@ -152,7 +152,7 @@ private void WriteFactoryMethodBodyForUnionModel(CodeMethod codeElement, CodeCla } private void WriteFactoryMethodBodyForIntersectionModel(CodeMethod codeElement, CodeClass parentClass, CodeParameter parseNodeParameter, LanguageWriter writer) { - writer.WriteLine($"var {ResultVarName} = new {parentClass.GetFullyQualifiedName()}();"); + writer.WriteLine($"var {ResultVarName} = new {parentClass.GetFullName()}();"); var includeElse = false; foreach (var property in parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) .Where(static x => x.Type is not CodeType propertyType || propertyType.IsCollection || propertyType.TypeDefinition is not CodeClass) @@ -204,7 +204,7 @@ private void WriteFactoryMethodBody(CodeMethod codeElement, CodeClass parentClas else if (parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForIntersectionType) WriteFactoryMethodBodyForIntersectionModel(codeElement, parentClass, parseNodeParameter, writer); else - writer.WriteLine($"return new {parentClass.GetFullyQualifiedName()}();"); + writer.WriteLine($"return new {parentClass.GetFullName()}();"); } private void WriteRequestBuilderBody(CodeClass parentClass, CodeMethod codeElement, LanguageWriter writer) { @@ -359,7 +359,7 @@ private string GetDeserializationMethodName(CodeTypeBase propType, CodeMethod me return $"GetCollectionOfObjectValues<{propertyType}>({propertyType}.CreateFromDiscriminatorValue){collectionMethod}"; } else if (currentType.TypeDefinition is CodeEnum enumType) - return $"GetEnumValue<{enumType.GetFullyQualifiedName()}>()"; + return $"GetEnumValue<{enumType.GetFullName()}>()"; } return propertyType switch { @@ -664,7 +664,7 @@ private string GetSerializationMethodName(CodeTypeBase propType, CodeMethod meth else return $"WriteCollectionOfObjectValues<{propertyType}>"; else if (currentType.TypeDefinition is CodeEnum enumType) - return $"WriteEnumValue<{enumType.GetFullyQualifiedName()}>"; + return $"WriteEnumValue<{enumType.GetFullName()}>"; } return propertyType switch diff --git a/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs b/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs index 91e7c58e61..6fa8eaa98c 100644 --- a/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs +++ b/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs @@ -7,28 +7,38 @@ namespace Kiota.Builder.Writers.CSharp; internal static class TypeDefinitionExtensions { - public static string GetFullyQualifiedName(this ITypeDefinition typeDefinition) + public static string GetFullName(this ITypeDefinition typeDefinition) { ArgumentNullException.ThrowIfNull(typeDefinition); var fullNameBuilder = new StringBuilder(); - return GetFullyQualifiedName(typeDefinition, fullNameBuilder).ToString(); + return AppendTypeName(typeDefinition, fullNameBuilder).ToString(); } - private static StringBuilder GetFullyQualifiedName(ITypeDefinition codeClass, StringBuilder fullNameBuilder) + private static StringBuilder AppendTypeName(ITypeDefinition typeDefinition, StringBuilder fullNameBuilder) { - fullNameBuilder.Insert(0, codeClass.Name.ToFirstCharacterUpperCase()); - if (codeClass.Parent is CodeClass parentClass) + if (string.IsNullOrEmpty(typeDefinition.Name)) + throw new ArgumentException("Cannot append a full name for a type without a name.", nameof(typeDefinition)); + + fullNameBuilder.Insert(0, typeDefinition.Name.ToFirstCharacterUpperCase()); + if (typeDefinition.Parent is null) + return fullNameBuilder; + + if (typeDefinition.Parent is ITypeDefinition parentTypeDefinition) { fullNameBuilder.Insert(0, '.'); - return GetFullyQualifiedName(parentClass, fullNameBuilder); + return AppendTypeName(parentTypeDefinition, fullNameBuilder); } - if (codeClass.Parent is CodeNamespace ns && !string.IsNullOrEmpty(ns.Name)) + else if (typeDefinition.Parent is CodeNamespace codeNamespace) { - fullNameBuilder.Insert(0, '.'); - fullNameBuilder.Insert(0, ns.Name); - } + if (!string.IsNullOrEmpty(codeNamespace.Name)) + fullNameBuilder.Insert(0, $"{codeNamespace.Name}."); - return fullNameBuilder; + return fullNameBuilder; + } + else + { + throw new InvalidOperationException($"Type {typeDefinition.Name} contains an invalid parent of type {typeDefinition.Parent.GetType().FullName}."); + } } } From 01a4353dc80e2ebdc0e2f7bd2b0ab23a312a6f44 Mon Sep 17 00:00:00 2001 From: Peter Wurzinger Date: Fri, 31 May 2024 22:00:28 +0200 Subject: [PATCH 4/8] Tests for TypeDefinitionExtensions --- .../CSharp/TypeDefinitionExtensionsTests.cs | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 tests/Kiota.Builder.Tests/Writers/CSharp/TypeDefinitionExtensionsTests.cs diff --git a/tests/Kiota.Builder.Tests/Writers/CSharp/TypeDefinitionExtensionsTests.cs b/tests/Kiota.Builder.Tests/Writers/CSharp/TypeDefinitionExtensionsTests.cs new file mode 100644 index 0000000000..8425b73e89 --- /dev/null +++ b/tests/Kiota.Builder.Tests/Writers/CSharp/TypeDefinitionExtensionsTests.cs @@ -0,0 +1,140 @@ +using System; +using Kiota.Builder.CodeDOM; +using Kiota.Builder.Writers.CSharp; + +using Xunit; + +namespace Kiota.Builder.Tests.Writers.CSharp; + +public sealed class TypeDefinitionExtensionsTests +{ + [Fact] + public void ReturnsFullNameForTypeWithoutNamespace() + { + var rootNamespace = CodeNamespace.InitRootNamespace(); + var myClass = new CodeClass + { + Name = "myClass" + }; + rootNamespace.AddClass(myClass); + + var fullName = TypeDefinitionExtensions.GetFullName(myClass); + + Assert.Equal("MyClass", fullName); + } + + [Fact] + public void ReturnsFullNameForTypeInNamespace() + { + var rootNamespace = CodeNamespace.InitRootNamespace(); + + var myNamespace = rootNamespace.AddNamespace("MyNamespace"); + var myClass = new CodeClass + { + Name = "myClass", + }; + myNamespace.AddClass(myClass); + + var fullName = TypeDefinitionExtensions.GetFullName(myClass); + + Assert.Equal("MyNamespace.MyClass", fullName); + } + + [Fact] + public void ReturnsFullNameForNestedTypes() + { + var rootNamespace = CodeNamespace.InitRootNamespace(); + + var myNamespace = rootNamespace.AddNamespace("MyNamespace"); + + var myParentClass = new CodeClass + { + Name = "myParentClass" + }; + myNamespace.AddClass(myParentClass); + + var myNestedClass = new CodeClass + { + Name = "myNestedClass", + }; + myParentClass.AddInnerClass(myNestedClass); + + var parentClassFullName = TypeDefinitionExtensions.GetFullName(myParentClass); + var nestedClassFullName = TypeDefinitionExtensions.GetFullName(myNestedClass); + + Assert.Equal("MyNamespace.MyParentClass", parentClassFullName); + Assert.Equal("MyNamespace.MyParentClass.MyNestedClass", nestedClassFullName); + } + + [Fact] + public void ThrowsIfTypeIsNull() + { + Assert.Throws("typeDefinition", () => TypeDefinitionExtensions.GetFullName(null!)); + } + + [Theory] + [InlineData("")] + [InlineData(null)] + public void ThrowsIfTypeDoesNotHaveAName(string typeName) + { + var myClass = new CodeClass + { + Name = typeName + }; + + Assert.Throws("typeDefinition", () => TypeDefinitionExtensions.GetFullName(myClass)); + } + + [Fact] + public void ThrowsIfTypesParentIsInvalid() + { + var myClass = new CodeClass + { + Name = "myClass", + Parent = new CodeConstant() + }; + + Assert.Throws(() => TypeDefinitionExtensions.GetFullName(myClass)); + } + + [Fact] + public void CapitalizesTypeNamesInTypeHierarchyButNotTheNamespace() + { + var rootNamespace = CodeNamespace.InitRootNamespace(); + var myNamespace = rootNamespace.AddNamespace("myNamespace"); + + var myParentClass = new CodeClass + { + Name = "myParentClass" + }; + myNamespace.AddClass(myParentClass); + + var myNestedClass = new CodeClass + { + Name = "myNestedClass", + }; + myParentClass.AddInnerClass(myNestedClass); + + var nestedClassFullName = TypeDefinitionExtensions.GetFullName(myNestedClass); + + Assert.Equal("myNamespace.MyParentClass.MyNestedClass", nestedClassFullName); + } + + [Fact] + public void DoesNotAppendNamespaceSegmentIfNamespaceNameIsEmpty() + { + var rootNamespace = CodeNamespace.InitRootNamespace(); + var myNamespace = rootNamespace.AddNamespace("ThisWillBeEmpty"); + myNamespace.Name = ""; + + var myClass = new CodeClass + { + Name = "myClass" + }; + myNamespace.AddClass(myClass); + + var fullName = TypeDefinitionExtensions.GetFullName(myClass); + + Assert.Equal("MyClass", fullName); + } +} From 69943e574c8a312bc544b0b6941e1cd9ccaff3b9 Mon Sep 17 00:00:00 2001 From: "peter.wurzinger" Date: Mon, 3 Jun 2024 12:07:36 +0200 Subject: [PATCH 5/8] Change encoding to UTF8wBOM --- .../Writers/CSharp/TypeDefinitionExtensionsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Kiota.Builder.Tests/Writers/CSharp/TypeDefinitionExtensionsTests.cs b/tests/Kiota.Builder.Tests/Writers/CSharp/TypeDefinitionExtensionsTests.cs index 8425b73e89..41c397b34f 100644 --- a/tests/Kiota.Builder.Tests/Writers/CSharp/TypeDefinitionExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/CSharp/TypeDefinitionExtensionsTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using Kiota.Builder.CodeDOM; using Kiota.Builder.Writers.CSharp; From a2ae9110b7652a0d69e7ae74cf4f9cb61f4c684b Mon Sep 17 00:00:00 2001 From: Peter Wurzinger Date: Mon, 3 Jun 2024 16:55:32 +0200 Subject: [PATCH 6/8] Accept suggestion of using a switch statement to distinguish nested code elements Co-authored-by: Andrew Omondi --- .../CSharp/TypeDefinitionExtensions.cs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs b/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs index 6fa8eaa98c..b96d8270db 100644 --- a/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs +++ b/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs @@ -21,24 +21,24 @@ private static StringBuilder AppendTypeName(ITypeDefinition typeDefinition, Stri throw new ArgumentException("Cannot append a full name for a type without a name.", nameof(typeDefinition)); fullNameBuilder.Insert(0, typeDefinition.Name.ToFirstCharacterUpperCase()); - if (typeDefinition.Parent is null) - return fullNameBuilder; - - if (typeDefinition.Parent is ITypeDefinition parentTypeDefinition) - { - fullNameBuilder.Insert(0, '.'); - return AppendTypeName(parentTypeDefinition, fullNameBuilder); - } - else if (typeDefinition.Parent is CodeNamespace codeNamespace) + switch (typeDefinition.Parent) { - if (!string.IsNullOrEmpty(codeNamespace.Name)) - fullNameBuilder.Insert(0, $"{codeNamespace.Name}."); + case null: + return fullNameBuilder; + case ITypeDefinition parentTypeDefinition: + { + fullNameBuilder.Insert(0, '.'); + return AppendTypeName(parentTypeDefinition, fullNameBuilder); + } + case CodeNamespace codeNamespace: + { + if (!string.IsNullOrEmpty(codeNamespace.Name)) + fullNameBuilder.Insert(0, $"{codeNamespace.Name}."); - return fullNameBuilder; - } - else - { - throw new InvalidOperationException($"Type {typeDefinition.Name} contains an invalid parent of type {typeDefinition.Parent.GetType().FullName}."); + return fullNameBuilder; + } + default: + throw new InvalidOperationException($"Type {typeDefinition.Name} contains an invalid parent of type {typeDefinition.Parent.GetType().FullName}."); } } } From 6e602bbbe786a8d2a5eef4b942f8e5e615ec700b Mon Sep 17 00:00:00 2001 From: "peter.wurzinger" Date: Mon, 3 Jun 2024 17:02:15 +0200 Subject: [PATCH 7/8] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34e10fa314..197c5d5e4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixes a bug where indexers in include/exclude patters were not normalized if the indexer was the last segment without a slash at the end [#4715](https://github.com/microsoft/kiota/issues/4715) - Fixes a bug where CLI generation doesnot handle parameters of type string array. [#4707](https://github.com/microsoft/kiota/issues/4707) - Fixed a bug where models would not be created when a multipart content schema existed with no encoding [#4734](https://github.com/microsoft/kiota/issues/4734) +- Types generated by Kiota are now referenced with their full name to avoid namespace ambiguities [#4475](https://github.com/microsoft/kiota/issues/4475) ## [1.14.0] - 2024-05-02 From 4ea2012075d97eaeeaf48aef8d57985e908a22da Mon Sep 17 00:00:00 2001 From: "peter.wurzinger" Date: Tue, 4 Jun 2024 08:50:09 +0200 Subject: [PATCH 8/8] Format whitespaces according to dotnet format --- .../Writers/CSharp/TypeDefinitionExtensions.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs b/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs index b96d8270db..c6ef2fac85 100644 --- a/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs +++ b/src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs @@ -26,17 +26,17 @@ private static StringBuilder AppendTypeName(ITypeDefinition typeDefinition, Stri case null: return fullNameBuilder; case ITypeDefinition parentTypeDefinition: - { - fullNameBuilder.Insert(0, '.'); - return AppendTypeName(parentTypeDefinition, fullNameBuilder); - } + { + fullNameBuilder.Insert(0, '.'); + return AppendTypeName(parentTypeDefinition, fullNameBuilder); + } case CodeNamespace codeNamespace: - { - if (!string.IsNullOrEmpty(codeNamespace.Name)) - fullNameBuilder.Insert(0, $"{codeNamespace.Name}."); + { + if (!string.IsNullOrEmpty(codeNamespace.Name)) + fullNameBuilder.Insert(0, $"{codeNamespace.Name}."); - return fullNameBuilder; - } + return fullNameBuilder; + } default: throw new InvalidOperationException($"Type {typeDefinition.Name} contains an invalid parent of type {typeDefinition.Parent.GetType().FullName}."); }