diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs index f8bd0588eac..1441718d0a5 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs @@ -152,7 +152,7 @@ private ScmMethodProvider BuildConvenienceMethod(MethodProvider protocolMethod, methodSignature = new MethodSignature( methodName, DocHelpers.GetFormattableDescription(ServiceMethod.Operation.Summary, ServiceMethod.Operation.Doc) ?? FormattableStringHelpers.FromString(ServiceMethod.Operation.Name), - protocolMethod.Signature.Modifiers, + GetConvenienceMethodModifiers(protocolMethod.Signature.Modifiers, signatureParameters), GetResponseType(ServiceMethod.Operation.Responses, true, isAsync, out _), null, signatureParameters); @@ -215,6 +215,23 @@ .. GetStackVariablesForReturnValueConversion(result, responseBodyType, isAsync, return convenienceMethod; } + private MethodSignatureModifiers GetConvenienceMethodModifiers( + MethodSignatureModifiers modifiers, + IReadOnlyList signatureParameters) + { + var enclosingTypeModifiers = EnclosingType.DeclarationModifiers; + if (modifiers.HasFlag(MethodSignatureModifiers.Public) && + !enclosingTypeModifiers.HasFlag(TypeSignatureModifiers.Internal) && + !enclosingTypeModifiers.HasFlag(TypeSignatureModifiers.Private) && + signatureParameters.Any(p => !p.Type.IsPublic)) + { + modifiers &= ~MethodSignatureModifiers.Public; + modifiers |= MethodSignatureModifiers.Internal; + } + + return modifiers; + } + private IEnumerable GetStackVariablesForProtocolParamConversion(IReadOnlyList convenienceMethodParameters, out Dictionary declarations) { List statements = new List(); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs index 65e6edf5982..6bd6a7743c5 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs @@ -2001,5 +2001,143 @@ public void ConvenienceMethod_JsonListBody_DoesNotUseXmlFromEnumerable() Assert.IsFalse(methodBody.Contains("rootNameHint")); Assert.IsFalse(methodBody.Contains("childNameHint")); } + + [Test] + public void ConvenienceMethod_WithInternalParameterType_IsInternal() + { + // A model that is customized to be internal via client.tsp (an access override) results in a + // convenience method parameter whose type is internal. The convenience method must be + // generated as internal to avoid an inconsistent accessibility compilation error. + var internalModel = InputFactory.Model( + "InternalModel", + access: "internal", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: [InputFactory.Property("Name", InputPrimitiveType.String)]); + + var bodyParam = InputFactory.BodyParameter("body", internalModel, isRequired: true); + var methodBodyParam = InputFactory.MethodParameter( + "body", + internalModel, + isRequired: true, + location: InputRequestLocation.Body); + + var operation = InputFactory.Operation( + "Foo", + httpMethod: "POST", + parameters: [bodyParam], + responses: [InputFactory.OperationResponse([200])]); + + var serviceMethod = InputFactory.BasicServiceMethod("Foo", operation, parameters: [methodBodyParam]); + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + + MockHelpers.LoadMockGenerator(clients: () => [inputClient], inputModels: () => [internalModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + + var convenienceMethods = client!.Methods.Where(m => + m.Signature.Parameters.All(p => p.Name != "content") && + m.Signature.Name.StartsWith("Foo")).ToList(); + Assert.AreEqual(2, convenienceMethods.Count); + + foreach (var convenienceMethod in convenienceMethods) + { + // The parameter type is internal. + Assert.IsTrue(convenienceMethod.Signature.Parameters.Any(p => !p.Type.IsPublic)); + // The convenience method should therefore be internal, not public. + Assert.IsTrue(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal)); + Assert.IsFalse(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + } + } + + [Test] + public async Task ConvenienceMethod_WithCustomCodeInternalParameterType_IsInternal() + { + // A model that is customized to be internal via custom code (an internal partial class) + // results in a convenience method parameter whose type is internal. The convenience method + // must be generated as internal to avoid an inconsistent accessibility compilation error. + var customInternalModel = InputFactory.Model( + "CustomInternalModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: [InputFactory.Property("Name", InputPrimitiveType.String)]); + + var bodyParam = InputFactory.BodyParameter("body", customInternalModel, isRequired: true); + var methodBodyParam = InputFactory.MethodParameter( + "body", + customInternalModel, + isRequired: true, + location: InputRequestLocation.Body); + + var operation = InputFactory.Operation( + "Foo", + httpMethod: "POST", + parameters: [bodyParam], + responses: [InputFactory.OperationResponse([200])]); + + var serviceMethod = InputFactory.BasicServiceMethod("Foo", operation, parameters: [methodBodyParam]); + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + + await MockHelpers.LoadMockGeneratorAsync( + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync(), + clients: () => [inputClient], + inputModels: () => [customInternalModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + + var convenienceMethods = client!.Methods.Where(m => + m.Signature.Parameters.All(p => p.Name != "content") && + m.Signature.Name.StartsWith("Foo")).ToList(); + Assert.AreEqual(2, convenienceMethods.Count); + + foreach (var convenienceMethod in convenienceMethods) + { + // The parameter type is internal because it was customized via custom code. + Assert.IsTrue(convenienceMethod.Signature.Parameters.Any(p => !p.Type.IsPublic)); + // The convenience method should therefore be internal, not public. + Assert.IsTrue(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal)); + Assert.IsFalse(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + } + } + + [Test] + public void ConvenienceMethod_WithPublicParameterType_IsPublic() + { + // When all convenience method parameter types are public, the method should remain public. + var publicModel = InputFactory.Model( + "PublicModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: [InputFactory.Property("Name", InputPrimitiveType.String)]); + + var bodyParam = InputFactory.BodyParameter("body", publicModel, isRequired: true); + var methodBodyParam = InputFactory.MethodParameter( + "body", + publicModel, + isRequired: true, + location: InputRequestLocation.Body); + + var operation = InputFactory.Operation( + "Foo", + httpMethod: "POST", + parameters: [bodyParam], + responses: [InputFactory.OperationResponse([200])]); + + var serviceMethod = InputFactory.BasicServiceMethod("Foo", operation, parameters: [methodBodyParam]); + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + + MockHelpers.LoadMockGenerator(clients: () => [inputClient], inputModels: () => [publicModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + + var convenienceMethods = client!.Methods.Where(m => + m.Signature.Parameters.All(p => p.Name != "content") && + m.Signature.Name.StartsWith("Foo")).ToList(); + Assert.AreEqual(2, convenienceMethods.Count); + + foreach (var convenienceMethod in convenienceMethods) + { + Assert.IsTrue(convenienceMethod.Signature.Parameters.All(p => p.Type.IsPublic)); + Assert.IsTrue(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.IsFalse(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal)); + } + } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/ConvenienceMethod_WithCustomCodeInternalParameterType_IsInternal/CustomInternalModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/ConvenienceMethod_WithCustomCodeInternalParameterType_IsInternal/CustomInternalModel.cs new file mode 100644 index 00000000000..72f19011ba7 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/ConvenienceMethod_WithCustomCodeInternalParameterType_IsInternal/CustomInternalModel.cs @@ -0,0 +1,9 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +namespace Sample.Models +{ + internal partial class CustomInternalModel + { + } +}