diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel.TestProjects/Unbranded-TypeSpec/src/Generated/Models/RoundTripModel.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel.TestProjects/Unbranded-TypeSpec/src/Generated/Models/RoundTripModel.cs index c9d5a102c3..b8e08700c3 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel.TestProjects/Unbranded-TypeSpec/src/Generated/Models/RoundTripModel.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel.TestProjects/Unbranded-TypeSpec/src/Generated/Models/RoundTripModel.cs @@ -18,10 +18,10 @@ public partial class RoundTripModel public int RequiredInt { get; set; } /// Required collection of enums. - public IList RequiredCollection { get; set; } + public IList RequiredCollection { get; } /// Required dictionary of enums. - public IDictionary RequiredDictionary { get; set; } + public IDictionary RequiredDictionary { get; } /// Required model. public Thing RequiredModel { get; set; } @@ -30,25 +30,25 @@ public partial class RoundTripModel public string IntExtensibleEnum { get; set; } /// this is a collection of int based extensible enum. - public IList IntExtensibleEnumCollection { get; set; } + public IList IntExtensibleEnumCollection { get; } /// this is a float based extensible enum. public string FloatExtensibleEnum { get; set; } /// this is a collection of float based extensible enum. - public IList FloatExtensibleEnumCollection { get; set; } + public IList FloatExtensibleEnumCollection { get; } /// this is a float based fixed enum. public string FloatFixedEnum { get; set; } /// this is a collection of float based fixed enum. - public IList FloatFixedEnumCollection { get; set; } + public IList FloatFixedEnumCollection { get; } /// this is a int based fixed enum. public string IntFixedEnum { get; set; } /// this is a collection of int based fixed enum. - public IList IntFixedEnumCollection { get; set; } + public IList IntFixedEnumCollection { get; } /// this is a string based fixed enum. public string StringFixedEnum { get; set; } @@ -60,10 +60,10 @@ public partial class RoundTripModel public System.BinaryData OptionalUnknown { get; set; } /// required record of unknown. - public IDictionary RequiredRecordUnknown { get; set; } + public IDictionary RequiredRecordUnknown { get; } /// optional record of unknown. - public IDictionary OptionalRecordUnknown { get; set; } + public IDictionary OptionalRecordUnknown { get; } /// required readonly record of unknown. public IDictionary ReadOnlyRequiredRecordUnknown { get; } diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel.TestProjects/Unbranded-TypeSpec/src/Generated/Models/Thing.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel.TestProjects/Unbranded-TypeSpec/src/Generated/Models/Thing.cs index b87b446b2d..993943fa6e 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel.TestProjects/Unbranded-TypeSpec/src/Generated/Models/Thing.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel.TestProjects/Unbranded-TypeSpec/src/Generated/Models/Thing.cs @@ -18,7 +18,7 @@ public partial class Thing public System.BinaryData RequiredUnion { get; set; } /// required literal string. - public string RequiredLiteralString { get; set; } = "accept"; + public string RequiredLiteralString { get; } = "accept"; /// required literal int. public string RequiredLiteralInt { get; set; } = "123"; @@ -27,7 +27,7 @@ public partial class Thing public string RequiredLiteralFloat { get; set; } = "1.23"; /// required literal bool. - public bool RequiredLiteralBool { get; set; } = false; + public bool RequiredLiteralBool { get; } = false; /// optional literal string. public string OptionalLiteralString { get; set; } diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Models/Types/ModelTypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Models/Types/ModelTypeProvider.cs index 489674962c..5a103a4c12 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Models/Types/ModelTypeProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Models/Types/ModelTypeProvider.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using Microsoft.Generator.CSharp.Expressions; using Microsoft.Generator.CSharp.Input; @@ -48,21 +49,56 @@ protected override PropertyDeclaration[] BuildProperties() private PropertyDeclaration BuildPropertyDeclaration(InputModelProperty property) { var propertyType = CodeModelPlugin.Instance.TypeFactory.CreateCSharpType(property.Type); - MethodSignatureModifiers setterModifier = !property.IsReadOnly ? MethodSignatureModifiers.Public : MethodSignatureModifiers.None; + var propHasSetter = PropertyHasSetter(propertyType, property); + MethodSignatureModifiers setterModifier = propHasSetter ? MethodSignatureModifiers.Public : MethodSignatureModifiers.None; - // Exclude setter generation for collection properties https://github.com/Azure/autorest.csharp/issues/4616 // Add Remarks and Example for BinaryData Properties https://github.com/Azure/autorest.csharp/issues/4617 var propertyDeclaration = new PropertyDeclaration( description: FormattableStringHelpers.FromString(property.Description), modifiers: MethodSignatureModifiers.Public, propertyType: propertyType, name: property.Name.FirstCharToUpperCase(), - propertyBody: new AutoPropertyBody(!property.IsReadOnly, setterModifier, GetPropertyInitializationValue(property, propertyType)) + propertyBody: new AutoPropertyBody(propHasSetter, setterModifier, GetPropertyInitializationValue(property, propertyType)) ); return propertyDeclaration; } + /// + /// Returns true if the property has a setter. + /// + /// The of the property. + /// The . + private bool PropertyHasSetter(CSharpType type, InputModelProperty prop) + { + if (prop.IsDiscriminator) + { + return true; + } + + if (prop.IsReadOnly) + { + return false; + } + + if (IsStruct) + { + return false; + } + + if (type.IsLiteral && prop.IsRequired) + { + return false; + } + + if (type.IsCollection && !type.IsReadOnlyMemory) + { + return type.IsNullable; + } + + return true; + } + private ConstantExpression? GetPropertyInitializationValue(InputModelProperty property, CSharpType propertyType) { if (!property.IsRequired) diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Models/Types/ModelTypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Models/Types/ModelTypeProviderTests.cs new file mode 100644 index 0000000000..46857e983c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Models/Types/ModelTypeProviderTests.cs @@ -0,0 +1,114 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Reflection; +using Microsoft.Generator.CSharp.Input; +using Moq; +using NUnit.Framework; + +namespace Microsoft.Generator.CSharp.Tests +{ + public class ModelTypeProviderTests + { +#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. + private GeneratorContext _generatorContext; +#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable. + private readonly string _configFilePath = Path.Combine(AppContext.BaseDirectory, "mocks"); + private FieldInfo? _mockPlugin; + + [SetUp] + public void Setup() + { + // initialize the mock singleton instance of the plugin + _mockPlugin = typeof(CodeModelPlugin).GetField("_instance", BindingFlags.Static | BindingFlags.NonPublic); + _generatorContext = new GeneratorContext(Configuration.Load(_configFilePath)); + } + + [TearDown] + public void Teardown() + { + _mockPlugin?.SetValue(null, null); + } + + // Validates that the property body's setter is correctly set based on the property type + [TestCaseSource(nameof(BuildProperties_ValidatePropertySettersTestCases))] + public void BuildProperties_ValidatePropertySetters(InputModelProperty inputModelProperty, CSharpType type, bool hasSetter) + { + var mockPluginInstance = new Mock(_generatorContext) { }; + var mockTypeFactory = new Mock() { }; + mockTypeFactory.Setup(t => t.CreateCSharpType(It.IsAny())).Returns(type); + mockPluginInstance.SetupGet(p => p.TypeFactory).Returns(mockTypeFactory.Object); + _mockPlugin?.SetValue(null, mockPluginInstance.Object); + + var props = new[] + { + inputModelProperty + }; + + var inputModel = new InputModelType("mockInputModel", "mockNamespace", "public", null, null, InputModelTypeUsage.RoundTrip, props, null, new List(), null, null, null, false); + var modelTypeProvider = new ModelTypeProvider(inputModel, null); + var properties = modelTypeProvider.Properties; + + Assert.IsNotNull(properties); + Assert.AreEqual(1, properties.Count); + + // validate the setter + var prop1 = properties[0]; + Assert.IsNotNull(prop1); + + var autoPropertyBody = prop1.PropertyBody as AutoPropertyBody; + Assert.IsNotNull(autoPropertyBody); + Assert.AreEqual(hasSetter, autoPropertyBody?.HasSetter); + } + + public static IEnumerable BuildProperties_ValidatePropertySettersTestCases + { + get + { + // list property + yield return new TestCaseData( + new InputModelProperty("prop1", "prop1", "public", new InputList("mockProp", new InputPrimitiveType(InputTypeKind.String, false), false, false), false, false, false), + new CSharpType(typeof(IList)), + false); + // read only list property + yield return new TestCaseData( + new InputModelProperty("prop1", "prop1", "public", new InputList("mockProp", new InputPrimitiveType(InputTypeKind.String, false), false, false), false, true, false), + new CSharpType(typeof(IReadOnlyList)), + false); + // nullable list property + yield return new TestCaseData( + new InputModelProperty("prop1", "prop1", "public", new InputList("mockProp", new InputPrimitiveType(InputTypeKind.String, false), false, false), false, false, false), + new CSharpType(typeof(IList), true), + true); + // dictionary property + yield return new TestCaseData( + new InputModelProperty("prop1", "prop1", "public", new InputDictionary("mockProp", new InputPrimitiveType(InputTypeKind.String, false), new InputPrimitiveType(InputTypeKind.String, false), false), false, false, false), + new CSharpType(typeof(IDictionary)), + false); + // nullable dictionary property + yield return new TestCaseData( + new InputModelProperty("prop1", "prop1", "public", new InputDictionary("mockProp", new InputPrimitiveType(InputTypeKind.String, false), new InputPrimitiveType(InputTypeKind.String, false), false), false, false, false), + new CSharpType(typeof(IDictionary), true), + true); + // primitive type property + yield return new TestCaseData( + new InputModelProperty("prop1", "prop1", "public", new InputPrimitiveType(InputTypeKind.String, false), false, false, false), + new CSharpType(typeof(string)), + true); + // read only primitive type property + yield return new TestCaseData( + new InputModelProperty("prop1", "prop1", "public", new InputPrimitiveType(InputTypeKind.String, false), false, true, false), + new CSharpType(typeof(string)), + false); + // readonlymemory property + yield return new TestCaseData( + new InputModelProperty("prop1", "prop1", "public", new InputList("mockProp", new InputPrimitiveType(InputTypeKind.String, false), true, false), false, false, false), + new CSharpType(typeof(ReadOnlyMemory<>)), + true); + } + } + } +}