Skip to content

Commit 8e3c0ba

Browse files
Copilotjorgerangel-msft
Copilot
and
jorgerangel-msft
authored
Make generated convenience methods internal when a parameter type is internal (#10852)
A convenience method generated as `public` but taking a parameter whose type is `internal` (e.g. a model customized to internal via `client.tsp` or custom code) fails to compile with an inconsistent-accessibility error. ## Changes - **`ScmMethodProviderCollection`**: added `GetConvenienceMethodModifiers`, which downgrades a convenience method from `public` to `internal` when the enclosing type is not internal and not private (covering public, protected, and similar visibilities) and any signature parameter type is non-public (`!p.Type.IsPublic`). Applied only to the generator-controlled signature path; customized partial-method signatures keep the user's explicit modifiers, and already-internal/private enclosing types are left untouched. - **Tests**: added coverage for the internal-parameter case via a `client.tsp` access override (method becomes `internal`), the internal-parameter case via custom code using a TestData `internal partial class` (method becomes `internal`), and the public-parameter case (method stays `public`). ```csharp private MethodSignatureModifiers GetConvenienceMethodModifiers( MethodSignatureModifiers modifiers, IReadOnlyList<ParameterProvider> 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; } ``` Regenerating the existing test libraries produced no output diffs, confirming the change only affects the previously-broken internal-parameter scenario. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
1 parent 3c23dd2 commit 8e3c0ba

3 files changed

Lines changed: 165 additions & 1 deletion

File tree

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ private ScmMethodProvider BuildConvenienceMethod(MethodProvider protocolMethod,
152152
methodSignature = new MethodSignature(
153153
methodName,
154154
DocHelpers.GetFormattableDescription(ServiceMethod.Operation.Summary, ServiceMethod.Operation.Doc) ?? FormattableStringHelpers.FromString(ServiceMethod.Operation.Name),
155-
protocolMethod.Signature.Modifiers,
155+
GetConvenienceMethodModifiers(protocolMethod.Signature.Modifiers, signatureParameters),
156156
GetResponseType(ServiceMethod.Operation.Responses, true, isAsync, out _),
157157
null,
158158
signatureParameters);
@@ -215,6 +215,23 @@ .. GetStackVariablesForReturnValueConversion(result, responseBodyType, isAsync,
215215
return convenienceMethod;
216216
}
217217

218+
private MethodSignatureModifiers GetConvenienceMethodModifiers(
219+
MethodSignatureModifiers modifiers,
220+
IReadOnlyList<ParameterProvider> signatureParameters)
221+
{
222+
var enclosingTypeModifiers = EnclosingType.DeclarationModifiers;
223+
if (modifiers.HasFlag(MethodSignatureModifiers.Public) &&
224+
!enclosingTypeModifiers.HasFlag(TypeSignatureModifiers.Internal) &&
225+
!enclosingTypeModifiers.HasFlag(TypeSignatureModifiers.Private) &&
226+
signatureParameters.Any(p => !p.Type.IsPublic))
227+
{
228+
modifiers &= ~MethodSignatureModifiers.Public;
229+
modifiers |= MethodSignatureModifiers.Internal;
230+
}
231+
232+
return modifiers;
233+
}
234+
218235
private IEnumerable<MethodBodyStatement> GetStackVariablesForProtocolParamConversion(IReadOnlyList<ParameterProvider> convenienceMethodParameters, out Dictionary<string, ValueExpression> declarations)
219236
{
220237
List<MethodBodyStatement> statements = new List<MethodBodyStatement>();

‎packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2001,5 +2001,143 @@ public void ConvenienceMethod_JsonListBody_DoesNotUseXmlFromEnumerable()
20012001
Assert.IsFalse(methodBody.Contains("rootNameHint"));
20022002
Assert.IsFalse(methodBody.Contains("childNameHint"));
20032003
}
2004+
2005+
[Test]
2006+
public void ConvenienceMethod_WithInternalParameterType_IsInternal()
2007+
{
2008+
// A model that is customized to be internal via client.tsp (an access override) results in a
2009+
// convenience method parameter whose type is internal. The convenience method must be
2010+
// generated as internal to avoid an inconsistent accessibility compilation error.
2011+
var internalModel = InputFactory.Model(
2012+
"InternalModel",
2013+
access: "internal",
2014+
usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json,
2015+
properties: [InputFactory.Property("Name", InputPrimitiveType.String)]);
2016+
2017+
var bodyParam = InputFactory.BodyParameter("body", internalModel, isRequired: true);
2018+
var methodBodyParam = InputFactory.MethodParameter(
2019+
"body",
2020+
internalModel,
2021+
isRequired: true,
2022+
location: InputRequestLocation.Body);
2023+
2024+
var operation = InputFactory.Operation(
2025+
"Foo",
2026+
httpMethod: "POST",
2027+
parameters: [bodyParam],
2028+
responses: [InputFactory.OperationResponse([200])]);
2029+
2030+
var serviceMethod = InputFactory.BasicServiceMethod("Foo", operation, parameters: [methodBodyParam]);
2031+
var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]);
2032+
2033+
MockHelpers.LoadMockGenerator(clients: () => [inputClient], inputModels: () => [internalModel]);
2034+
2035+
var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient);
2036+
2037+
var convenienceMethods = client!.Methods.Where(m =>
2038+
m.Signature.Parameters.All(p => p.Name != "content") &&
2039+
m.Signature.Name.StartsWith("Foo")).ToList();
2040+
Assert.AreEqual(2, convenienceMethods.Count);
2041+
2042+
foreach (var convenienceMethod in convenienceMethods)
2043+
{
2044+
// The parameter type is internal.
2045+
Assert.IsTrue(convenienceMethod.Signature.Parameters.Any(p => !p.Type.IsPublic));
2046+
// The convenience method should therefore be internal, not public.
2047+
Assert.IsTrue(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal));
2048+
Assert.IsFalse(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public));
2049+
}
2050+
}
2051+
2052+
[Test]
2053+
public async Task ConvenienceMethod_WithCustomCodeInternalParameterType_IsInternal()
2054+
{
2055+
// A model that is customized to be internal via custom code (an internal partial class)
2056+
// results in a convenience method parameter whose type is internal. The convenience method
2057+
// must be generated as internal to avoid an inconsistent accessibility compilation error.
2058+
var customInternalModel = InputFactory.Model(
2059+
"CustomInternalModel",
2060+
usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json,
2061+
properties: [InputFactory.Property("Name", InputPrimitiveType.String)]);
2062+
2063+
var bodyParam = InputFactory.BodyParameter("body", customInternalModel, isRequired: true);
2064+
var methodBodyParam = InputFactory.MethodParameter(
2065+
"body",
2066+
customInternalModel,
2067+
isRequired: true,
2068+
location: InputRequestLocation.Body);
2069+
2070+
var operation = InputFactory.Operation(
2071+
"Foo",
2072+
httpMethod: "POST",
2073+
parameters: [bodyParam],
2074+
responses: [InputFactory.OperationResponse([200])]);
2075+
2076+
var serviceMethod = InputFactory.BasicServiceMethod("Foo", operation, parameters: [methodBodyParam]);
2077+
var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]);
2078+
2079+
await MockHelpers.LoadMockGeneratorAsync(
2080+
compilation: async () => await Helpers.GetCompilationFromDirectoryAsync(),
2081+
clients: () => [inputClient],
2082+
inputModels: () => [customInternalModel]);
2083+
2084+
var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient);
2085+
2086+
var convenienceMethods = client!.Methods.Where(m =>
2087+
m.Signature.Parameters.All(p => p.Name != "content") &&
2088+
m.Signature.Name.StartsWith("Foo")).ToList();
2089+
Assert.AreEqual(2, convenienceMethods.Count);
2090+
2091+
foreach (var convenienceMethod in convenienceMethods)
2092+
{
2093+
// The parameter type is internal because it was customized via custom code.
2094+
Assert.IsTrue(convenienceMethod.Signature.Parameters.Any(p => !p.Type.IsPublic));
2095+
// The convenience method should therefore be internal, not public.
2096+
Assert.IsTrue(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal));
2097+
Assert.IsFalse(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public));
2098+
}
2099+
}
2100+
2101+
[Test]
2102+
public void ConvenienceMethod_WithPublicParameterType_IsPublic()
2103+
{
2104+
// When all convenience method parameter types are public, the method should remain public.
2105+
var publicModel = InputFactory.Model(
2106+
"PublicModel",
2107+
usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json,
2108+
properties: [InputFactory.Property("Name", InputPrimitiveType.String)]);
2109+
2110+
var bodyParam = InputFactory.BodyParameter("body", publicModel, isRequired: true);
2111+
var methodBodyParam = InputFactory.MethodParameter(
2112+
"body",
2113+
publicModel,
2114+
isRequired: true,
2115+
location: InputRequestLocation.Body);
2116+
2117+
var operation = InputFactory.Operation(
2118+
"Foo",
2119+
httpMethod: "POST",
2120+
parameters: [bodyParam],
2121+
responses: [InputFactory.OperationResponse([200])]);
2122+
2123+
var serviceMethod = InputFactory.BasicServiceMethod("Foo", operation, parameters: [methodBodyParam]);
2124+
var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]);
2125+
2126+
MockHelpers.LoadMockGenerator(clients: () => [inputClient], inputModels: () => [publicModel]);
2127+
2128+
var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient);
2129+
2130+
var convenienceMethods = client!.Methods.Where(m =>
2131+
m.Signature.Parameters.All(p => p.Name != "content") &&
2132+
m.Signature.Name.StartsWith("Foo")).ToList();
2133+
Assert.AreEqual(2, convenienceMethods.Count);
2134+
2135+
foreach (var convenienceMethod in convenienceMethods)
2136+
{
2137+
Assert.IsTrue(convenienceMethod.Signature.Parameters.All(p => p.Type.IsPublic));
2138+
Assert.IsTrue(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public));
2139+
Assert.IsFalse(convenienceMethod.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal));
2140+
}
2141+
}
20042142
}
20052143
}

‎packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/ConvenienceMethod_WithCustomCodeInternalParameterType_IsInternal/CustomInternalModel.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
namespace Sample.Models
5+
{
6+
internal partial class CustomInternalModel
7+
{
8+
}
9+
}

0 commit comments

Comments
 (0)