Skip to content

Make generated convenience methods internal when a parameter type is internal#10852

Open
Copilot wants to merge 6 commits into
mainfrom
copilot/make-generated-conv-method-internal
Open

Make generated convenience methods internal when a parameter type is internal#10852
Copilot wants to merge 6 commits into
mainfrom
copilot/make-generated-conv-method-internal

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 1, 2026

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).
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.

…ernal

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service Bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Jun 1, 2026
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copilot AI changed the title [WIP] Make generated conv method internal if parameter type is internal Make generated convenience methods internal when a parameter type is internal Jun 1, 2026
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jun 1, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@10852

commit: fc4d4da

…stom-code test

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@JoshLove-msft
Copy link
Copy Markdown
Contributor

Would such methods get filtered by the default value of unreferenced-types-handling? Or are these still considered root methods?

@JoshLove-msft
Copy link
Copy Markdown
Contributor

@copilot

@JoshLove-msft
Copy link
Copy Markdown
Contributor

Nevermind, this is just internalizing methods, not types.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 1, 2026

@copilot

These convenience methods aren't filtered out. unreferenced-types-handling (default removeOrInternalize) operates on whole type declarations, not individual methods — and the client type that owns the convenience method is always a root document (IsClientDocument/IsRootDocument in PostProcessor.cs), so it's never removed or internalized regardless of any method's accessibility.

The method being downgraded to internal only affects the C# accessibility modifier on that member; the method (and its reference to the internal parameter model) still lives on the root client type. The reference map still walks that method, so the internal model remains referenced and is kept (it stays internal since it was already internal) rather than being removed as unreferenced.

@jorgerangel-msft jorgerangel-msft added this pull request to the merge queue Jun 2, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 2, 2026
@jorgerangel-msft jorgerangel-msft added this pull request to the merge queue Jun 2, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 2, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

No changes needing a change description found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Generated Conv Method Internal if Parameter Type is Internal

4 participants