Extensions: DocumentationCommentId and ExtensionGroupingName/ExtensionMarkerName APIs, update grouping type metadata name #80164

jcouv · 2025-09-07T06:26:43Z

update metadata name for grouping type (proposed design change)
add ExtensionGroupingName/ExtensionMarkerName APIs
update docID APIs
update symbol display to match

Proposed API design: #80163
Address part of #78957 (extension public API follow-ups)

Based on feedback from API review, I'm moving the API from ITypeSymbol down into INamedTypeSymbol.

Relates to test plan #76130

Filled #80165 for crash in VB test

Note: this was merged into SDK .NET 10 RC2.

ExtensionGroupingName/ExtensionMarkerName APIs

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs

src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Extension.cs

jcouv · 2025-09-09T19:18:49Z

Moved APIs down into INamedTypeSymbol per API review

AlekseyTs · 2025-09-10T16:31:07Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

+                if (symbol.IsExtension)
+                {
+                    _builder.Append('.');
+                    _builder.Append(symbol.ExtensionMarkerName);


_builder.Append(symbol.ExtensionMarkerName);

Should type parameters be appended? #Closed

If we are supposed to reflect metadata name here, we should be good, because ExtensionMarkerName is it.

AlekseyTs · 2025-09-10T16:34:10Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

            {
+                if (symbol.IsExtension)
+                {
+                    return false;


return false;

It is not obvious why we are doing this. Consider adding a comment #Closed

AlekseyTs · 2025-09-10T16:37:05Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

                    }
                }
+
+                GetMatchingExtensions(container, memberName, results); // Note: the arity is already in the content-based extension grouping name


// Note: the arity is already in the content-based extension grouping name

I do not think we can rely on this for imported symbols, other tools are free to generate names differently. #Closed

AlekseyTs · 2025-09-10T16:39:49Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

                    }
                }
+
+                GetMatchingExtensions(container, memberName, results);


GetMatchingExtensions(container, memberName, results);

It looks like we are missing an arity check here as well #Closed

AlekseyTs · 2025-09-10T16:42:04Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

+                ImmutableArray<INamedTypeSymbol> unnamedNamedTypes = container.GetTypeMembers("");
+                foreach (var namedType in unnamedNamedTypes)
+                {
+                    if (namedType.IsExtension && namedType.ExtensionGroupingName == memberName)


ExtensionGroupingName

Do we ever need to match marker name too? #Closed

~~I don't think so~~ Added above. We need to match E.GroupingType.MatchingType back to the extension

AlekseyTs · 2025-09-10T16:59:50Z

src/Compilers/CSharp/Portable/Symbols/PublicModel/TypeSymbol.cs


 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics;


Is the added using necessary? #Closed

AlekseyTs · 2025-09-10T17:03:01Z

src/Compilers/CSharp/Portable/Symbols/Source/ExtensionGroupingInfo.cs

                    continue;
                }

-                var sourceNamedType = (SourceNamedTypeSymbol)type;


var sourceNamedType = (SourceNamedTypeSymbol)type;

My recommendation is to revert changes in this file. #Closed

The thread got marked as resolved, but it looks like changes were not reverted.

The code changed below due to moving APIs down to NamedTypeSymbol

The code changed below due to moving APIs down to NamedTypeSymbol

SourceNamedTypeSymbol is a NamedTypeSymbol. I think changes in this function are not needed

Then we need to add assertions for nullability

Then we need to add assertions for nullability

I have no objection to that. I do object to rewriting code without an intent to change behavior.

AlekseyTs · 2025-09-10T21:53:39Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

-                    if (symbol.TypeParameters.Length > 0)
+                    if (symbol.TypeParameters.Length > 0 && !symbol.IsExtension)
                    {
                        _builder.Append('`');


builder.Append('`');

Are we supposed to add this only when MangleName property is true? Perhaps instead of having this if we should use MetadataName instead of Name above? #Closed

Using MetadataName here would be equivalent to using ExtensionMarkerName, but we want ExtensionGroupingName here:

for a member, we'll display Container.ExtensionGroupingType.Member

for the extension itself, we'll display Container.ExtensionGroupingType.ExtensionMarkerType (the last segment is added by caller who initiates visit on an extension)

AlekseyTs · 2025-09-15T21:08:38Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

            }

