Skip to content

Commit

Permalink
Revert "Merge pull request microsoft#4751 from peterwurzinger/fix/4475"
Browse files Browse the repository at this point in the history
This reverts commit c05fb25.
  • Loading branch information
0xced committed Jun 11, 2024
1 parent 2bb3e51 commit c885247
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 216 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ 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 doesn't 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)
- Fixes a bug where warnings about discriminator not being inherited were generated [#4761](https://github.com/microsoft/kiota/issues/4761)
- Fix scalar member composed type serialization in PHP [#2827](https://github.com/microsoft/kiota/issues/2827)
- Trims unused components from output openApi document when generating plugins [#4672](https://github.com/microsoft/kiota/issues/4672)
Expand Down
72 changes: 67 additions & 5 deletions src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = TranslateType(currentType);
var typeName = TranslateTypeAndAvoidUsingNamespaceSegmentNames(currentType, targetElement);
var nullableSuffix = ShouldTypeHaveNullableMarker(code, typeName) && includeNullableInformation ? NullableMarkerAsString : string.Empty;
var collectionPrefix = currentType.CollectionKind == CodeTypeCollectionKind.Complex && includeCollectionInformation ? "List<" : string.Empty;
var collectionSuffix = currentType.CollectionKind switch
Expand All @@ -196,14 +196,76 @@ 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<string>(0, StringComparer.OrdinalIgnoreCase);

public override string TranslateType(CodeType type)
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<CodeNamespace>().Name}.{typeName}";
return typeName;
}

private static bool DoesTypeExistsInSameNamesSpaceAsTarget(CodeType currentType, CodeElement targetElement)
{
ArgumentNullException.ThrowIfNull(type);
return currentType?.TypeDefinition?.GetImmediateParentOfType<CodeNamespace>()?.Name.Equals(targetElement?.GetImmediateParentOfType<CodeNamespace>()?.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<CodeNamespace>();
var rootNs = ns?.GetRootNamespace();
while (ns is not null && ns != rootNs && !hasChildWithName)
{
hasChildWithName = ns.GetChildElements(true).OfType<CodeClass>().Any(c => c.Name?.Equals(typeName, StringComparison.OrdinalIgnoreCase) == true);
ns = ns.Parent is CodeNamespace n ? n : (ns.GetImmediateParentOfType<CodeNamespace>());
}
}
return hasChildWithName;
}

if (type.TypeDefinition is ITypeDefinition typeDefinition)
return typeDefinition.GetFullName();
private static bool DoesTypeExistsInOtherImportedNamespaces(CodeType currentType, CodeElement targetElement)
{
if (currentType.TypeDefinition is CodeClass { Parent: CodeNamespace currentTypeNamespace } codeClass)
{
var targetClass = targetElement.GetImmediateParentOfType<CodeClass>();
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<CodeNamespace>())
.DistinctBy(static declaredNamespace => declaredNamespace.Name);

return importedNamespaces.Any(importedNamespace => (importedNamespace.FindChildByName<CodeClass>(codeClass.Name, false) != null)
|| (importedNamespace.FindChildByName<CodeEnum>(codeClass.Name, false) != null));
}
return false;
}

