Extensions: refine tracking of used imports by jcouv · Pull Request #80485 · dotnet/roslyn

jcouv · 2025-09-28T19:00:21Z

Previously, all using directives that included extension blocks would have counted the using as "used". Now, the lookup is based on specific name, arity and options we're looking for, and only extension members that match will result in the using counting as "used".
The symbols reported from the semantic model APIs (CandidateSymbols and GetMemberGroup()) are also affected/trimmed.

Addresses part of #79440 ("Consider refining GetExtensionDeclarations logic (shown below) to tracked used usings more finely")

Relates to test plan #76130

AlekseyTs · 2025-09-30T12:13:23Z

/azp run

azure-pipelines · 2025-09-30T12:13:40Z

Azure Pipelines successfully started running 2 pipeline(s).

AlekseyTs · 2025-09-30T12:18:50Z

src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs

+        /// An alternativeName should only provided if a name is provided.
        /// </summary>
-        internal virtual void GetExtensionDeclarations(ArrayBuilder<NamedTypeSymbol> extensions, Binder originalBinder)
+        internal virtual void GetCandidateExtensionMembers(


Consider adding an "InSingleBinder" suffix to the name #Closed

jjonescz · 2025-09-30T12:10:13Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                string nameTrue = OperatorFacts.UnaryOperatorNameFromOperatorKind(UnaryOperatorKind.True, isChecked: false);
+                signature.Method.OriginalDefinition.ContainingType.ContainingType.GetExtensionMembers(extensionCandidates,
+                    nameTrue, alternativeName: null, arity: 0,
+                    LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember);


Consider moving this common logic into unaryOperatorOverloadResolution #ByDesign

I prefer leaving the call to GetCandidateExtensionMembers in the same code where the ArrayBuilder is declared. So we declared, populate, clear and free it in the same place

jjonescz · 2025-09-30T12:20:08Z

src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs

+        /// Return the extension members from this specific binding scope
        /// Since the lookup of extension members is iterative, proceeding one binding scope at a time,
-        /// GetExtensionDeclarations should not defer to the next binding scope. Instead, the caller is
+        /// GetCandiateExtensionMembers should not defer to the next binding scope. Instead, the caller is


Suggested change

/// GetCandiateExtensionMembers should not defer to the next binding scope. Instead, the caller is

/// GetCandidateExtensionMembers should not defer to the next binding scope. Instead, the caller is

``` #Resolved

jjonescz · 2025-09-30T12:20:34Z

src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs

+        /// GetCandiateExtensionMembers should not defer to the next binding scope. Instead, the caller is
        /// responsible for walking the nested binding scopes from innermost to outermost. This method is overridden
        /// to search the available members list in binding types that represent types, namespaces, and usings.
+        /// An alternativeName should only provided if a name is provided.


Suggested change

/// An alternativeName should only provided if a name is provided.

/// An alternativeName should only be provided if a name is provided.

``` #Resolved

jjonescz · 2025-09-30T12:25:28Z

src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs

        }

 #nullable enable
+        internal static bool IsInvalidExtensionReceiverParameter(ParameterSymbol thisParam)


Can this be private? Also consider inverting the logic and simplifying this to "is valid extension receiver parameter". #Resolved

jjonescz · 2025-09-30T12:27:33Z

src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs

+        }

-        internal void GetExtensionContainers(ArrayBuilder<NamedTypeSymbol> extensions)
+        internal void GetExtensionMembers(ArrayBuilder<Symbol> members, string? name, string? alternativeName, int arity, LookupOptions options)


Should we assert that only LookupOptions.AllMethodsOnArityZero or LookupOptions.MustBeInvocableIfMember are specified? #Resolved

jjonescz · 2025-09-30T12:28:59Z

src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs

 }
+""";
+
+        internal string ExtensionMarkerAttributeIL = """


Suggested change

internal string ExtensionMarkerAttributeIL = """

internal static readonly string ExtensionMarkerAttributeIL = """

