From 0193f8cb6d0f48f8d5f0a97f754f8071c8f0a006 Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Fri, 11 Oct 2024 15:10:11 -0500 Subject: [PATCH 1/3] fix: additional fixes for custom nullable types --- .../src/Utilities/TypeSymbolExtensions.cs | 27 +++++++++++++------ .../ModelProviders/ModelCustomizationTests.cs | 24 +++++++++++++++++ .../MockInputModel.cs | 19 +++++++++++++ .../NamedTypeSymbolProviderTests.cs | 22 +++++++++++---- 4 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/CanChangePropertyTypeToEnum/MockInputModel.cs diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Utilities/TypeSymbolExtensions.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Utilities/TypeSymbolExtensions.cs index 1efa2c8704..7cc69634ec 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Utilities/TypeSymbolExtensions.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Utilities/TypeSymbolExtensions.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.Generator.CSharp.Primitives; @@ -152,7 +153,7 @@ public static string GetFullyQualifiedNameFromDisplayString(this ISymbol typeSym { // Special case for types that would not be defined in corlib, but should still be considered framework types. "System.BinaryData" => typeof(BinaryData), - _ => System.Type.GetType(fullyQualifiedName) + _ => Type.GetType(fullyQualifiedName) }; } @@ -163,18 +164,28 @@ private static CSharpType ConstructCSharpTypeFromSymbol( { var typeArg = namedTypeSymbol?.TypeArguments.FirstOrDefault(); bool isValueType = typeSymbol.IsValueType; - bool isEnum = typeSymbol.TypeKind == TypeKind.Enum; bool isNullable = typeSymbol.NullableAnnotation == NullableAnnotation.Annotated; + bool isEnum = typeSymbol.TypeKind == TypeKind.Enum || isNullable && typeArg?.TypeKind == TypeKind.Enum; bool isNullableUnknownType = isNullable && typeArg?.TypeKind == TypeKind.Error; string name = isNullableUnknownType ? fullyQualifiedName : typeSymbol.Name; string[] pieces = fullyQualifiedName.Split('.'); + List arguments = []; + INamedTypeSymbol? namedTypeArg = typeArg as INamedTypeSymbol; + INamedTypeSymbol? enumUnderlyingType = !isNullable ? namedTypeSymbol?.EnumUnderlyingType : namedTypeArg?.EnumUnderlyingType; + + // For nullable types, we need to get the type arguments from the underlying type. + if (namedTypeSymbol?.IsGenericType == true && + (!isNullable || (namedTypeArg?.IsGenericType == true))) + { + arguments.AddRange(namedTypeSymbol.TypeArguments.Select(GetCSharpType)); + } // handle nullables - if (isNullable) + if (isNullable && typeArg != null) { // System.Nullable`1[T] -> T - name = typeArg != null ? GetFullyQualifiedName(typeArg) : fullyQualifiedName; - pieces = name.Split('.'); + name = typeArg.Name; + pieces = GetFullyQualifiedName(typeArg).Split('.'); } return new CSharpType( @@ -183,14 +194,14 @@ private static CSharpType ConstructCSharpTypeFromSymbol( isValueType, isNullable, typeSymbol.ContainingType is not null ? GetCSharpType(typeSymbol.ContainingType) : null, - namedTypeSymbol is not null && !isNullableUnknownType ? [.. namedTypeSymbol.TypeArguments.Select(GetCSharpType)] : [], + arguments, typeSymbol.DeclaredAccessibility == Accessibility.Public, isValueType && !isEnum, baseType: typeSymbol.BaseType is not null && typeSymbol.BaseType.TypeKind != TypeKind.Error && !isNullableUnknownType ? GetCSharpType(typeSymbol.BaseType) : null, - underlyingEnumType: namedTypeSymbol is not null && namedTypeSymbol.EnumUnderlyingType is not null - ? GetCSharpType(namedTypeSymbol.EnumUnderlyingType).FrameworkType + underlyingEnumType: enumUnderlyingType != null + ? GetCSharpType(enumUnderlyingType).FrameworkType : null); } diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/ModelCustomizationTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/ModelCustomizationTests.cs index 26136558eb..b04ba7b1fa 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/ModelCustomizationTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/ModelCustomizationTests.cs @@ -112,6 +112,30 @@ public async Task CanChangePropertyType() Assert.AreEqual(new CSharpType(typeof(int[])), modelTypeProvider.CustomCodeView.Properties[0].Type); } + [Test] + public async Task CanChangePropertyTypeToEnum() + { + var props = new[] + { + InputFactory.Property("Prop1", InputPrimitiveType.String) + }; + + var inputModel = InputFactory.Model("mockInputModel", properties: props); + + var plugin = await MockHelpers.LoadMockPluginAsync( + inputModelTypes: [inputModel], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelTypeProvider = plugin.Object.OutputLibrary.TypeProviders.Single(t => t.Name == "MockInputModel"); + AssertCommon(modelTypeProvider, "Sample.Models", "MockInputModel"); + + // the property should be added to the custom code view + Assert.AreEqual(1, modelTypeProvider.CustomCodeView!.Properties.Count); + // the property type should be changed + Assert.AreEqual("SomeEnum", modelTypeProvider.CustomCodeView.Properties[0].Type.Name); + Assert.IsTrue(modelTypeProvider.CustomCodeView.Properties[0].Type.IsNullable); + } + [Test] public async Task CanChangePropertyAccessibility() { diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/CanChangePropertyTypeToEnum/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/CanChangePropertyTypeToEnum/MockInputModel.cs new file mode 100644 index 0000000000..6b90385402 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/TestData/ModelCustomizationTests/CanChangePropertyTypeToEnum/MockInputModel.cs @@ -0,0 +1,19 @@ +#nullable disable + +using Sample; +using Microsoft.Generator.CSharp.Customization; + +namespace Sample.Models +{ + public partial class MockInputModel + { + // CUSTOM: Changed type from string. + [CodeGenMember("Prop1")] + public SomeEnum? Prop1 { get; } + } + + public enum SomeEnum + { + Foo, + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs index 2d996ebf02..b66395653a 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs @@ -74,6 +74,7 @@ public void ValidateProperties() [TestCase(typeof(IList))] [TestCase(typeof(IList))] [TestCase(typeof(IList))] + [TestCase(typeof(IList))] [TestCase(typeof(ReadOnlyMemory?))] [TestCase(typeof(ReadOnlyMemory))] [TestCase(typeof(ReadOnlyMemory))] @@ -83,7 +84,10 @@ public void ValidateProperties() [TestCase(typeof(string[]))] [TestCase(typeof(IDictionary))] [TestCase(typeof(BinaryData))] - public void ValidatePropertyTypes(Type propertyType) + [TestCase(typeof(SomeEnum), true)] + [TestCase(typeof(SomeEnum?), true)] + [TestCase(typeof(IDictionary))] + public void ValidatePropertyTypes(Type propertyType, bool isEnum = false) { // setup var namedSymbol = new NamedSymbol(propertyType); @@ -99,8 +103,9 @@ public void ValidatePropertyTypes(Type propertyType) Assert.IsNotNull(property); bool isNullable = Nullable.GetUnderlyingType(propertyType) != null; - var expectedType = propertyType.FullName!.StartsWith("System") ? new CSharpType(propertyType, isNullable) : - new CSharpType(propertyType.Name, propertyType.Namespace!, false, isNullable, null, [], false, false); + var expectedType = propertyType.FullName!.StartsWith("System") + ? new CSharpType(propertyType, isNullable) + : new CSharpType(propertyType.Name, propertyType.Namespace!, false, isNullable, null, [], false, false); var propertyCSharpType = property!.Type; @@ -109,7 +114,9 @@ public void ValidatePropertyTypes(Type propertyType) Assert.AreEqual(expectedType.IsList, propertyCSharpType.IsList); Assert.AreEqual(expectedType.Arguments.Count, propertyCSharpType.Arguments.Count); Assert.AreEqual(expectedType.IsCollection, propertyCSharpType.IsCollection); - Assert.AreEqual(expectedType.IsFrameworkType, propertyCSharpType.IsFrameworkType); + + var isFrameworkType = !isEnum && expectedType.IsFrameworkType; + Assert.AreEqual(isFrameworkType, propertyCSharpType.IsFrameworkType); for (var i = 0; i < expectedType.Arguments.Count; i++) { @@ -118,7 +125,7 @@ public void ValidatePropertyTypes(Type propertyType) } // validate the underlying types aren't nullable - if (isNullable && expectedType.IsFrameworkType) + if (isNullable && isFrameworkType) { var underlyingType = propertyCSharpType.FrameworkType; Assert.IsTrue(Nullable.GetUnderlyingType(underlyingType) == null); @@ -278,6 +285,11 @@ protected override PropertyProvider[] BuildProperties() protected override string BuildName() => "PropertyType"; } + private enum SomeEnum + { + Foo + } + internal static INamedTypeSymbol? GetSymbol(INamespaceSymbol namespaceSymbol, string name) { foreach (var childNamespaceSymbol in namespaceSymbol.GetNamespaceMembers()) From 3fb96c90853181fb81c35e28b231723c5fe8de0d Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Mon, 14 Oct 2024 16:28:03 -0500 Subject: [PATCH 2/3] pr feedback --- .../src/Providers/TypeProvider.cs | 2 +- .../src/Utilities/TypeSymbolExtensions.cs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/TypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/TypeProvider.cs index c16966262c..1d6d1a9c66 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/TypeProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/TypeProvider.cs @@ -506,7 +506,7 @@ private static bool IsMatch(MethodSignatureBase customMethod, MethodSignatureBas for (int i = 0; i < customMethod.Parameters.Count; i++) { - if (!customMethod.Parameters[i].Type.Name.EndsWith(method.Parameters[i].Type.Name)) + if (customMethod.Parameters[i].Type.Name != method.Parameters[i].Type.Name) { return false; } diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Utilities/TypeSymbolExtensions.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Utilities/TypeSymbolExtensions.cs index 7cc69634ec..6d39629c5e 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Utilities/TypeSymbolExtensions.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Utilities/TypeSymbolExtensions.cs @@ -12,6 +12,7 @@ namespace Microsoft.Generator.CSharp internal static class TypeSymbolExtensions { private const string GlobalPrefix = "global::"; + private const string NullableTypeName = "System.Nullable"; public static bool IsSameType(this INamedTypeSymbol symbol, CSharpType type) { @@ -111,18 +112,17 @@ public static string GetFullyQualifiedName(this ITypeSymbol typeSymbol) // Handle nullable types if (typeSymbol.NullableAnnotation == NullableAnnotation.Annotated && !IsCollectionType(namedTypeSymbol)) { - const string nullableTypeName = "System.Nullable"; var argTypeSymbol = namedTypeSymbol.TypeArguments.FirstOrDefault(); if (argTypeSymbol != null) { if (argTypeSymbol.TypeKind == TypeKind.Error) { - return GetFullyQualifiedName(argTypeSymbol); + return $"{NullableTypeName}`1[{argTypeSymbol}]"; } string[] typeArguments = [.. namedTypeSymbol.TypeArguments.Select(arg => "[" + GetFullyQualifiedName(arg) + "]")]; - return $"{nullableTypeName}`{namedTypeSymbol.TypeArguments.Length}[{string.Join(", ", typeArguments)}]"; + return $"{NullableTypeName}`{namedTypeSymbol.TypeArguments.Length}[{string.Join(", ", typeArguments)}]"; } } else if (namedTypeSymbol.TypeArguments.Length > 0 && !IsCollectionType(namedTypeSymbol)) @@ -164,8 +164,8 @@ private static CSharpType ConstructCSharpTypeFromSymbol( { var typeArg = namedTypeSymbol?.TypeArguments.FirstOrDefault(); bool isValueType = typeSymbol.IsValueType; - bool isNullable = typeSymbol.NullableAnnotation == NullableAnnotation.Annotated; - bool isEnum = typeSymbol.TypeKind == TypeKind.Enum || isNullable && typeArg?.TypeKind == TypeKind.Enum; + bool isNullable = fullyQualifiedName.StartsWith(NullableTypeName); + bool isEnum = typeSymbol.TypeKind == TypeKind.Enum || (isNullable && typeArg?.TypeKind == TypeKind.Enum); bool isNullableUnknownType = isNullable && typeArg?.TypeKind == TypeKind.Error; string name = isNullableUnknownType ? fullyQualifiedName : typeSymbol.Name; string[] pieces = fullyQualifiedName.Split('.'); From a33c369e91574a8dc70d5f2db4993204e8818fdc Mon Sep 17 00:00:00 2001 From: Jorge Rangel Date: Mon, 14 Oct 2024 16:58:23 -0500 Subject: [PATCH 3/3] update tests --- .../NamedTypeSymbolProviderTests.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs index b66395653a..381c646ff6 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs @@ -102,10 +102,15 @@ public void ValidatePropertyTypes(Type propertyType, bool isEnum = false) var property = _namedTypeSymbolProvider.Properties.FirstOrDefault(); Assert.IsNotNull(property); - bool isNullable = Nullable.GetUnderlyingType(propertyType) != null; - var expectedType = propertyType.FullName!.StartsWith("System") + Type? nullableUnderlyingType = Nullable.GetUnderlyingType(propertyType); + var propertyName = nullableUnderlyingType?.Name ?? propertyType.Name; + bool isNullable = nullableUnderlyingType != null; + bool isSystemType = propertyType.FullName!.StartsWith("System") + && (!isNullable || nullableUnderlyingType?.Namespace?.StartsWith("System") == true); + + var expectedType = isSystemType ? new CSharpType(propertyType, isNullable) - : new CSharpType(propertyType.Name, propertyType.Namespace!, false, isNullable, null, [], false, false); + : new CSharpType(propertyName, propertyType.Namespace!, false, isNullable, null, [], false, false); var propertyCSharpType = property!.Type; @@ -114,9 +119,7 @@ public void ValidatePropertyTypes(Type propertyType, bool isEnum = false) Assert.AreEqual(expectedType.IsList, propertyCSharpType.IsList); Assert.AreEqual(expectedType.Arguments.Count, propertyCSharpType.Arguments.Count); Assert.AreEqual(expectedType.IsCollection, propertyCSharpType.IsCollection); - - var isFrameworkType = !isEnum && expectedType.IsFrameworkType; - Assert.AreEqual(isFrameworkType, propertyCSharpType.IsFrameworkType); + Assert.AreEqual(expectedType.IsFrameworkType, propertyCSharpType.IsFrameworkType); for (var i = 0; i < expectedType.Arguments.Count; i++) { @@ -125,7 +128,7 @@ public void ValidatePropertyTypes(Type propertyType, bool isEnum = false) } // validate the underlying types aren't nullable - if (isNullable && isFrameworkType) + if (isNullable && expectedType.IsFrameworkType) { var underlyingType = propertyCSharpType.FrameworkType; Assert.IsTrue(Nullable.GetUnderlyingType(underlyingType) == null);