From 782dca54ed80d8ecf92fc51d63f18b1906641aaf Mon Sep 17 00:00:00 2001 From: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com> Date: Fri, 6 Sep 2024 14:35:58 -0700 Subject: [PATCH] Support customizing property names (#4362) Fixes https://github.com/microsoft/typespec/issues/4257 --- .../src/Providers/ModelProvider.cs | 59 +++++++++++-------- .../src/Providers/NamedTypeSymbolProvider.cs | 5 +- .../src/Providers/PropertyProvider.cs | 4 ++ .../src/Providers/TypeProvider.cs | 22 +++++++ .../src/SourceInput/CodeGenAttributes.cs | 8 +-- .../src/SourceInput/SourceInputModel.cs | 4 -- .../ModelProviders/ModelCustomizationTests.cs | 30 +++++++++- ...TestCustomization_CanChangePropertyName.cs | 27 +++++++++ 8 files changed, 125 insertions(+), 34 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/TestData/ModelCustomizationTests/TestCustomization_CanChangePropertyName.cs diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs index b51ce09854..c60f62ce73 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs @@ -172,38 +172,41 @@ protected override PropertyProvider[] BuildProperties() continue; var outputProperty = CodeModelPlugin.Instance.TypeFactory.CreatePropertyProvider(property, this); - if (outputProperty != null) + if (outputProperty is null) + continue; + + if (HasCustomProperty(outputProperty)) + continue; + + if (!property.IsDiscriminator) { - if (!property.IsDiscriminator) + var derivedProperty = InputDerivedProperties.FirstOrDefault(p => p.Value.ContainsKey(property.Name)).Value?[property.Name]; + if (derivedProperty is not null) { - var derivedProperty = InputDerivedProperties.FirstOrDefault(p => p.Value.ContainsKey(property.Name)).Value?[property.Name]; - if (derivedProperty is not null) + if (derivedProperty.Type.Equals(property.Type) && DomainEqual(property, derivedProperty)) { - if (derivedProperty.Type.Equals(property.Type) && DomainEqual(property, derivedProperty)) - { - outputProperty.Modifiers |= MethodSignatureModifiers.Virtual; - } + outputProperty.Modifiers |= MethodSignatureModifiers.Virtual; } - var baseProperty = baseProperties.GetValueOrDefault(property.Name); - if (baseProperty is not null) + } + var baseProperty = baseProperties.GetValueOrDefault(property.Name); + if (baseProperty is not null) + { + if (baseProperty.Type.Equals(property.Type) && DomainEqual(baseProperty, property)) { - if (baseProperty.Type.Equals(property.Type) && DomainEqual(baseProperty, property)) - { - outputProperty.Modifiers |= MethodSignatureModifiers.Override; - } - else - { - outputProperty.Modifiers |= MethodSignatureModifiers.New; - var fieldName = $"_{baseProperty.Name.ToVariableName()}"; - outputProperty.Body = new ExpressionPropertyBody( - This.Property(fieldName).NullCoalesce(Default), - outputProperty.Body.HasSetter ? This.Property(fieldName).Assign(Value) : null); - } + outputProperty.Modifiers |= MethodSignatureModifiers.Override; + } + else + { + outputProperty.Modifiers |= MethodSignatureModifiers.New; + var fieldName = $"_{baseProperty.Name.ToVariableName()}"; + outputProperty.Body = new ExpressionPropertyBody( + This.Property(fieldName).NullCoalesce(Default), + outputProperty.Body.HasSetter ? This.Property(fieldName).Assign(Value) : null); } } - - properties.Add(outputProperty); } + + properties.Add(outputProperty); } if (AdditionalPropertiesProperty != null) @@ -214,6 +217,14 @@ protected override PropertyProvider[] BuildProperties() return [.. properties]; } + private bool HasCustomProperty(PropertyProvider property) + { + if (CustomCodeView == null) + return false; + + return CustomCodeView.PropertyNames.Contains(property.Name); + } + private static bool DomainEqual(InputModelProperty baseProperty, InputModelProperty derivedProperty) { if (baseProperty.IsRequired != derivedProperty.IsRequired) diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/NamedTypeSymbolProvider.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/NamedTypeSymbolProvider.cs index 666337b445..ea29226729 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/NamedTypeSymbolProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/NamedTypeSymbolProvider.cs @@ -64,7 +64,10 @@ protected override PropertyProvider[] BuildProperties() GetCSharpType(propertySymbol.Type), propertySymbol.Name, new AutoPropertyBody(propertySymbol.SetMethod is not null), - this); + this) + { + Attributes = propertySymbol.GetAttributes() + }; properties.Add(propertyProvider); } return [.. properties]; diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/PropertyProvider.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/PropertyProvider.cs index 9bac3cb18b..b863b0ab21 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/PropertyProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/PropertyProvider.cs @@ -2,8 +2,10 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using Microsoft.CodeAnalysis; using Microsoft.Generator.CSharp.Expressions; using Microsoft.Generator.CSharp.Input; using Microsoft.Generator.CSharp.Primitives; @@ -36,6 +38,8 @@ public class PropertyProvider public TypeProvider EnclosingType { get; } + internal IEnumerable? Attributes { get; init; } + // for mocking #pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. protected PropertyProvider() 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 ad069a8226..e332431bcd 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 @@ -6,6 +6,7 @@ using System.Linq; using Microsoft.Generator.CSharp.Expressions; using Microsoft.Generator.CSharp.Primitives; +using Microsoft.Generator.CSharp.SourceInput; using Microsoft.Generator.CSharp.Statements; namespace Microsoft.Generator.CSharp.Providers @@ -13,6 +14,8 @@ namespace Microsoft.Generator.CSharp.Providers public abstract class TypeProvider { private Lazy _customCodeView; + private HashSet? _propertyNames; + protected TypeProvider() { _customCodeView = new(GetCustomCodeView); @@ -122,6 +125,8 @@ private TypeSignatureModifiers GetDeclarationModifiersInternal() private IReadOnlyList? _properties; public IReadOnlyList Properties => _properties ??= BuildProperties(); + internal HashSet PropertyNames => _propertyNames ??= BuildPropertyNames(); + private IReadOnlyList? _methods; public IReadOnlyList Methods => _methods ??= BuildMethods(); @@ -146,6 +151,23 @@ private TypeSignatureModifiers GetDeclarationModifiersInternal() protected virtual PropertyProvider[] BuildProperties() => []; + private HashSet BuildPropertyNames() + { + var propertyNames = new HashSet(); + foreach (var property in Properties) + { + propertyNames.Add(property.Name); + foreach (var attribute in property.Attributes ?? []) + { + if (CodeGenAttributes.TryGetCodeGenMemberAttributeValue(attribute, out var name)) + { + propertyNames.Add(name); + } + } + } + return propertyNames; + } + protected virtual FieldProvider[] BuildFields() => []; protected virtual CSharpType[] BuildImplements() => []; diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/SourceInput/CodeGenAttributes.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/SourceInput/CodeGenAttributes.cs index f522118bd9..90efb66d55 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/SourceInput/CodeGenAttributes.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/SourceInput/CodeGenAttributes.cs @@ -10,7 +10,7 @@ namespace Microsoft.Generator.CSharp.SourceInput { - internal class CodeGenAttributes + internal static class CodeGenAttributes { public const string CodeGenSuppressAttributeName = "CodeGenSuppressAttribute"; @@ -24,7 +24,7 @@ internal class CodeGenAttributes public const string CodeGenSerializationAttributeName = "CodeGenSerializationAttribute"; - public bool TryGetCodeGenMemberAttributeValue(AttributeData attributeData, [MaybeNullWhen(false)] out string name) + public static bool TryGetCodeGenMemberAttributeValue(AttributeData attributeData, [MaybeNullWhen(false)] out string name) { name = null; if (attributeData.AttributeClass?.Name != CodeGenMemberAttributeName) @@ -34,7 +34,7 @@ public bool TryGetCodeGenMemberAttributeValue(AttributeData attributeData, [Mayb return name != null; } - public bool TryGetCodeGenSerializationAttributeValue(AttributeData attributeData, [MaybeNullWhen(false)] out string propertyName, out IReadOnlyList? serializationNames, out string? serializationHook, out string? deserializationHook, out string? bicepSerializationHook) + public static bool TryGetCodeGenSerializationAttributeValue(AttributeData attributeData, [MaybeNullWhen(false)] out string propertyName, out IReadOnlyList? serializationNames, out string? serializationHook, out string? deserializationHook, out string? bicepSerializationHook) { propertyName = null; serializationNames = null; @@ -80,7 +80,7 @@ public bool TryGetCodeGenSerializationAttributeValue(AttributeData attributeData return propertyName != null && (serializationNames != null || serializationHook != null || deserializationHook != null || bicepSerializationHook != null); } - public bool TryGetCodeGenModelAttributeValue(AttributeData attributeData, out string[]? usage, out string[]? formats) + public static bool TryGetCodeGenModelAttributeValue(AttributeData attributeData, out string[]? usage, out string[]? formats) { usage = null; formats = null; diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/SourceInput/SourceInputModel.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/SourceInput/SourceInputModel.cs index 97f3227a7e..eb82c9f3d3 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/SourceInput/SourceInputModel.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/SourceInput/SourceInputModel.cs @@ -16,8 +16,6 @@ namespace Microsoft.Generator.CSharp.SourceInput { public class SourceInputModel { - private readonly CodeGenAttributes _codeGenAttributes; - public Compilation? Customization { get; } private Lazy _previousContract; public Compilation? PreviousContract => _previousContract.Value; @@ -29,8 +27,6 @@ public SourceInputModel(Compilation? customization) Customization = customization; _previousContract = new(() => LoadBaselineContract().GetAwaiter().GetResult()); - _codeGenAttributes = new CodeGenAttributes(); - _nameMap = new(PopulateNameMap); } 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 a1dab37ccc..f18551947d 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 @@ -11,7 +11,7 @@ namespace Microsoft.Generator.CSharp.Tests.Providers // the namespace here is cr public class ModelCustomizationTests { // Validates that the property body's setter is correctly set based on the property type - [TestCase] + [Test] public void TestCustomization_CanChangeModelName() { MockHelpers.LoadMockPlugin(customization: Helpers.GetCompilationFromFile()); @@ -31,5 +31,33 @@ public void TestCustomization_CanChangeModelName() Assert.AreEqual(customCodeView?.Name, modelTypeProvider.Type.Name); Assert.AreEqual(customCodeView?.Type.Namespace, modelTypeProvider.Type.Namespace); } + + [Test] + public void TestCustomization_CanChangePropertyName() + { + MockHelpers.LoadMockPlugin(customization: Helpers.GetCompilationFromFile()); + + var props = new[] + { + InputFactory.Property("Prop1", InputFactory.Array(InputPrimitiveType.String)) + }; + + var inputModel = InputFactory.Model("mockInputModel", properties: props); + var modelTypeProvider = new ModelProvider(inputModel); + var customCodeView = modelTypeProvider.CustomCodeView; + + Assert.IsNotNull(customCodeView); + Assert.AreEqual("MockInputModel", modelTypeProvider.Type.Name); + Assert.AreEqual("Sample.Models", modelTypeProvider.Type.Namespace); + Assert.AreEqual(customCodeView?.Name, modelTypeProvider.Type.Name); + Assert.AreEqual(customCodeView?.Type.Namespace, modelTypeProvider.Type.Namespace); + + // the property should be filtered from the model provider + Assert.AreEqual(0, modelTypeProvider.Properties.Count); + + // the property should be added to the custom code view + Assert.AreEqual(1, customCodeView!.Properties.Count); + Assert.AreEqual("Prop2", customCodeView.Properties[0].Name); + } } } diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/TestData/ModelCustomizationTests/TestCustomization_CanChangePropertyName.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/TestData/ModelCustomizationTests/TestCustomization_CanChangePropertyName.cs new file mode 100644 index 0000000000..a2ea89d564 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/TestData/ModelCustomizationTests/TestCustomization_CanChangePropertyName.cs @@ -0,0 +1,27 @@ +#nullable disable + +using System; + +namespace Sample +{ + // TODO: if we decide to use the public APIs, we do not have to define this attribute here. Tracking: https://github.com/Azure/autorest.csharp/issues/4551 + [AttributeUsage(AttributeTargets.Property | AttributeTargets.Field)] + internal class CodeGenMemberAttribute : Attribute + { + public CodeGenMemberAttribute() : base(null) + { + } + + public CodeGenMemberAttribute(string originalName) : base(originalName) + { + } + } +} +namespace Sample.Models +{ + public partial class MockInputModel + { + [CodeGenMember("Prop1")] + public string[] Prop2 { get; set; } + } +}