Skip to content

Commit

Permalink
Merge pull request #4871 from peterwurzinger/fix/4796
Browse files Browse the repository at this point in the history 
Fix 4796: Prepend global:: to references of generated C# types
  • Loading branch information
@baywet
baywet authored Jun 21, 2024
2 parents 439f4ff + 5a6b21e commit 40e1196
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Remove LINQ usage in generated code.
- Ensures descriptions are not empty in sliced OpenApi file when generating a plugin.
- Plugins do not emit parameters anymore. [#4841](https://github.com/microsoft/kiota/issues/4841)
- References to C# types generated by kiota are prefixed with `global::` to avoid name collisions. [#4796](https://github.com/microsoft/kiota/issues/4796)


## [1.15.0] - 2024-06-06
Expand Down
1 change: 0 additions & 1 deletion src/Kiota.Builder/Writers/CSharp/CodeClassDeclarationWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ public override void WriteCodeElement(ClassDeclaration codeElement, LanguageWrit
var derivedTypes = (codeElement.Inherits is null ? Enumerable.Empty<string?>() : new string?[] { conventions.GetTypeString(codeElement.Inherits, parentClass) })
.Union(codeElement.Implements.Select(static x => x.Name))
.OfType<string>()
.Select(static x => x.ToFirstCharacterUpperCase())
.ToArray();
var derivation = derivedTypes.Length != 0 ? ": " + derivedTypes.Aggregate(static (x, y) => $"{x}, {y}") : string.Empty;
bool hasDescription = conventions.WriteLongDescription(parentClass, writer);
Expand Down
4 changes: 2 additions & 2 deletions src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ private string GetDeserializationMethodName(CodeTypeBase propType, CodeMethod me
{
"byte[]" => "GetByteArrayValue()",
_ when conventions.IsPrimitiveType(propertyType) => $"Get{propertyType.TrimEnd(CSharpConventionService.NullableMarker).ToFirstCharacterUpperCase()}Value()",
_ => $"GetObjectValue<{propertyType.ToFirstCharacterUpperCase()}>({propertyType}.CreateFromDiscriminatorValue)",
_ => $"GetObjectValue<{propertyType}>({propertyType}.CreateFromDiscriminatorValue)",
};
}
protected void WriteRequestExecutorBody(CodeMethod codeElement, RequestParams requestParams, CodeClass parentClass, bool isVoid, string returnTypeWithoutCollectionInformation, LanguageWriter writer)
Expand Down Expand Up @@ -671,7 +671,7 @@ private string GetSerializationMethodName(CodeTypeBase propType, CodeMethod meth
{
"byte[]" => "WriteByteArrayValue",
_ when conventions.IsPrimitiveType(propertyType) => $"Write{propertyType.TrimEnd(CSharpConventionService.NullableMarker).ToFirstCharacterUpperCase()}Value",
_ => $"WriteObjectValue<{propertyType.ToFirstCharacterUpperCase()}{(includeNullableRef ? "?" : string.Empty)}>",
_ => $"WriteObjectValue<{propertyType}{(includeNullableRef ? "?" : string.Empty)}>",
};
}
}
2 changes: 1 addition & 1 deletion src/Kiota.Builder/Writers/CSharp/TypeDefinitionExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ private static StringBuilder AppendTypeName(ITypeDefinition typeDefinition, Stri
case CodeNamespace codeNamespace:
{
if (!string.IsNullOrEmpty(codeNamespace.Name))
fullNameBuilder.Insert(0, $"{codeNamespace.Name}.");
fullNameBuilder.Insert(0, $"global::{codeNamespace.Name}.");

return fullNameBuilder;
}
Expand Down
24 changes: 12 additions & 12 deletions tests/Kiota.Builder.Tests/Writers/CLI/CliCodeMethodWriterTests.cs
View file
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 global::Test.Name.Sub.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 global::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);
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 global::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 Test.TestNavItemRequestBuilder", result);
Assert.Contains("var builder = new global::Test.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 global::Test.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 global::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);
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 global::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);
Expand Down Expand Up @@ -702,9 +702,9 @@ public void WritesContainerCommandWithConflictingTypes()
var result = tw.ToString();

Assert.Contains("var command = new Command(\"user\");", result);
Assert.Contains("var builder = new Test.A.B.C.D.E.F.TestRequestBuilder", result);
Assert.Contains("var builder = new global::Test.A.B.C.D.E.F.TestRequestBuilder", result);
// Test case insensitive match
Assert.Contains("var builder = new Test.A.B.C.D.E.F.TestRequestBuilder2", result);
Assert.Contains("var builder = new global::Test.A.B.C.D.E.F.TestRequestBuilder2", result);
Assert.Contains("nonExecCommands.Add(builder.BuildTestMethod1());", result);
Assert.Contains("foreach (var cmd in nonExecCommands)", result);
Assert.Contains("command.AddCommand(cmd);", 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<global::Test.Content>(global::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);
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<global::Test.Content>(global::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);
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<global::Test.Content>(global::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);
Expand Down
12 changes: 6 additions & 6 deletions tests/Kiota.Builder.Tests/Writers/CSharp/CodeMethodWriterTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -737,8 +737,8 @@ 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 models.ConflictingModelBaseClass()", result);
Assert.Contains("\"namespaceLevelOne.ConflictingModel\" => new global::namespaceLevelOne.ConflictingModel(),", result); //Assert the disambiguation happens due to the enum imported
Assert.Contains("_ => new global::models.ConflictingModelBaseClass()", result);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
Expand Down Expand Up @@ -1532,7 +1532,7 @@ public void WritesConstructorWithEnumValue()
writer.Write(method);
var result = tw.ToString();
Assert.Contains(parentClass.Name.ToFirstCharacterUpperCase(), result);
Assert.Contains($"{propName.ToFirstCharacterUpperCase()} = {modelsNamespace.Name}.{codeEnum.Name.ToFirstCharacterUpperCase()}.{defaultValue.CleanupSymbolName()}", result);//ensure symbol is cleaned up
Assert.Contains($"{propName.ToFirstCharacterUpperCase()} = global::{modelsNamespace.Name}.{codeEnum.Name.ToFirstCharacterUpperCase()}.{defaultValue.CleanupSymbolName()}", result);//ensure symbol is cleaned up
}
[Fact]
public void WritesConstructorAndIncludesSanitizedEnumValue()
Expand All @@ -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()} = {modelsNamespace.Name}.{codeEnum.Name.ToFirstCharacterUpperCase()}.{defaultValue.CleanupSymbolName()}", result);//ensure symbol is cleaned up
Assert.Contains($"{propName.ToFirstCharacterUpperCase()} = global::{modelsNamespace.Name}.{codeEnum.Name.ToFirstCharacterUpperCase()}.{defaultValue.CleanupSymbolName()}", result);//ensure symbol is cleaned up
}
[Fact]
public void WritesConstructorAndDisambiguatesEnumType()
Expand All @@ -1581,8 +1581,8 @@ public void WritesConstructorAndDisambiguatesEnumType()
writer.Write(method);
var result = tw.ToString();
Assert.Contains(parentClass.Name.ToFirstCharacterUpperCase(), result);
Assert.Contains("models.TestEnum.First;", result);//ensure enum is fully qualified due to conflicting property name
Assert.Contains($"{propName.ToFirstCharacterUpperCase()} = models.TestEnum.First;", result);//ensure enum is fully qualified due to conflicting property name
Assert.Contains("global::models.TestEnum.First;", result);//ensure enum is fully qualified due to conflicting property name
Assert.Contains($"{propName.ToFirstCharacterUpperCase()} = global::models.TestEnum.First;", result);//ensure enum is fully qualified due to conflicting property name
}
[Fact]
public void DoesNotWriteConstructorWithDefaultFromComposedType()
Expand Down
6 changes: 3 additions & 3 deletions tests/Kiota.Builder.Tests/Writers/CSharp/CodePropertyWriterTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void WritesRequestBuilder()
writer.Write(property);
var result = tw.ToString();
Assert.Contains("get =>", result);
Assert.Contains($"new {rootNamespace.Name}.{TypeName}", result);
Assert.Contains($"new global::{rootNamespace.Name}.{TypeName}", result);
Assert.Contains("RequestAdapter", result);
Assert.Contains("PathParameters", result);
}
Expand Down Expand Up @@ -103,7 +103,7 @@ public void MapsCustomPropertiesToBackingStore()
property.Kind = CodePropertyKind.Custom;
writer.Write(property);
var result = tw.ToString();
Assert.Contains("get { return BackingStore?.Get<" + rootNamespace.Name + ".SomeCustomClass>(\"propertyName\"); }", result);
Assert.Contains("get { return BackingStore?.Get<global::" + rootNamespace.Name + ".SomeCustomClass>(\"propertyName\"); }", result);
Assert.Contains("set { BackingStore?.Set(\"propertyName\", value);", result);
}
[Fact]
Expand All @@ -113,7 +113,7 @@ public void MapsAdditionalDataPropertiesToBackingStore()
property.Kind = CodePropertyKind.AdditionalData;
writer.Write(property);
var result = tw.ToString();
Assert.Contains("get { return BackingStore.Get<" + rootNamespace.Name + ".SomeCustomClass>(\"propertyName\") ?? new Dictionary<string, object>(); }", result);
Assert.Contains("get { return BackingStore.Get<global::" + rootNamespace.Name + ".SomeCustomClass>(\"propertyName\") ?? new Dictionary<string, object>(); }", result);
Assert.Contains("set { BackingStore.Set(\"propertyName\", value);", result);
}
[Fact]
Expand Down
24 changes: 20 additions & 4 deletions tests/Kiota.Builder.Tests/Writers/CSharp/TypeDefinitionExtensionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public void ReturnsFullNameForTypeInNamespace()