-            private static void GetMatchingTypes(List<INamespaceOrTypeSymbol> containers, string memberName, int arity, List<ISymbol> results)
+            private static void GetMatchingTypes(List<INamespaceOrTypeSymbol> containers, string memberName, int arity, bool isTerminal, List<ISymbol> results)


isTerminal

It would be good to explain meaning of this parameter in a comment #Closed

AlekseyTs · 2025-09-15T21:09:44Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

                for (int i = 0, n = containers.Count; i < n; i++)
                {
-                    GetMatchingTypes(containers[i], memberName, arity, results);
+                    GetMatchingTypes(containers[i], memberName, arity, isTerminal: isTerminal, results);


isTerminal

Should we pass false for all, but the last item? #Closed

AlekseyTs · 2025-09-15T21:10:44Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

+                    && container is INamedTypeSymbol { IsExtension: true } extension
+                    && extension.ExtensionMarkerName == memberName)
+                {
+                    results.Add(extension);


results.Add(extension)

Where did we check the arity? #Closed

AlekseyTs · 2025-09-15T21:13:15Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

+
+            private static void GetMatchingExtensions(INamespaceOrTypeSymbol container, string memberName, int arity, List<ISymbol> results)
+            {
+                ImmutableArray<INamedTypeSymbol> unnamedNamedTypes = container.GetTypeMembers("");


ImmutableArray unnamedNamedTypes = container.GetTypeMembers("");

We probably should not attempt to look for an extension in a namespace #Closed

AlekseyTs · 2025-09-15T21:57:15Z

src/Compilers/VisualBasic/Portable/SymbolDisplay/SymbolDisplayVisitor.Types.vb

+        Friend Sub AddExtensionMarkerName(extension As INamedTypeSymbol)
+            Debug.Assert(extension.IsExtension)
+            AddNestedTypeSeparator()
+            Builder.Add(CreatePart(SymbolDisplayPartKind.ClassName, extension, extension.ExtensionMarkerName, True))


True

It is not obvious why this is the right value. #Closed

AlekseyTs · 2025-09-15T22:18:17Z

Are we testing that VB and C# produce the same doc ids for the same entities? I think we need to verify/observe that the issues outlined in dotnet/csharplang#9682 are not present. #Closed

AlekseyTs · 2025-09-15T22:19:47Z

src/Compilers/Core/Portable/DocumentationCommentId.cs

+                    && container is INamedTypeSymbol { IsExtension: true } extension
+                    && extension.ExtensionMarkerName == memberName)
+                {
+                    results.Add(extension);


results.Add(extension);

Are we testing this code path? #Closed

Yes, each of the new tests does

AlekseyTs · 2025-09-15T22:20:06Z

Done with review pass (commit 8) #Closed

jjonescz · 2025-09-16T07:09:26Z

src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Types.cs

            }
        }
+
+        internal void AddExtensionMarkerName(INamedTypeSymbol extension)


This seems to be used only once. Consider inlining or at least marking private. #ByDesign

If this is marked as private, then the caller can't call it.
If it's inlined, then we can't use Builder which is protected.

jjonescz · 2025-09-16T07:15:34Z

src/Compilers/VisualBasic/Portable/SymbolDisplay/SymbolDisplayVisitor.Types.vb

            End If
        End Sub

+        Friend Sub AddExtensionMarkerName(extension As INamedTypeSymbol)


Same comment here. #ByDesign

AlekseyTs · 2025-09-16T14:58:11Z

src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests2.cs

+        var comp2 = CreateCompilation("", references: [comp.EmitToImageReference()]);
+        validate(comp2);
+
+        var vbComp = CreateVisualBasicCompilation("", referencedAssemblies: [comp.EmitToImageReference()]);


comp.EmitToImageReference()

Consider capturing in a local and reusing in two places that are doing the same work now.

AlekseyTs · 2025-09-16T15:13:51Z

src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests2.cs

+        var comp2 = CreateCompilation("", references: [comp.EmitToImageReference()]);
+        validate(comp2);
+
+        var vbComp = CreateVisualBasicCompilation("", referencedAssemblies: [comp.EmitToImageReference()]);


comp.EmitToImageReference()

Similar comment to reuse result of emit rather than doing it twice.

AlekseyTs

LGTM (commit 12)

jcouv self-assigned this Sep 7, 2025

jcouv added Area-Compilers Feature - Extension Everything The extension everything feature labels Sep 7, 2025

dotnet-policy-service bot added VSCode Needs API Review Needs to be reviewed by the API review council labels Sep 7, 2025