public override string TranslateType(CodeType type)
{
ArgumentNullException.ThrowIfNull(type);
return type.Name switch
{
"integer" => "int",
Expand Down
16 changes: 7 additions & 9 deletions src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,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.GetFullName();
writer.WriteLine($"return new {fullName}({rawUrlParameter.Name.ToFirstCharacterLowerCase()}, {requestAdapterProperty.Name.ToFirstCharacterUpperCase()});");
writer.WriteLine($"return new {parentClass.Name.ToFirstCharacterUpperCase()}({rawUrlParameter.Name.ToFirstCharacterLowerCase()}, {requestAdapterProperty.Name.ToFirstCharacterUpperCase()});");
}
private static readonly CodePropertyTypeComparer CodePropertyTypeForwardComparer = new();
private static readonly CodePropertyTypeComparer CodePropertyTypeBackwardComparer = new(true);
Expand All @@ -119,13 +117,13 @@ private void WriteFactoryMethodBodyForInheritedModel(CodeMethod codeElement, Cod
{
writer.WriteLine($"\"{mappedType.Key}\" => new {conventions.GetTypeString(mappedType.Value.AllTypes.First(), codeElement)}(),");
}
writer.WriteLine($"_ => new {parentClass.GetFullName()}(),");
writer.WriteLine($"_ => new {parentClass.Name.ToFirstCharacterUpperCase()}(),");
writer.CloseBlock("};");
}
private const string ResultVarName = "result";
private void WriteFactoryMethodBodyForUnionModel(CodeMethod codeElement, CodeClass parentClass, CodeParameter parseNodeParameter, LanguageWriter writer)
{
writer.WriteLine($"var {ResultVarName} = new {parentClass.GetFullName()}();");
writer.WriteLine($"var {ResultVarName} = new {parentClass.Name.ToFirstCharacterUpperCase()}();");
var includeElse = false;
foreach (var property in parentClass.GetPropertiesOfKind(CodePropertyKind.Custom)
.OrderBy(static x => x, CodePropertyTypeForwardComparer)
Expand All @@ -152,7 +150,7 @@ private void WriteFactoryMethodBodyForUnionModel(CodeMethod codeElement, CodeCla
}
private void WriteFactoryMethodBodyForIntersectionModel(CodeMethod codeElement, CodeClass parentClass, CodeParameter parseNodeParameter, LanguageWriter writer)
{
writer.WriteLine($"var {ResultVarName} = new {parentClass.GetFullName()}();");
writer.WriteLine($"var {ResultVarName} = new {parentClass.Name.ToFirstCharacterUpperCase()}();");
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)
Expand Down Expand Up @@ -204,7 +202,7 @@ private void WriteFactoryMethodBody(CodeMethod codeElement, CodeClass parentClas
else if (parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForIntersectionType)
WriteFactoryMethodBodyForIntersectionModel(codeElement, parentClass, parseNodeParameter, writer);
else
writer.WriteLine($"return new {parentClass.GetFullName()}();");
writer.WriteLine($"return new {parentClass.Name.ToFirstCharacterUpperCase()}();");
}
private void WriteRequestBuilderBody(CodeClass parentClass, CodeMethod codeElement, LanguageWriter writer)
{
Expand Down Expand Up @@ -359,7 +357,7 @@ private string GetDeserializationMethodName(CodeTypeBase propType, CodeMethod me
return $"GetCollectionOfObjectValues<{propertyType}>({propertyType}.CreateFromDiscriminatorValue){collectionMethod}";
}
else if (currentType.TypeDefinition is CodeEnum enumType)
return $"GetEnumValue<{enumType.GetFullName()}>()";
return $"GetEnumValue<{enumType.Name.ToFirstCharacterUpperCase()}>()";
}
return propertyType switch
{
Expand Down Expand Up @@ -664,7 +662,7 @@ private string GetSerializationMethodName(CodeTypeBase propType, CodeMethod meth
else
return $"WriteCollectionOfObjectValues<{propertyType}>";
else if (currentType.TypeDefinition is CodeEnum enumType)
return $"WriteEnumValue<{enumType.GetFullName()}>";
return $"WriteEnumValue<{enumType.Name.ToFirstCharacterUpperCase()}>";

}
return propertyType switch
Expand Down
44 changes: 0 additions & 44 deletions src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs

This file was deleted.

20 changes: 10 additions & 10 deletions tests/Kiota.Builder.Tests/Writers/CLI/CliCodeMethodWriterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public void WritesIndexerCommands()
writer.Write(method);
var result = tw.ToString();

Assert.Contains("var builder = new Test.Name.Sub.TestClass", result);
Assert.Contains("var builder = new TestClass", result);
Assert.Contains("var commands = new List<Command>();", result);
Assert.Contains("commands.Add(builder.BuildTestMethod1());", result);
Assert.Contains("commands.AddRange(builder.BuildTestMethod2());", result);
Expand Down Expand Up @@ -325,7 +325,7 @@ public void WritesMatchingIndexerCommandsIntoExecutableCommand()
writer.Write(method);
var result = tw.ToString();

Assert.Contains("var testItemIdx = new Test.Name.Sub.TestItemRequestBuilder();", result);
Assert.Contains("var testItemIdx = new TestItemRequestBuilder();", result);
Assert.Contains("var command = testItemIdx.BuildTestMethod1();", result);
Assert.Contains("var cmds = testItemIdx.BuildTestMethod2();", result);
Assert.DoesNotContain("execCommands.AddRange(cmds.Item1)", result);
Expand Down Expand Up @@ -418,12 +418,12 @@ public void WritesMatchingIndexerCommandsIntoContainerCommand()
writer.Write(method);
var result = tw.ToString();

Assert.Contains("var testItemIndexer = new Test.Name.Sub.TestIndexItemRequestBuilder();", result);
Assert.Contains("var testItemIndexer = new 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 Test.TestNavItemRequestBuilder", result);
Assert.Contains("var builder = new TestNavItemRequestBuilder", result);
Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod11());", result);
Assert.Contains("return command;", result);
Assert.DoesNotContain("nonExecCommands.Add(builder.BuildTestMethod3());", result);
Expand Down Expand Up @@ -553,7 +553,7 @@ public void WritesNavCommandThatSkipsReusedNavCommandInstance()
var result = tw.ToString();

Assert.Contains("var command = new Command(\"user\");", result);
Assert.Contains("var builder = new Test.TestNavItemRequestBuilder();", result);
Assert.Contains("var builder = new TestNavItemRequestBuilder();", result);
Assert.Contains("execCommands.Add(builder.BuildExecutableTestMethod());", result);
Assert.Contains("return command;", result);
Assert.DoesNotContain("BuildNavTestMethod", result);
Expand Down Expand Up @@ -596,7 +596,7 @@ public void WritesContainerCommands()
var result = tw.ToString();

Assert.Contains("var command = new Command(\"user\");", result);
Assert.Contains("var builder = new Test.Name.Sub1.Sub2.TestClass1", result);
Assert.Contains("var builder = new TestClass1", result);
Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod1());", result);
Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod2());", result);
Assert.Contains("return command;", result);
Expand Down Expand Up @@ -637,7 +637,7 @@ public void WritesRequestBuilderWithParametersCommands()
var result = tw.ToString();

Assert.Contains("var command = new Command(\"user\");", result);
Assert.Contains("var builder = new Test.Name.Sub1.Sub2.TestClass1", result);
Assert.Contains("var builder = new TestClass1", result);
Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod1());", result);
Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod2());", result);
Assert.Contains("return command;", result);
Expand Down Expand Up @@ -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<Test.Content>(Test.Content.CreateFromDiscriminatorValue);", result);
Assert.Contains("var model = parseNode.GetObjectValue<Content>(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);
Expand Down Expand Up @@ -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<Test.Content>(Test.Content.CreateFromDiscriminatorValue);", result);
Assert.Contains("var model = parseNode.GetObjectValue<Content>(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);
Expand Down Expand Up @@ -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<Test.Content>(Test.Content.CreateFromDiscriminatorValue)?.ToList();", result);
Assert.Contains("var model = parseNode.GetCollectionOfObjectValues<Content>(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);
Expand Down
Loading

0 comments on commit c885247

Please sign in to comment.