var fullName = TypeDefinitionExtensions.GetFullName(myClass);

Assert.Equal("MyNamespace.MyClass", fullName);
Assert.Equal("global::MyNamespace.MyClass", fullName);
}

[Fact]
Expand All @@ -62,8 +62,8 @@ public void ReturnsFullNameForNestedTypes()
var parentClassFullName = TypeDefinitionExtensions.GetFullName(myParentClass);
var nestedClassFullName = TypeDefinitionExtensions.GetFullName(myNestedClass);

Assert.Equal("MyNamespace.MyParentClass", parentClassFullName);
Assert.Equal("MyNamespace.MyParentClass.MyNestedClass", nestedClassFullName);
Assert.Equal("global::MyNamespace.MyParentClass", parentClassFullName);
Assert.Equal("global::MyNamespace.MyParentClass.MyNestedClass", nestedClassFullName);
}

[Fact]
Expand Down Expand Up @@ -117,7 +117,7 @@ public void CapitalizesTypeNamesInTypeHierarchyButNotTheNamespace()

var nestedClassFullName = TypeDefinitionExtensions.GetFullName(myNestedClass);

Assert.Equal("myNamespace.MyParentClass.MyNestedClass", nestedClassFullName);
Assert.Equal("global::myNamespace.MyParentClass.MyNestedClass", nestedClassFullName);
}

[Fact]
Expand All @@ -137,4 +137,20 @@ public void DoesNotAppendNamespaceSegmentIfNamespaceNameIsEmpty()

Assert.Equal("MyClass", fullName);
}

[Fact]
public void PrependsGlobalNamespaceAliasToNamespaces()
{
var rootNamespace = CodeNamespace.InitRootNamespace();
var myNamespace = rootNamespace.AddNamespace("MyNamespace");
var myClass = new CodeClass()
{
Name = "MyClass"
};
myNamespace.AddClass(myClass);

var fullName = TypeDefinitionExtensions.GetFullName(myClass);

Assert.StartsWith("global::", fullName, StringComparison.Ordinal);
}
}

0 comments on commit 40e1196

Please sign in to comment.