jcouv force-pushed the extensions-docid branch 2 times, most recently from fe339a9 to c7f70d0 Compare September 7, 2025 07:04

Extensions: DocumentationCommentId and

2a30d08

ExtensionGroupingName/ExtensionMarkerName APIs

jcouv force-pushed the extensions-docid branch from c7f70d0 to 2a30d08 Compare September 7, 2025 07:50

jcouv marked this pull request as ready for review September 7, 2025 10:47

jcouv requested review from a team as code owners September 7, 2025 10:47

jcouv mentioned this pull request Sep 8, 2025

Extensions: public API follow-ups #78957

Open

13 tasks

jjonescz reviewed Sep 8, 2025

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs Outdated Show resolved Hide resolved

src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Extension.cs Outdated Show resolved Hide resolved

jcouv requested review from jjonescz and AlekseyTs September 8, 2025 20:23

jjonescz approved these changes Sep 9, 2025

View reviewed changes

jcouv mentioned this pull request Sep 9, 2025

Extensions: add SyntaxGenerator support and AssociatedExtensionImplementation API #80170

Merged

Move APIs into INamedTypeSymbol

4549de2

Remove accidental addition

6282fa4

AlekseyTs reviewed Sep 10, 2025

View reviewed changes

Comment

b09c943

jcouv marked this pull request as ready for review September 15, 2025 18:36

AlekseyTs reviewed Sep 15, 2025

View reviewed changes

jcouv requested a review from jjonescz September 15, 2025 22:10

AlekseyTs reviewed Sep 15, 2025

View reviewed changes

Address feedback

96992dd

jcouv requested a review from AlekseyTs September 16, 2025 00:55

jjonescz reviewed Sep 16, 2025

View reviewed changes

jcouv added 3 commits September 16, 2025 03:02

Merge remote-tracking branch 'dotnet/main' into extensions-docid

cd4aa71

Resolve conflicts

63cfb02

Tweak

8aff7b2

AlekseyTs reviewed Sep 16, 2025

View reviewed changes

AlekseyTs approved these changes Sep 16, 2025

View reviewed changes

jjonescz approved these changes Sep 16, 2025

View reviewed changes

Extract locals in test

d8beff7

jcouv merged commit aa0d5d8 into dotnet:main Sep 17, 2025
28 checks passed

jcouv deleted the extensions-docid branch September 17, 2025 07:14

dotnet-policy-service bot added this to the Next milestone Sep 17, 2025

akhera99 modified the milestones: Next, 18.0 P1, 18.0 P2 Sep 22, 2025

jcouv mentioned this pull request Sep 25, 2025

DocumentationCommentId.GetSymbolsForDeclarationId doesn't work with method declared in extension #78606

Closed

Extensions: DocumentationCommentId and ExtensionGroupingName/ExtensionMarkerName APIs, update grouping type metadata name #80164

Extensions: DocumentationCommentId and ExtensionGroupingName/ExtensionMarkerName APIs, update grouping type metadata name #80164

Uh oh!

Conversation

jcouv commented Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jcouv commented Sep 9, 2025

Uh oh!

AlekseyTs Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

jcouv commented Sep 7, 2025 •

edited

Loading

AlekseyTs Sep 10, 2025 •

edited

Loading

AlekseyTs Sep 10, 2025 •

edited

Loading

AlekseyTs Sep 10, 2025 •

edited

Loading

AlekseyTs Sep 10, 2025 •

edited

Loading

AlekseyTs Sep 10, 2025 •

edited

Loading

jcouv Sep 10, 2025 •

edited

Loading

AlekseyTs Sep 10, 2025 •

edited

Loading

AlekseyTs Sep 10, 2025 •

edited

Loading

AlekseyTs Sep 10, 2025 •

edited

Loading

AlekseyTs Sep 15, 2025 •

edited

Loading

AlekseyTs Sep 15, 2025 •

edited

Loading

AlekseyTs Sep 15, 2025 •

edited

Loading

AlekseyTs Sep 15, 2025 •

edited

Loading

AlekseyTs Sep 15, 2025 •

edited

Loading

AlekseyTs commented Sep 15, 2025 •

edited

Loading

AlekseyTs Sep 15, 2025 •

edited

Loading

AlekseyTs commented Sep 15, 2025 •

edited

Loading

jjonescz Sep 16, 2025 •

edited by jcouv

Loading

jjonescz Sep 16, 2025 •

edited by jcouv

Loading