Merge branch 'main' into checkAvailableTypeName

.chronus/changes/http-specs-fix-dotnet-compatibility-failure-2024-10-11-16-9-50.md

@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@typespec/http-specs"
+---
+
+Fix dotnet compatibility failure in http-specs

packages/http-client-csharp/eng/scripts/Generate.ps1

@@ -56,15 +56,11 @@ $failingSpecs = @(
     Join-Path 'http' 'payload' 'pageable'
     Join-Path 'http' 'resiliency' 'srv-driven'
     Join-Path 'http' 'routes'
-    Join-Path 'http' 'serialization' 'encoded-name' 'json'
     Join-Path 'http' 'server' 'endpoint' 'not-defined'
-    Join-Path 'http' 'server' 'versions' 'versioned'
     Join-Path 'http' 'special-headers' 'conditional-request'
     Join-Path 'http' 'special-headers' 'repeatability'
     Join-Path 'http' 'type' 'model' 'flatten'
-    Join-Path 'http' 'type' 'model' 'inheritance' 'nested-discriminator'
     Join-Path 'http' 'type' 'model' 'inheritance' 'not-discriminated'
-    Join-Path 'http' 'type' 'model' 'inheritance' 'recursive'
     Join-Path 'http' 'type' 'model' 'templated'
 )
 

packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel.StubLibrary/src/StubLibraryVisitor.cs

@@ -39,11 +39,6 @@ internal class StubLibraryVisitor : ScmLibraryVisitor
                 return null;
             }
 
-            /* remove constructors for ClientOptions */
-            if (type.Implements.Any(i => i.Equals(typeof(ClientPipelineOptions))))
-            {
-                type.Update(constructors: []);
-            }
             return type;
         }
 
@@ -71,7 +66,10 @@ private static bool IsCallingBaseCtor(ConstructorProvider constructor)
 
         protected override FieldProvider? Visit(FieldProvider field)
         {
-            return field.Modifiers.HasFlag(FieldModifiers.Public) ? field : null;
+            // For ClientOptions, keep the non-public field as this currently represents the latest service version for a client.
+            return (field.Modifiers.HasFlag(FieldModifiers.Public) || field.EnclosingType.Implements.Any(i => i.Equals(typeof(ClientPipelineOptions))))
+                ? field
+                : null;
         }
 
         protected override MethodProvider? Visit(MethodProvider method)

packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/src/Providers/ScmMethodProviderCollection.cs

@@ -320,21 +320,9 @@ private IReadOnlyList<ValueExpression> GetProtocolMethodArguments(IReadOnlyList<
         {
             List<ValueExpression> conversions = new List<ValueExpression>();
             bool addedSpreadSource = false;
-            bool firstOptional = false;
-            bool requestOptionsExpressionAdded = false;
 
             foreach (var param in convenienceMethodParameters)
             {
-                if (!firstOptional && param.DefaultValue is not null)
-                {
-                    firstOptional = true;
-                    if (!ShouldAddOptionalRequestOptionsParameter())
-                    {
-                        // insert the required request options argument expression before the first optional argument expression
-                        conversions.Add(Null);
-                        requestOptionsExpressionAdded = true;
-                    }
-                }
                 if (param.SpreadSource is not null)
                 {
                     if (!addedSpreadSource)
@@ -375,10 +363,8 @@ private IReadOnlyList<ValueExpression> GetProtocolMethodArguments(IReadOnlyList<
                     conversions.Add(param);
                 }
             }
-            if (!requestOptionsExpressionAdded)
-            {
-                conversions.Add(Null);
-            }
+            // RequestOptions argument
+            conversions.Add(Null);
             return conversions;
         }
 
@@ -418,13 +404,28 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod
             bool addOptionalRequestOptionsParameter = ShouldAddOptionalRequestOptionsParameter();
             ParameterProvider requestOptionsParameter = addOptionalRequestOptionsParameter ? ScmKnownParameters.OptionalRequestOptions : ScmKnownParameters.RequestOptions;
 
+            if (!addOptionalRequestOptionsParameter && optionalParameters.Count > 0)
+            {
+                // If there are optional parameters, but the request options parameter is not optional, make the optional parameters nullable required.
+                // This is to ensure that the request options parameter is always the last parameter.
+                foreach (var parameter in optionalParameters)
+                {
+                    parameter.DefaultValue = null;
+                    parameter.Type = parameter.Type.WithNullable(true);
+                }
+                // Now, the request options parameter can be optional due to the above changes to the method signature.
+                requestOptionsParameter = ScmKnownParameters.OptionalRequestOptions;
+                requiredParameters.AddRange(optionalParameters);
+                optionalParameters.Clear();
+            }
+
             var methodSignature = new MethodSignature(
                 isAsync ? _cleanOperationName + "Async" : _cleanOperationName,
                 FormattableStringHelpers.FromString(Operation.Description),
                 methodModifier,
                 GetResponseType(Operation.Responses, false, isAsync, out _),
                 $"The response returned from the service.",
-                addOptionalRequestOptionsParameter ? [.. requiredParameters, .. optionalParameters, requestOptionsParameter] : [.. requiredParameters, requestOptionsParameter, .. optionalParameters]);
+                [.. requiredParameters, .. optionalParameters, requestOptionsParameter]);
             var processMessageName = isAsync ? "ProcessMessageAsync" : "ProcessMessage";
 
             MethodBodyStatement[] methodBody =
@@ -487,6 +488,14 @@ private bool ShouldAddOptionalRequestOptionsParameter()
                 {
                     return true;
                 }
+                if (ProtocolMethodParameters[i].DefaultValue == null && ConvenienceMethodParameters[i].DefaultValue != null)
+                {
+                    return true;
+                }
+                if (ProtocolMethodParameters[i].DefaultValue != null && ConvenienceMethodParameters[i].DefaultValue == null)
+                {
+                    return true;
+                }
             }
 
             return false;

packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs

@@ -448,23 +448,25 @@ public void TestRequestOptionsParameterInSignature(InputOperation inputOperation
             Assert.IsNotNull(requestOptionsParameterInAsyncMethod);
             Assert.AreEqual(shouldBeOptional, requestOptionsParameterInAsyncMethod!.Type.IsNullable);
 
+            // request options should always be last parameter
+            Assert.AreEqual("RequestOptions", syncMethod.Signature.Parameters[^1].Type.Name);
+            Assert.AreEqual("RequestOptions", asyncMethod.Signature.Parameters[^1].Type.Name);
+
             if (shouldBeOptional)
             {
                 Assert.IsNotNull(requestOptionsParameterInSyncMethod.DefaultValue);
                 Assert.IsNotNull(requestOptionsParameterInAsyncMethod.DefaultValue);
-                // request options should be last optional parameter
-                Assert.AreEqual("RequestOptions", syncMethod.Signature.Parameters[^1].Type.Name);
-                Assert.AreEqual("RequestOptions", asyncMethod.Signature.Parameters[^1].Type.Name);
             }
-            else
+
+            if (shouldBeOptional && hasOptionalParameter)
             {
-                Assert.IsNull(requestOptionsParameterInSyncMethod.DefaultValue);
-                Assert.IsNull(requestOptionsParameterInAsyncMethod.DefaultValue);
-                // optional parameters should come after the required request options
-                if (hasOptionalParameter)
+                var optionalParameter = syncMethod.Signature.Parameters[^2];
+                // The optional parameter should be required in protocol method
+                Assert.IsNull(optionalParameter.DefaultValue);
+                // It should also be nullable for value types
+                if (optionalParameter.Type.IsValueType)
                 {
-                    Assert.AreEqual("RequestOptions", syncMethod.Signature.Parameters[^2].Type.Name);
-                    Assert.AreEqual("RequestOptions", asyncMethod.Signature.Parameters[^2].Type.Name);
+                    Assert.IsTrue(optionalParameter.Type.IsNullable);
                 }
             }
         }
@@ -736,8 +738,8 @@ public static IEnumerable<TestCaseData> RequestOptionsParameterInSignatureTestCa
                                 isRequired: true),
                         ]), false, false);
 
