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

Unconditionally generate C# classes with nullable reference types #3945

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
19 changes: 1 addition & 18 deletions src/Kiota.Builder/Writers/CSharp/CSharpConventionService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
Expand All @@ -21,23 +21,6 @@ public class CSharpConventionService : CommonLanguageConventionService
public const string NullableEnableDirective = "#nullable enable";
public const string NullableRestoreDirective = "#nullable restore";

public static void WriteNullableOpening(LanguageWriter writer)
{
ArgumentNullException.ThrowIfNull(writer);
writer.WriteLine($"#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER", false);
writer.WriteLine(NullableEnableDirective, false);
}
public static void WriteNullableMiddle(LanguageWriter writer)
{
ArgumentNullException.ThrowIfNull(writer);
writer.WriteLine(NullableRestoreDirective, false);
writer.WriteLine("#else", false);
}
public static void WriteNullableClosing(LanguageWriter writer)
{
ArgumentNullException.ThrowIfNull(writer);
writer.WriteLine("#endif", false);
}
private const string ReferenceTypePrefix = "<see cref=\"";
private const string ReferenceTypeSuffix = "\"/>";
#pragma warning disable S1006 // Method overrides should not change parameter defaults
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ namespace Kiota.Builder.Writers.CSharp;
public class CodeClassDeclarationWriter : BaseElementWriter<ClassDeclaration, CSharpConventionService>
{
public static string AutoGenerationHeader => "// <auto-generated/>";
public static string NullableHeader => "#nullable enable";
public CodeClassDeclarationWriter(CSharpConventionService conventionService) : base(conventionService) { }
public override void WriteCodeElement(ClassDeclaration codeElement, LanguageWriter writer)
{
Expand All @@ -17,6 +18,7 @@ public override void WriteCodeElement(ClassDeclaration codeElement, LanguageWrit
if (codeElement.Parent?.Parent is CodeNamespace)
{
writer.WriteLine(AutoGenerationHeader);
writer.WriteLine(NullableHeader);
codeElement.Usings
.Where(x => (x.Declaration?.IsExternal ?? true) || !x.Declaration.Name.Equals(codeElement.Name, StringComparison.OrdinalIgnoreCase)) // needed for circular requests patterns like message folder
.Select(static x => x.Declaration?.IsExternal ?? false ?
Expand Down
15 changes: 5 additions & 10 deletions src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using Kiota.Builder.CodeDOM;
Expand Down Expand Up @@ -631,18 +631,13 @@ private void WriteMethodPrototype(CodeMethod code, CodeClass parentClass, Langua
GetParameterSignatureWithNullableRefType(p, code) :
conventions.GetParameterSignature(p, code))
.ToList());
CSharpConventionService.WriteNullableOpening(writer);
writer.WriteLine($"{conventions.GetAccessModifier(code.Access)} {staticModifier}{hideModifier}{completeReturnTypeWithNullable}{methodName}({nullableParameters}){baseSuffix}");
writer.WriteLine("{");
CSharpConventionService.WriteNullableMiddle(writer);
}

writer.WriteLine($"{conventions.GetAccessModifier(code.Access)} {staticModifier}{hideModifier}{completeReturnType}{methodName}({parameters}){baseSuffix}");
else
{
writer.WriteLine($"{conventions.GetAccessModifier(code.Access)} {staticModifier}{hideModifier}{completeReturnType}{methodName}({parameters}){baseSuffix}");
}
writer.WriteLine("{");

if (includeNullableReferenceType)
CSharpConventionService.WriteNullableClosing(writer);

}

private string GetParameterSignatureWithNullableRefType(CodeParameter parameter, CodeElement targetElement)
Expand Down
12 changes: 1 addition & 11 deletions src/Kiota.Builder/Writers/CSharp/CodePropertyWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,7 @@ public override void WriteCodeElement(CodeProperty codeElement, LanguageWriter w
CodePropertyKind.QueryParameter);// Other property types are appropriately constructor initialized
conventions.WriteShortDescription(codeElement, writer);
conventions.WriteDeprecationAttribute(codeElement, writer);
if (isNullableReferenceType)
{
CSharpConventionService.WriteNullableOpening(writer);
WritePropertyInternal(codeElement, writer, $"{propertyType}?");
CSharpConventionService.WriteNullableMiddle(writer);
}

WritePropertyInternal(codeElement, writer, propertyType);// Always write the normal way

if (isNullableReferenceType)
CSharpConventionService.WriteNullableClosing(writer);
WritePropertyInternal(codeElement, writer, isNullableReferenceType ? $"{propertyType}?" : propertyType);
}

private void WritePropertyInternal(CodeProperty codeElement, LanguageWriter writer, string propertyType)
Expand Down
26 changes: 13 additions & 13 deletions tests/Kiota.Builder.Tests/Writers/CSharp/CodeMethodWriterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ public void WritesRequestExecutorBody()
Assert.Contains(AsyncKeyword, result);
Assert.Contains("await", result);
Assert.Contains("cancellationToken", result);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesRequestExecutorBodyWithUntypedReturnValue()
Expand All @@ -505,7 +505,7 @@ public void WritesRequestExecutorBodyWithUntypedReturnValue()
Assert.Contains(AsyncKeyword, result);
Assert.Contains("await", result);
Assert.Contains("cancellationToken", result);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesRequestGeneratorBodyForMultipart()
Expand All @@ -519,7 +519,7 @@ public void WritesRequestGeneratorBodyForMultipart()
writer.Write(method);
var result = tw.ToString();
Assert.Contains("SetContentFromParsable", result);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesRequestExecutorBodyForCollection()
Expand All @@ -544,7 +544,7 @@ public void WritesRequestExecutorBodyForCollection()
Assert.Contains("SendCollectionAsync", result);
Assert.Contains("return collectionResult?.ToList()", result);
Assert.Contains($"{ReturnTypeName}.CreateFromDiscriminatorValue", result);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void DoesntCreateDictionaryOnEmptyErrorMapping()
Expand All @@ -557,7 +557,7 @@ public void DoesntCreateDictionaryOnEmptyErrorMapping()
var result = tw.ToString();
Assert.DoesNotContain("var errorMapping = new Dictionary<string, Func<IParsable>>", result);
Assert.Contains("default", result);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesModelFactoryBodyForUnionModels()
Expand Down Expand Up @@ -943,7 +943,7 @@ public void WritesRequestExecutorBodyForCollections()
var result = tw.ToString();
Assert.Contains("SendCollectionAsync", result);
Assert.Contains("cancellationToken", result);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesRequestGeneratorBodyForNullableScalar()
Expand All @@ -961,7 +961,7 @@ public void WritesRequestGeneratorBodyForNullableScalar()
Assert.Contains("requestInfo.Configure(config)", result);
Assert.Contains("SetContentFromScalar", result);
Assert.Contains("return requestInfo;", result);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesRequestGeneratorBodyForScalar()
Expand All @@ -982,7 +982,7 @@ public void WritesRequestGeneratorBodyForScalar()
Assert.Contains("return requestInfo;", result);
Assert.Contains("async Task<double?>", result);//verify we only have one nullable marker
Assert.DoesNotContain("async Task<double??>", result);//verify we only have one nullable marker
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesRequestGeneratorBodyForParsable()
Expand All @@ -1000,7 +1000,7 @@ public void WritesRequestGeneratorBodyForParsable()
Assert.Contains("requestInfo.Configure(config)", result);
Assert.Contains("SetContentFromParsable", result);
Assert.Contains("return requestInfo;", result);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesRequestGeneratorBodyWhenUrlTemplateIsOverrode()
Expand All @@ -1015,7 +1015,7 @@ public void WritesRequestGeneratorBodyWhenUrlTemplateIsOverrode()
writer.Write(method);
var result = tw.ToString();
Assert.Contains("var requestInfo = new RequestInformation(Method.GET, \"{baseurl+}/foo/bar\", PathParameters)", result);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesRequestGeneratorBodyForScalarCollection()
Expand All @@ -1036,7 +1036,7 @@ public void WritesRequestGeneratorBodyForScalarCollection()
writer.Write(method);
var result = tw.ToString();
Assert.Contains("SetContentFromScalarCollection", result);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesRequestGeneratorBodyKnownRequestBodyType()
Expand All @@ -1056,7 +1056,7 @@ public void WritesRequestGeneratorBodyKnownRequestBodyType()
var result = tw.ToString();
Assert.Contains("SetStreamContent", result, StringComparison.OrdinalIgnoreCase);
Assert.Contains("application/json", result, StringComparison.OrdinalIgnoreCase);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesRequestGeneratorBodyUnknownRequestBodyType()
Expand Down Expand Up @@ -1086,7 +1086,7 @@ public void WritesRequestGeneratorBodyUnknownRequestBodyType()
Assert.Contains("SetStreamContent", result, StringComparison.OrdinalIgnoreCase);
Assert.DoesNotContain("application/json", result, StringComparison.OrdinalIgnoreCase);
Assert.Contains(", requestContentType", result, StringComparison.OrdinalIgnoreCase);
AssertExtensions.CurlyBracesAreClosed(result, 1);
AssertExtensions.CurlyBracesAreClosed(result);
}
[Fact]
public void WritesInheritedDeSerializerBody()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.IO;
using System.Linq;
using Kiota.Builder.CodeDOM;
Expand Down Expand Up @@ -84,7 +84,7 @@ public void WritesCustomProperty()
property.Kind = CodePropertyKind.Custom;
writer.Write(property);
var result = tw.ToString();
Assert.Contains($"{TypeName} {PropertyName}", result);
Assert.Contains($"{TypeName}? {PropertyName}", result);
Assert.Contains("get; set;", result);
}
[Fact]
Expand All @@ -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<" + rootNamespace.Name + ".SomeCustomClass?>(\"propertyName\"); }", result);
Assert.Contains("set { BackingStore?.Set(\"propertyName\", value);", result);
}
[Fact]
Expand Down