Extensions: pattern-based constructs

[jcouv]

This is mostly a test-focused PR which aims to:

1. validate pattern-based constructs and address problems with those,
2. address some low-hanging follow-up comments that just need testing.

The product changes are:

1. block delegate-returning properties in the few scenarios where they accidentally got allowed (for instance, GetEnumerator, MoveNext, Add, ...) to match the behavior of instance members
2. minor change to handle the extension parameter in ref analysis of positional patterns
3. adjust an error message
4. add implicit conversion on receiver in some scenarios

Coverage

Out-of scope for this PR

• collection/dictionary expressions (need to discuss spec first).
• dictionary initializer: new C() { [0] = ... }
• Index/Range indexers in list/spread patterns

Scenarios where extension methods play

• GetEnumerator/GetAsyncEnumerator in foreach (Nullability_ForEach_01)
• Deconstruct in deconstruction, in positional pattern and foreach (Nullability_Deconstruct_xx, Nullability_PositionalPattern_xx, Nullability_ForeachDeconstruct_xx)
• Add in collection initializers (see ExtensionMemberLookup_CollectionInitializer_NoMethod, Nullability_CollectionInitializer_01)
• GetPinnableReference in fixed (see Nullability_Fixed_01)
• GetAwaiter in await (see Nullability_Await_GetAwaiter_01)

Scenarios where extension methods don't play