-                // Protocol & convenience methods will have the same parameters, so RequestOptions should be required.
-                // One of the parameter is optional, so RequestOptions should come before it
+                // Protocol & convenience methods will have the same parameters.
+                // One of the parameter is optional, so it should be make required in the protocol method, and RequestOptions can be optional.
                 yield return new TestCaseData(
                     InputFactory.Operation(
                         "TestOperation",
@@ -753,7 +755,26 @@ public static IEnumerable<TestCaseData> RequestOptionsParameterInSignatureTestCa
                                 InputPrimitiveType.Int64,
                                 location: RequestLocation.None,
                                 isRequired: true),
-                        ]), false, true);
+                        ]), true, true);
+
+                // Protocol & convenience methods will have the same parameters.
+                // One of the parameter is optional value type, so it should be made nullable required in the protocol method, and RequestOptions can be optional.
+                yield return new TestCaseData(
+                    InputFactory.Operation(
+                        "TestOperation",
+                        parameters:
+                        [
+                            InputFactory.Parameter(
+                                "p1",
+                                InputPrimitiveType.Int32,
+                                location: RequestLocation.None,
+                                isRequired: false),
+                            InputFactory.Parameter(
+                                "p2",
+                                InputPrimitiveType.Int64,
+                                location: RequestLocation.None,
+                                isRequired: true),
+                        ]), true, true);
 
                 // convenience method only has a body param, so RequestOptions should be optional in protocol method.
                 yield return new TestCaseData(

packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/DiscriminatorTests.cs

@@ -279,23 +279,42 @@ public void TestNestedDiscriminatedModelWithOwnDiscriminator()
                 "plant",
                 properties:
                 [
-                    InputFactory.Property("foo", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
+                    InputFactory.Property("plantType", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
+                    InputFactory.Property("name", InputPrimitiveType.String, isRequired: true),
                 ],
                 discriminatedModels: new Dictionary<string, InputModelType>() { { "tree", treeModel } });
 
-            MockHelpers.LoadMockPlugin(inputModels: () => [baseModel, treeModel]);
+            MockHelpers.LoadMockPlugin(inputModels: () => [baseModel, treeModel, oakTreeModel]);
             var baseModelProvider = ClientModelPlugin.Instance.OutputLibrary.TypeProviders.OfType<ModelProvider>()
                 .FirstOrDefault(t => t.Name == "Plant");
             var treeModelProvider = ClientModelPlugin.Instance.OutputLibrary.TypeProviders.OfType<ModelProvider>()
                 .FirstOrDefault(t => t.Name == "Tree");
+            var oakTreeModelProvider = ClientModelPlugin.Instance.OutputLibrary.TypeProviders.OfType<ModelProvider>()
+                .FirstOrDefault(t => t.Name == "OakTree");
             Assert.IsNotNull(baseModelProvider);
             Assert.IsNotNull(treeModelProvider);
+            Assert.IsNotNull(oakTreeModelProvider);
+
+            var baseModelConstructor = baseModelProvider!.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected));
+            Assert.IsNotNull(baseModelConstructor);
+            Assert.IsTrue(baseModelConstructor!.Signature.Parameters.Any(p => p.Name == "name"));
+            Assert.IsTrue(baseModelConstructor.Signature.Parameters.Any(p => p.Name == "plantType"));
+
+            var treeModelConstructor = treeModelProvider!.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public));
+            Assert.IsNotNull(treeModelConstructor);
+            Assert.IsTrue(treeModelConstructor!.Signature.Parameters.Any(p => p.Name == "name"));
+            Assert.IsFalse(treeModelConstructor.Signature.Parameters.Any(p => p.Name == "plantType"));
+
+            var oakTreeModelConstructor = oakTreeModelProvider!.Constructors.FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public));
+            Assert.IsNotNull(oakTreeModelConstructor);
+            Assert.IsTrue(oakTreeModelConstructor!.Signature.Parameters.Any(p => p.Name == "name"));
+            Assert.IsFalse(oakTreeModelConstructor.Signature.Parameters.Any(p => p.Name == "plantType"));
 
             // validate the base discriminator deserialization method has the switch statement
             var baseDeserializationMethod = baseModelProvider!.SerializationProviders.FirstOrDefault()!.Methods
                 .FirstOrDefault(m => m.Signature.Name == "DeserializePlant");
             Assert.IsTrue(baseDeserializationMethod?.BodyStatements!.ToDisplayString().Contains(
-                $"if (element.TryGetProperty(\"foo\"u8, out global::System.Text.Json.JsonElement discriminator))"));
+                $"if (element.TryGetProperty(\"plantType\"u8, out global::System.Text.Json.JsonElement discriminator))"));
 
             var treeModelSerializationProvider = treeModelProvider!.SerializationProviders.FirstOrDefault();
             Assert.IsNotNull(treeModelSerializationProvider);

packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Properties/launchSettings.json

@@ -105,6 +105,11 @@
       "commandName": "Executable",
       "executablePath": "$(SolutionDir)/../dist/generator/Microsoft.Generator.CSharp.exe"
     },
+    "http-serialization-encoded-name-json": {
+      "commandLineArgs": "$(SolutionDir)/TestProjects/CadlRanch/http/serialization/encoded-name/json -p StubLibraryPlugin",
+      "commandName": "Executable",
+      "executablePath": "$(SolutionDir)/../dist/generator/Microsoft.Generator.CSharp.exe"
+    },
     "http-server-path-multiple": {
       "commandLineArgs": "$(SolutionDir)/TestProjects/CadlRanch/http/server/path/multiple -p StubLibraryPlugin",
       "commandName": "Executable",
@@ -120,6 +125,11 @@
       "commandName": "Executable",
       "executablePath": "$(SolutionDir)/../dist/generator/Microsoft.Generator.CSharp.exe"
     },
+    "http-server-versions-versioned": {
+      "commandLineArgs": "$(SolutionDir)/TestProjects/CadlRanch/http/server/versions/versioned -p StubLibraryPlugin",
+      "commandName": "Executable",
+      "executablePath": "$(SolutionDir)/../dist/generator/Microsoft.Generator.CSharp.exe"
+    },
     "http-type-array": {
       "commandLineArgs": "$(SolutionDir)/TestProjects/CadlRanch/http/type/array -p StubLibraryPlugin",
       "commandName": "Executable",
@@ -150,6 +160,16 @@
       "commandName": "Executable",
       "executablePath": "$(SolutionDir)/../dist/generator/Microsoft.Generator.CSharp.exe"
     },
