Implement scoping and shadowing rules for extension parameter and type parameters #77704

AlekseyTs · 2025-03-20T17:35:16Z

Relates to test plan #76130

…e parameters See dotnet/csharplang#9229 (commit 9)

jcouv · 2025-03-20T23:15:57Z

src/Compilers/CSharp/Portable/CSharpResources.resx

+    <value>Type parameter '{0}' has the same name as an extension container type parameter</value>
+  </data>
+  <data name="ERR_LocalSameNameAsExtensionParameter" xml:space="preserve">
+    <value>'{0}': a parameter, local variable, or local function cannot have the same name as an extension parameter</value>


an extension parameter

Why "an extension parameter" as opposed to "the extension parameter"?
Also applies to other messages #Closed

I used the following message as a template "'{0}': a parameter, local variable, or local function cannot have the same name as a method type parameter"

jcouv · 2025-03-20T23:25:07Z

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

+        {
+            get
+            {
+                int local() => p;


Do we have a test for the happy case, where the extension parameter is used in a non-static local function or lambda, inside an instance extension member? #Closed

Implementation_InstanceMethod_03_WithLocalFunction, for example?

jcouv · 2025-03-20T23:41:43Z

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

+                        else if (parameter.ContainingSymbol is NamedTypeSymbol { IsExtension: true } &&
+                                (InParameterDefaultValue || InAttributeArgument ||
+                                 this.ContainingMember() is not { Kind: not SymbolKind.NamedType, IsStatic: false } || // We are not in an instance member
+                                 (object)this.ContainingMember().ContainingSymbol != parameter.ContainingSymbol) &&


(object)this.ContainingMember().ContainingSymbol != parameter.ContainingSymbol

Is this condition necessary? #Resolved

That is for references in nested types. IsInDeclaringTypeInstanceMember does the same. Declaring a nested type in extension is an error, but I prefer to be consistent

Consider adding a test for this

jcouv · 2025-03-20T23:57:22Z

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

-                        diagnostics.Add(ErrorCode.ERR_LocalSameNameAsTypeParam, GetLocation(p), name);
+                        if (tp.ContainingSymbol is NamedTypeSymbol { IsExtension: true })
+                        {
+                            if (p.ContainingSymbol != (object)tp.ContainingSymbol)


nit: consider leaving a comment (the error for naming the extension parameter after an extension type parameter is reported elsewhere) #Resolved

jcouv · 2025-03-21T00:07:17Z

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

                    }

                    if (!pNames.Add(name))
                    {
-                        // The parameter name '{0}' is a duplicate
-                        diagnostics.Add(ErrorCode.ERR_DuplicateParamName, GetLocation(p), name);
+                        if (parameters[0] is { ContainingSymbol: NamedTypeSymbol { IsExtension: true }, Name: var receiverName } && receiverName == name)


&& receiverName == name

Consider adding a test to cover this part of the condition (extension(int p1) void M(int p2, int p2)) #Resolved

jcouv · 2025-03-21T00:36:41Z

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

+
+using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
+using System.Runtime.CompilerServices;


nit: unused using? #Resolved

jcouv · 2025-03-21T00:41:02Z

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

+        {
+            Debug.Assert(result.IsClear);
+
+            if ((options & (LookupOptions.NamespaceAliasesOnly | LookupOptions.NamespacesOrTypesOnly)) != 0)


| LookupOptions.NamespacesOrTypesOnly

I didn't follow, what's the reasoning for checking some options early, but letting others be handled by CheckViability?
For comparison, InMethodBinder and WithLambdaParametersBinder only bail out early for NamespaceAliasesOnly. #Resolved

I didn't follow, what's the reasoning for checking some options early, but letting others be handled by

Why do the work if we know that parameters do not satisfy the request. Especially, when we are binding types names in source, we want to avoid having to create symbols for members, etc. This helps with avoiding cycles. In terms of implementation, I used WithPrimaryConstructorParametersBinder as a template.

jcouv · 2025-03-21T00:53:01Z

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

+";
+        var comp = CreateCompilation(src);
+
+        comp.VerifyDiagnostics(


VerifyDiagnostics

nit: consider VerifyEmitDiagnostics over VerifyDiagnostics. Applies to other added tests too

jcouv · 2025-03-21T01:17:06Z

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

-            // (5,14): error CS0102: The type 'Extensions.extension<T>(object)' already contains a definition for 'T'
-            //         void T() { }
-            Diagnostic(ErrorCode.ERR_DuplicateNameInClass, "T").WithArguments("Extensions.extension<T>(object)", "T").WithLocation(5, 14));
+        comp.VerifyEmitDiagnostics();


Consider adding a similar test for an accessor: extension<get_P>(...) int P => 0; #Resolved

jcouv · 2025-03-21T01:19:57Z

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

+            _type = type;
+        }
+
+        internal override void AddLookupSymbolsInfoInSingleBinder(LookupSymbolsInfo result, LookupOptions options, Binder originalBinder)


We probably need some direct LookupSymbols tests to cover this method #Resolved

jcouv

LGTM Thanks (iteration 4) with some minor questions/suggestions

AlekseyTs · 2025-03-21T03:24:03Z

@jjonescz, @dotnet/roslyn-compiler For the second review.

jjonescz · 2025-03-21T10:52:58Z

src/Compilers/CSharp/Portable/CSharpResources.resx

+    <value>'{0}': a parameter, local variable, or local function cannot have the same name as an extension parameter</value>
+  </data>
+  <data name="ERR_ValueParameterSameNameAsExtensionParameter" xml:space="preserve">
+    <value>'value': an automatically-generated parameter name conflicts with an extension parameter name</value>


automatically-generated parameter

I think the language term is "implicit parameter". Same comment for ERR_ValueParameterSameNameAsExtensionTypeParameter. #Resolved

The terminology was "copied" from ERR_DuplicateGeneratedName

Implement scoping and shadowing rules for extension parameter and typ…

6050758

…e parameters See dotnet/csharplang#9229 (commit 9)

AlekseyTs added Area-Compilers Feature - Extension Everything The extension everything feature labels Mar 20, 2025

AlekseyTs requested review from jjonescz and jcouv March 20, 2025 17:35

AlekseyTs requested a review from a team as a code owner March 20, 2025 17:35

dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Mar 20, 2025

Remove an extra empty line

048e179

jcouv self-assigned this Mar 20, 2025

Optimize

57cc979

build-analysis bot mentioned this pull request Mar 20, 2025

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Open

3 tasks

Adjust AssertMemberExposure check

e8db8ca

jcouv reviewed Mar 20, 2025

View reviewed changes

jcouv reviewed Mar 21, 2025

View reviewed changes

jcouv approved these changes Mar 21, 2025

View reviewed changes

PR feedback

b467715

AlekseyTs requested a review from a team March 21, 2025 03:24

AlekseyTs mentioned this pull request Mar 21, 2025

Extensions: definite assignment and region analysis #77675

Merged

jjonescz approved these changes Mar 21, 2025

View reviewed changes

AlekseyTs merged commit 4482c72 into dotnet:features/extensions Mar 21, 2025
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement scoping and shadowing rules for extension parameter and type parameters #77704

Implement scoping and shadowing rules for extension parameter and type parameters #77704

AlekseyTs commented Mar 20, 2025 •

edited by jcouv

Loading

jcouv Mar 20, 2025 •

edited

Loading

AlekseyTs Mar 20, 2025

jcouv Mar 20, 2025 •

edited

Loading

AlekseyTs Mar 20, 2025

jcouv Mar 20, 2025 •

edited by AlekseyTs

Loading

AlekseyTs Mar 20, 2025 •

edited

Loading

jcouv Mar 21, 2025

jcouv Mar 20, 2025 •

edited by AlekseyTs

Loading

jcouv Mar 21, 2025 •

edited by AlekseyTs

Loading

jcouv Mar 21, 2025 •

edited by AlekseyTs

Loading

jcouv Mar 21, 2025 •

edited by AlekseyTs

Loading

AlekseyTs Mar 21, 2025 •

edited

Loading

jcouv Mar 21, 2025

jcouv Mar 21, 2025 •

edited by AlekseyTs

Loading

jcouv Mar 21, 2025 •

edited by AlekseyTs

Loading

jcouv left a comment

AlekseyTs commented Mar 21, 2025

jjonescz Mar 21, 2025 •

edited by AlekseyTs

Loading

AlekseyTs Mar 21, 2025

Implement scoping and shadowing rules for extension parameter and type parameters #77704

Implement scoping and shadowing rules for extension parameter and type parameters #77704

Conversation

AlekseyTs commented Mar 20, 2025 • edited by jcouv Loading

jcouv Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Mar 20, 2025

Choose a reason for hiding this comment

jcouv Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Mar 20, 2025

Choose a reason for hiding this comment

jcouv Mar 20, 2025 • edited by AlekseyTs Loading

Choose a reason for hiding this comment

AlekseyTs Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

jcouv Mar 21, 2025

Choose a reason for hiding this comment

jcouv Mar 20, 2025 • edited by AlekseyTs Loading

Choose a reason for hiding this comment

jcouv Mar 21, 2025 • edited by AlekseyTs Loading

Choose a reason for hiding this comment

jcouv Mar 21, 2025 • edited by AlekseyTs Loading

Choose a reason for hiding this comment

jcouv Mar 21, 2025 • edited by AlekseyTs Loading

Choose a reason for hiding this comment

AlekseyTs Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

jcouv Mar 21, 2025

Choose a reason for hiding this comment

jcouv Mar 21, 2025 • edited by AlekseyTs Loading

Choose a reason for hiding this comment

jcouv Mar 21, 2025 • edited by AlekseyTs Loading

Choose a reason for hiding this comment

jcouv left a comment

Choose a reason for hiding this comment

AlekseyTs commented Mar 21, 2025

jjonescz Mar 21, 2025 • edited by AlekseyTs Loading

Choose a reason for hiding this comment

AlekseyTs Mar 21, 2025

Choose a reason for hiding this comment

AlekseyTs commented Mar 20, 2025 •

edited by jcouv

Loading

jcouv Mar 20, 2025 •

edited

Loading

jcouv Mar 20, 2025 •

edited

Loading

jcouv Mar 20, 2025 •

edited by AlekseyTs

Loading

AlekseyTs Mar 20, 2025 •

edited

Loading

jcouv Mar 20, 2025 •

edited by AlekseyTs

Loading

jcouv Mar 21, 2025 •

edited by AlekseyTs

Loading

jcouv Mar 21, 2025 •

edited by AlekseyTs

Loading

jcouv Mar 21, 2025 •

edited by AlekseyTs

Loading

AlekseyTs Mar 21, 2025 •

edited

Loading

jcouv Mar 21, 2025 •

edited by AlekseyTs

Loading

jcouv Mar 21, 2025 •

edited by AlekseyTs

Loading

jjonescz Mar 21, 2025 •

edited by AlekseyTs

Loading