Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 4796: Prepend global:: to references of generated C# types #4871

Merged
merged 4 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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
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)}>",
};
}
}
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
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
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
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
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);
}
}
Loading