Add support for with(...) elements in collection expressions.#80495
Conversation
| @@ -0,0 +1,21 @@ | |||
| # Breaking changes in Roslyn after .NET 10.0.100 through .NET 11 (version number tbd) | |||
There was a problem hiding this comment.
not sure what to put here since we don't have the release numbers for these things yet.
There was a problem hiding this comment.
Pull Request Overview
This PR adds parsing support for with(...) elements in collection expressions as part of implementing collection expression arguments. The work represents initial parsing implementation with basic error handling, while binding and emit functionality remain incomplete.
- Introduces
WithElementSyntaxas a new collection element type forwith(...)syntax - Adds language version gating for the new feature (requires C# Preview/C# 14+)
- Updates parser to recognize
withfollowed by parenthesized argument list as collection arguments
Reviewed Changes
Copilot reviewed 26 out of 32 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Compilers/CSharp/Portable/Syntax/SyntaxKind.cs | Adds WithElement syntax kind enumeration |
| src/Compilers/CSharp/Portable/Syntax/Syntax.xml | Defines WithElementSyntax node structure with WithKeyword and ArgumentList |
| src/Compilers/CSharp/Portable/Parser/LanguageParser.cs | Implements parsing logic for with(...) collection elements |
| src/Compilers/CSharp/Portable/Errors/*.cs | Adds error codes and messages for collection argument validation |
| src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs | Provides basic binding with error reporting (incomplete implementation) |
| Various generated files | Auto-generated syntax tree visitors, factory methods, and API surface |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests_WithElement_Constructor.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests_WithElement_Constructor.cs
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests_WithElement_Constructor.cs
Outdated
Show resolved
Hide resolved
|
Taking a look |
src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CollectionExpression.cs
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests_WithElement_Constructor.cs
Outdated
Show resolved
Hide resolved
| CreateCompilation(source, targetFramework: TargetFramework.Net80).VerifyDiagnostics( | ||
| // (8,14): error CS9244: The type 'ReadOnlySpan<int>' may not be a ref struct or a type parameter allowing ref structs in order to use it as parameter 'T' in the generic type or method 'List<T>' | ||
| // List<ReadOnlySpan<int>> list = [[with(), 1, 2, 3]]; | ||
| Diagnostic(ErrorCode.ERR_NotRefStructConstraintNotSatisfied, "ReadOnlySpan<int>").WithArguments("System.Collections.Generic.List<T>", "T", "System.ReadOnlySpan<int>").WithLocation(8, 14), |
There was a problem hiding this comment.
It might be worthwhile to also test IEnumerable<ReadOnlySpan<int>> which is a valid type (maybe starting in net9?), but tends to break things as it's not a valid type for a collection expression
There was a problem hiding this comment.
interface work comes in teh followup PR. i'd like to add that test there if i may.
| } | ||
| """; | ||
|
|
||
| // PROTOTYPE: This error is not correct. |
There was a problem hiding this comment.
Are you saying that a different error message should be reported instead?
There was a problem hiding this comment.
yes. the current diagnostic says: "must have an applicable constructor that can be called with no arguments."
That's not true. The issue here is that it has two parameters, but only one arg is being supplied. so we should hav a different error message.
so: good that we have an error. but bad that the error message is incorrect.
|
|
||
| CreateCompilation(source).VerifyDiagnostics( | ||
| // (12,28): error CS1729: 'MyList<int>' does not contain a constructor that takes 0 arguments | ||
| // MyList<int> list = [with(10)]; |
There was a problem hiding this comment.
nit: normally we would report that the candidate isn't accessible. can track this in followup or whatever as preferred.
| } | ||
|
|
||
| [Fact] | ||
| public void WithElement_WithLambda_InferenceWithArgAndConstructor_3() |
There was a problem hiding this comment.
Did any of these tests demonstrate that the 'with' element is not contributing to type inference? e.g. perhaps a test showing M<T>(MyCollection<T>) should be ambiguous for M([with(value)]).
01334a7
into
dotnet:features/collection-expression-arguments
Followup to #80545
This is only for collections creatable with 'new', as well as the right errors for arrays/spans. Interfaces/
Createcollections will also error, but that will change in followup PRs.Relates to test plan #80613