+    "http-type-model-inheritance-nested-discriminator": {
+      "commandLineArgs": "$(SolutionDir)/TestProjects/CadlRanch/http/type/model/inheritance/nested-discriminator -p StubLibraryPlugin",
+      "commandName": "Executable",
+      "executablePath": "$(SolutionDir)/../dist/generator/Microsoft.Generator.CSharp.exe"
+    },
+    "http-type-model-inheritance-recursive": {
+      "commandLineArgs": "$(SolutionDir)/TestProjects/CadlRanch/http/type/model/inheritance/recursive -p StubLibraryPlugin",
+      "commandName": "Executable",
+      "executablePath": "$(SolutionDir)/../dist/generator/Microsoft.Generator.CSharp.exe"
+    },
     "http-type-model-inheritance-single-discriminator": {
       "commandLineArgs": "$(SolutionDir)/TestProjects/CadlRanch/http/type/model/inheritance/single-discriminator -p StubLibraryPlugin",
       "commandName": "Executable",

packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs

@@ -447,6 +447,53 @@ private ConstructorProvider BuildFullConstructor()
                 this);
         }
 
+        private IReadOnlyList<PropertyProvider> GetAllBasePropertiesForConstructorInitialization()
+        {
+            var properties = new List<PropertyProvider>();
+            var modelProvider = BaseModelProvider;
+            bool isDirectBase = true;
+            while (modelProvider != null)
+            {
+                foreach (var property in modelProvider.CanonicalView.Properties)
+                {
+                    if (property.IsDiscriminator)
+                    {
+                        // In the case of nested discriminators, we only need to include the direct base discriminator property,
+                        // as this is the only one that will be initialized in this model's constructor.
+                        if (isDirectBase)
+                        {
+                            properties.Add(property);
+                        }
+                    }
+                    else
+                    {
+                        properties.Add(property);
+                    }
+                }
+
+                modelProvider = modelProvider.BaseModelProvider;
+                isDirectBase = false;
+            }
+
+            return properties;
+        }
+
+        private IReadOnlyList<FieldProvider> GetAllBaseFieldsForConstructorInitialization()
+        {
+            var fields = new List<FieldProvider>();
+            var modelProvider = BaseModelProvider;
+            while (modelProvider != null)
+            {
+                foreach (var field in modelProvider.CanonicalView.Fields)
+                {
+                    fields.Add(field);
+                }
+                modelProvider = modelProvider.BaseModelProvider;
+            }
+
+            return fields;
+        }
+
         private (IReadOnlyList<ParameterProvider> Parameters, ConstructorInitializer? Initializer) BuildConstructorParameters(
             bool isPrimaryConstructor)
         {
@@ -457,9 +504,8 @@ private ConstructorProvider BuildFullConstructor()
 
             if (isPrimaryConstructor)
             {
-                // the primary ctor should only include the properties of the direct base model
-                baseProperties = BaseModelProvider?.CanonicalView.Properties ?? [];
-                baseFields = BaseModelProvider?.CanonicalView.Fields ?? [];
+                baseProperties = GetAllBasePropertiesForConstructorInitialization();
+                baseFields = GetAllBaseFieldsForConstructorInitialization();
             }
             else if (BaseModelProvider?.FullConstructor.Signature != null)
             {

packages/http-client-csharp/generator/TestProjects/CadlRanch.Tests/Http/Serialization/EncodedName/Json/EncodedNameJsonTests.cs

@@ -0,0 +1,29 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Threading.Tasks;
+using NUnit.Framework;
+using Serialization.EncodedName.Json.Models;
+using Serialization.EncodedName.Json;
+
+namespace TestProjects.CadlRanch.Tests.Http.Serialization.EncodedName.Json
+{
+    public class EncodedNameJsonTests : CadlRanchTestBase
+    {
+        [CadlRanchTest]
+        public Task PropertySend() => Test(async (host) =>
+        {
+            var response = await new JsonClient(host, null).GetPropertyClient().SendAsync(new JsonEncodedNameModel(true));
+            Assert.AreEqual(204, response.GetRawResponse().Status);
+        });
+
+        [CadlRanchTest]
+        public Task PropertyGet() => Test(async (host) =>
+        {
+            var response = await new JsonClient(host, null).GetPropertyClient().GetAsync();
+
+            Assert.AreEqual(200, response.GetRawResponse().Status);
+            Assert.IsTrue(response.Value.DefaultName);
+        });
+    }
+}