Allow collection expressions with CreateCompilation
#71134
Conversation
dotnet-issue-labeler
bot
added
the
untriaged
|
@dotnet/roslyn-compiler Small PR for review. Thanks |
1 similar comment
|
@dotnet/roslyn-compiler Small PR for review. Thanks |
| IEnumerator<CSharpTestSource> IEnumerable<CSharpTestSource>.GetEnumerator() => throw new NotImplementedException(); | ||
| IEnumerator IEnumerable.GetEnumerator() => throw new NotImplementedException(); | ||
|
|
||
| internal class CSharpTestSourceBuilder |
There was a problem hiding this comment.
Can this be static? Or sealed at least.
|
@dotnet/roslyn-compiler for second review. Thanks |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| #if !NET8_0_OR_GREATER |
There was a problem hiding this comment.
Just to make sure that we don't run afoul of any weird situations (such as razor test dependencies), we should typeforward in an #else: https://github.com/dotnet/roslyn/blob/main/docs/Target%20Framework%20Strategy.md#pattern-for-types #Resolved
There was a problem hiding this comment.
Thanks for the pointer. That's a useful doc
| IEnumerator<CSharpTestSource> IEnumerable<CSharpTestSource>.GetEnumerator() => throw new NotImplementedException(); | ||
| IEnumerator IEnumerable.GetEnumerator() => throw new NotImplementedException(); | ||
|
|
||
| internal static class CSharpTestSourceBuilder |
There was a problem hiding this comment.
Why does this need to be a separate type? Why not just make it a static member on this type? #WontFix
There was a problem hiding this comment.
It does not need to be a separate type, but it is customary for the collection and the collection builder to be different types, and I think it makes sense. See all the collection tests
There was a problem hiding this comment.
but it is customary for the collection and the collection builder to be different types
This is a brand new pattern, there's no "customary" at all 😆. I'd just as soon keep it simpler.
There was a problem hiding this comment.
All our collection builder tests beg to differ. The spec also calls out two different concepts: a "collection type" and a "builder type". It is clearer not to blend them and doesn't seem "simpler".
In any case, this seems a nitpick/preference, not a reason to hold this change up. Thanks
ghost
added this to the 
No description provided.