``` #Resolved

AlekseyTs · 2025-09-30T12:47:47Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                    extensionCandidatesInSingleScope.Clear();
+                    scope.Binder.GetCandidateExtensionMembers(extensionCandidatesInSingleScope,
+                        ordinaryInstanceOperatorName, checkedInstanceOperatorName, arity: 0,
+                        LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember, this);


LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember

Add MustBeInstance? #Closed

AlekseyTs · 2025-09-30T12:48:25Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

-                    extensionDeclarationsInSingleScope.Clear();
-                    scope.Binder.GetExtensionDeclarations(extensionDeclarationsInSingleScope, this);
+                    extensionCandidatesInSingleScope.Clear();
+                    scope.Binder.GetCandidateExtensionMembers(extensionCandidatesInSingleScope,


scope.Binder.GetCandidateExtensionMembers(extensionCandidatesInSingleScope,

It looks like this call belongs inside the following if. #Closed

AlekseyTs · 2025-09-30T12:49:38Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                    extensionCandidatesInSingleScope.Clear();
+                    scope.Binder.GetCandidateExtensionMembers(extensionCandidatesInSingleScope,
+                        staticOperatorName1, staticOperatorName2Opt, arity: 0,
+                        LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember, this);


LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember

Add MustNotBeInstance? #Closed

AlekseyTs · 2025-09-30T13:06:26Z

src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs

        }

-        internal static void AddOperators(ArrayBuilder<MethodSymbol> operators, ImmutableArray<Symbol> candidates)
+        internal static void AddOperators(ArrayBuilder<MethodSymbol> operators, IEnumerable<Symbol> candidates)


IEnumerable

Consider adding an overload for an ArrayBuilder instead #Closed

AlekseyTs · 2025-09-30T13:10:14Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                string nameTrue = OperatorFacts.UnaryOperatorNameFromOperatorKind(UnaryOperatorKind.True, isChecked: false);
+                signature.Method.OriginalDefinition.ContainingType.ContainingType.GetExtensionMembers(extensionCandidates,
+                    nameTrue, alternativeName: null, arity: 0,
+                    LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember);


LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember

Add MustNotBeInstance? #Closed

AlekseyTs · 2025-09-30T13:10:25Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                    extensionCandidates.Clear();
+                    signature.Method.OriginalDefinition.ContainingType.ContainingType.GetExtensionMembers(extensionCandidates,
+                        nameFalse, alternativeName: null, arity: 0,
+                        LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember);


LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember

Add MustNotBeInstance? #Closed

AlekseyTs · 2025-09-30T13:14:58Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                extensionCandidatesInSingleScope.Clear();
+                scope.Binder.GetCandidateExtensionMembers(extensionCandidatesInSingleScope,
+                    name1, name2Opt, arity: 0,
+                    LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember, this);


LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember

Add MustNotBeInstance? #Closed

AlekseyTs · 2025-09-30T13:15:52Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                extensionCandidatesInSingleScope.Clear();
+                scope.Binder.GetCandidateExtensionMembers(extensionCandidatesInSingleScope,
+                    name1, name2Opt, arity: 0,
+                    LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember, this);


LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember

Add MustNotBeInstance? #Closed

AlekseyTs · 2025-09-30T13:20:03Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                    extensionCandidatesInSingleScope.Clear();
+                    scope.Binder.GetCandidateExtensionMembers(extensionCandidatesInSingleScope,
+                        staticOperatorName1, staticOperatorName2Opt, arity: 0,
+                        LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember, this);


LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember

Add MustNotBeInstance? #Closed

AlekseyTs · 2025-09-30T13:20:21Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                    extensionCandidatesInSingleScope.Clear();
+                    scope.Binder.GetCandidateExtensionMembers(extensionCandidatesInSingleScope,
+                        ordinaryInstanceOperatorName, checkedInstanceOperatorName, arity: 0,
+                        LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember, this);


LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember

Add MustBeInstance? #Closed

AlekseyTs · 2025-09-30T13:21:12Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

-                    extensionDeclarationsInSingleScope.Clear();
-                    scope.Binder.GetExtensionDeclarations(extensionDeclarationsInSingleScope, this);
+                    extensionCandidatesInSingleScope.Clear();
+                    scope.Binder.GetCandidateExtensionMembers(extensionCandidatesInSingleScope,


scope.Binder.GetCandidateExtensionMembers(extensionCandidatesInSingleScope,

It looks like this call belongs inside the following if #Closed

AlekseyTs · 2025-09-30T13:24:58Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                    return;
+                }

-                    if (extensionDeclaration.ExtensionParameter is not { } extensionParameter ||


if (extensionDeclaration.ExtensionParameter is not { } extensionParameter ||

Where these checks are performed now? #Closed

Thanks for catching that. I'll restore the type checks. The check for extension parameter being null is in GetExtensionMembers now

AlekseyTs · 2025-09-30T13:36:53Z

src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorOverloadResolution.cs

-                {
-                    Debug.Assert(extensionDeclaration.IsExtension);
-
-                    if (extensionDeclaration.ExtensionParameter is null)


if (extensionDeclaration.ExtensionParameter is null)

Where this check is performed now? #Closed

NamedTypeSymbol.GetExtensionMembers(...)

AlekseyTs · 2025-09-30T13:38:28Z

src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorOverloadResolution.cs

-                {
-                    Debug.Assert(extensionDeclaration.IsExtension);
-
-                    if (extensionDeclaration.ExtensionParameter is null)


if (extensionDeclaration.ExtensionParameter is null)

Where this check is performed now? #Closed

AlekseyTs · 2025-09-30T13:52:26Z

Done with review pass (commit 3), tests aren't reviewed #Closed

jjonescz · 2025-10-01T08:30:13Z

src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs


        internal void GetExtensionMembers(ArrayBuilder<Symbol> members, string? name, string? alternativeName, int arity, LookupOptions options)
        {
+            Debug.Assert((options & ~(LookupOptions.IncludeExtensionMembers | LookupOptions.AllMethodsOnArityZero


This method does not seem to actually check LookupOptions.IncludeExtensionMembers #Closed

It doesn't look at it, but may receive that option.
It didn't feel necessary to require this options, as many of the callers directly call GetCandidateExtensionMemberInSingleScope so I don't think they should have to further signify that they want extensions.

AlekseyTs · 2025-10-01T14:15:12Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                string nameTrue = OperatorFacts.UnaryOperatorNameFromOperatorKind(UnaryOperatorKind.True, isChecked: false);
+                signature.Method.OriginalDefinition.ContainingType.ContainingType.GetExtensionMembers(extensionCandidates,
+                    nameTrue, alternativeName: null, arity: 0,
+                    LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember | LookupOptions.MustNotBeInstance);


MustBeInvocableIfMember

MustBeOperator? #Closed

AlekseyTs · 2025-10-01T14:15:24Z

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

+                    extensionCandidates.Clear();
+                    signature.Method.OriginalDefinition.ContainingType.ContainingType.GetExtensionMembers(extensionCandidates,
+                        nameFalse, alternativeName: null, arity: 0,
+                        LookupOptions.AllMethodsOnArityZero | LookupOptions.MustBeInvocableIfMember | LookupOptions.MustNotBeInstance);


MustBeInvocableIfMember

MustBeOperator? #Closed

AlekseyTs · 2025-10-01T14:34:42Z

src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs


 #nullable disable

+using System;


Is the using used? #Closed

AlekseyTs · 2025-10-01T14:44:27Z

src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs

+                }
+
+                if ((options & LookupOptions.MustBeInvocableIfMember) != 0
+                    && !Binder.IsInvocableMember(member, fieldsBeingBound: ConsList<FieldSymbol>.Empty))


fieldsBeingBound: ConsList.Empty

This looks suspicious and fragile. The list should probably be passed in, and, when Binder triggers the call we should pass one from the Binder that represents binding context, not the extension scoped being looked at. #Closed

AlekseyTs · 2025-10-01T15:13:49Z

src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs

+            return true;
+        }
+
+        internal void GetExtensionMembers(ArrayBuilder<Symbol> members, string? name, string? alternativeName, int arity, LookupOptions options)


GetExtensionMembers

It might be worth adding a comment saying that this method doesn't perform complete lookup viability check (like accessibility, etc.) And for other APIs that take LookupOptions with the only purpose to pass it to this method as well. #Closed

AlekseyTs · 2025-10-01T15:23:10Z

src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs

+
+                foreach (var candidate in candidates)
+                {
+                    if (extensionMemberMatches(candidate, name, alternativeName, arity, options))


if (extensionMemberMatches(candidate, name, alternativeName, arity, options))

Should we be filtering out types? #Closed

I don't think we need an extra filter. There's no lookup option to exclude types. So it's up to the caller to either request an operator, an invocable member, or deal with various members we could find.

Types are not supported in extensions, I suggest to filter them out always, not based on any option

AlekseyTs · 2025-10-01T15:31:08Z

Done with review pass (commit 5) #Closed

AlekseyTs · 2025-10-01T17:32:52Z

Done with review pass (commit 6). It looks like an assert related to the changes in this PR is firing in CI #Closed

AlekseyTs

LGTM (commit 7)

Julien Couvreur added 2 commits September 27, 2025 09:42

Don't count uninteresting extension blocks

68c941f

Lookup members instead of extension blocks

004e207

jcouv self-assigned this Sep 28, 2025

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

jcouv force-pushed the extensions-usings branch from bc7a754 to 02445d9 Compare September 28, 2025 19:14

jcouv mentioned this pull request Sep 28, 2025

Extensions: follow-ups related to using directives #79440

Closed

2 tasks

jcouv force-pushed the extensions-usings branch from 02445d9 to 47a08ad Compare September 29, 2025 09:22

More coverage

6a8302b

jcouv force-pushed the extensions-usings branch from 47a08ad to 6a8302b Compare September 29, 2025 14:37

jcouv marked this pull request as ready for review September 29, 2025 15:11

jcouv requested a review from a team as a code owner September 29, 2025 15:11

AlekseyTs reviewed Sep 30, 2025

View reviewed changes

jjonescz reviewed Sep 30, 2025

View reviewed changes

AlekseyTs reviewed Sep 30, 2025

View reviewed changes

Address feedback

96f618b

jcouv requested review from AlekseyTs and jjonescz September 30, 2025 19:15

Adjust static check

9cf12d8

jjonescz reviewed Oct 1, 2025

View reviewed changes

jjonescz approved these changes Oct 1, 2025

View reviewed changes

AlekseyTs reviewed Oct 1, 2025

View reviewed changes

Address feedback

83091ee

Dealing with extra lookup options

f639d9d

AlekseyTs approved these changes Oct 2, 2025

View reviewed changes

jcouv merged commit 7d32904 into dotnet:main Oct 2, 2025
24 checks passed

jcouv deleted the extensions-usings branch October 2, 2025 13:44

dotnet-policy-service bot added this to the Next milestone Oct 2, 2025

This was referenced Oct 4, 2025

[Automated] PRs inserted in VS build main-11103.153 #80558

Closed

[Automated] PRs inserted in VS build feature.debugger.main-11103.170 #80560

Closed

davidwengier modified the milestones: Next, 18.3 Jan 6, 2026

jcouv mentioned this pull request Jan 16, 2026

Severe SemanticModel performance regression with Roslyn 5.0 vs 4.14 - 4x slower #81815

Closed

	/// GetCandiateExtensionMembers should not defer to the next binding scope. Instead, the caller is
	/// GetCandidateExtensionMembers should not defer to the next binding scope. Instead, the caller is
	``` #Resolved

	/// An alternativeName should only provided if a name is provided.
	/// An alternativeName should only be provided if a name is provided.
	``` #Resolved

	internal string ExtensionMarkerAttributeIL = """
	internal static readonly string ExtensionMarkerAttributeIL = """
	``` #Resolved