• Dispose/DisposeAsync in using and foreach (see ExtensionMemberLookup_PatternBased_Dispose_RefStruct, ExtensionMemberLookup_PatternBased_Dispose_Async_NoMethod, TestPatternBasedDisposal_ExtensionMethod, TODO test in foreach)
• MoveNext/MoveNextAsync in foreach (see TestMoveNextPatternViaExtensions_OnInstanceGetEnumerator, InstanceMethodInvocation_PatternBased_ForEach_MoveNext, TestMoveNextAsyncPatternViaExtensions_01)
• Slice and int indexers in implicit indexers and list-patterns (see ExtensionMemberLookup_PatternBased_RangeIndexer_..., ExtensionMemberLookup_PatternBased_IndexIndexer_..., ExtensionMemberLookup_PatternBased_ListPattern_...
• GetResult in await (see ExtensionMemberLookup_PatternBased_Await_ExtensionGetResult)

Scenarios where extension properties play

• object initializer: new C() { ExtensionProperty = ... } (see ExtensionMemberLookup_ObjectInitializer, Nullability_ObjectInitializer_01, Nullability_ObjectInitializer_02)
• with: x with { ExtensionProperty = ... } (see ExtensionMemberLookup_With, Nullability_With_xx)
• property patterns: x is { ExtensionProperty: ... } (see ExtensionMemberLookup_Patterns, ExtensionMemberLookup_Patterns_ExtendedPropertyPattern)

Scenarios where extension properties don't play

• Current in foreach (see InstanceMethodInvocation_PatternBased_ForEach_Current)
• IsCompleted in await (see ExtensionMemberLookup_PatternBased_Await_ExtensionIsCompleted)
• Count/Length properties and indexers in list/spread pattern (see ExtensionMemberLookup_PatternBased_SpreadPattern_..., ExtensionMemberLookup_PatternBased_ListPattern_...)
• Count/Length properties and indexers in implicit indexers (see ExtensionMemberLookup_PatternBased_IndexIndexer_..., ExtensionMemberLookup_PatternBased_RangeIndexer_...)

Relates to test plan #76130

[jcouv]

📝 Covered by Deconstruct_01 and Deconstruct_02

[jcouv]

📝 Covered by PositionalPattern_01

[jcouv]

📝 Covered by Deconstruction_UnscopedRef_ExtensionMethod and Deconstruction_ScopedRef_ExtensionMethod

[jcouv]

📝 Covered by Nullability_Method_04

[jcouv]

public void InstanceMethodInvocation_PatternBased_ForEach_MoveNext()

📝 the test was split (one now focuses on MoveNext() and the other focuses on Current)

---

Refers to: src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs:12760 in 11f86e6. [](commit_id = 11f86e6, deletion_comment = False)

[AlekseyTs]

return method.ContainingType.ExtensionParameter;

It is not clear why do we want to have this parameter at all. How does this align with the language spec? #Closed

[jcouv]

I'm not sure I understand the question.
From a ref analysis perspective, it should be equivalent to deconstruct via invocation, deconstruction or recursive pattern. So all three need to account for scoped on the receiver parameter. Here's we're looking at a skeleton signature, so this is the parameter we need to check for presence or absence of scoped. See Deconstruction_ScopedRef_ExtensionMethod.
I can adjust the name of the local function (maybe "tryGetReceiverParameter") if that's what bothers you.

[AlekseyTs]

member

Should we check !IsStatic? #Closed

[AlekseyTs]

compilation.Conversions.ClassifyConversionFromType

Could we use IsValidExtensionMethodThisArgConversion instead? #Closed

[AlekseyTs]

memberSymbol

Is the member supposed to be an instance member? #Closed

[jjonescz]

formatting

Suggested change

	}}
	}
	}
	``` #Resolved

[jjonescz]

If the intention is to test all patterns including those that don't work, consider testing also Lock pattern (https://github.com/dotnet/csharplang/blob/main/proposals/csharp-13.0/lock-object.md) #Pending

[jcouv]

I'll add a test for new Lock, but I'm not planning to cover all well-known APIs that the compiler uses ;-) Those won't be subject to extensions (they are not today).

[jjonescz]

Consider making this static readonly instead of const to avoid duplicating the string in each DLL. See #74307 #Resolved

[AlekseyTs]

// Tracked by #76130 : report error for declaring type named "extension"

Has this been addressed? #Resolved

[jcouv]

Yes, this was done in a previous PR, but this comment was missed.

    [Fact]
    public void ReservedTypeName_01()
    {
        string source = """
using extension = int;
class extension { }
class C<extension> { }
""";

        var comp = CreateCompilation(source, parseOptions: TestOptions.Regular13);
        comp.VerifyEmitDiagnostics(
            // (1,1): hidden CS8019: Unnecessary using directive.
            // using extension = int;
            Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using extension = int;").WithLocation(1, 1),
            // (1,7): warning CS8981: The type name 'extension' only contains lower-cased ascii characters. Such names may become reserved for the language.
            // using extension = int;
            Diagnostic(ErrorCode.WRN_LowerCaseTypeName, "extension").WithArguments("extension").WithLocation(1, 7),
            // (2,7): warning CS8981: The type name 'extension' only contains lower-cased ascii characters. Such names may become reserved for the language.
            // class extension { }
            Diagnostic(ErrorCode.WRN_LowerCaseTypeName, "extension").WithArguments("extension").WithLocation(2, 7),
            // (3,9): warning CS8981: The type name 'extension' only contains lower-cased ascii characters. Such names may become reserved for the language.
            // class C<extension> { }
            Diagnostic(ErrorCode.WRN_LowerCaseTypeName, "extension").WithArguments("extension").WithLocation(3, 9));

        comp = CreateCompilation(source, parseOptions: TestOptions.RegularNext);
        comp.VerifyEmitDiagnostics(
            // (1,7): error CS9306: Types and aliases cannot be named 'extension'.
            // using extension = int;
            Diagnostic(ErrorCode.ERR_ExtensionTypeNameDisallowed, "extension").WithLocation(1, 7),
            // (2,7): error CS9306: Types and aliases cannot be named 'extension'.
            // class extension { }
            Diagnostic(ErrorCode.ERR_ExtensionTypeNameDisallowed, "extension").WithLocation(2, 7),
            // (3,9): error CS9306: Types and aliases cannot be named 'extension'.
            // class C<extension> { }
            Diagnostic(ErrorCode.ERR_ExtensionTypeNameDisallowed, "extension").WithLocation(3, 9));

        comp = CreateCompilation(source);
        comp.VerifyEmitDiagnostics(
            // (1,7): error CS9306: Types and aliases cannot be named 'extension'.
            // using extension = int;
            Diagnostic(ErrorCode.ERR_ExtensionTypeNameDisallowed, "extension").WithLocation(1, 7),
            // (2,7): error CS9306: Types and aliases cannot be named 'extension'.
            // class extension { }
            Diagnostic(ErrorCode.ERR_ExtensionTypeNameDisallowed, "extension").WithLocation(2, 7),
            // (3,9): error CS9306: Types and aliases cannot be named 'extension'.
            // class C<extension> { }
            Diagnostic(ErrorCode.ERR_ExtensionTypeNameDisallowed, "extension").WithLocation(3, 9));
    }

[AlekseyTs]

acceptOnlyMethods: acceptOnlyMethods);

Are we testing all call sites that pass true? #Closed

[AlekseyTs]

public void ExtensionMemberLookup_PatternBased_Fixed_NoApplicableMethod()

Is the "NoApplicableMethod" suffix still accurate? #Resolved

---

Refers to: src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs:21063 in b6cc211. [](commit_id = b6cc211, deletion_comment = False)

[AlekseyTs]

LGTM (commit 7)