Conversation

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

Uh oh!

Uh oh!

AlekseyTs commented Sep 30, 2025

Uh oh!

azure-pipelines bot commented Sep 30, 2025

Uh oh!

AlekseyTs Sep 30, 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!

jjonescz Sep 30, 2025 • edited by jcouv 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 30, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jjonescz Sep 30, 2025 • edited by jcouv 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 30, 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 30, 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 30, 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 30, 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 30, 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 30, 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 30, 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 30, 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 30, 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 30, 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 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

jcouv commented Sep 28, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

jjonescz Sep 30, 2025 •

edited by jcouv

Loading

jjonescz Sep 30, 2025 •

edited by jcouv

Loading

jjonescz Sep 30, 2025 •

edited by jcouv

Loading

jjonescz Sep 30, 2025 •

edited by jcouv

Loading

jjonescz Sep 30, 2025 •

edited by jcouv

Loading

jjonescz Sep 30, 2025 •

edited by jcouv

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs Sep 30, 2025 •

edited

Loading

AlekseyTs commented Sep 30, 2025 •

edited

Loading

jjonescz Oct 1, 2025 •

edited by jcouv

Loading

AlekseyTs Oct 1, 2025 •

edited

Loading

AlekseyTs Oct 1, 2025 •

edited

Loading

AlekseyTs Oct 1, 2025 •

edited

Loading

AlekseyTs Oct 1, 2025 •

edited

Loading

AlekseyTs Oct 1, 2025 •

edited

Loading

AlekseyTs Oct 1, 2025 •

edited

Loading