-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add more cases supported by 'use collection expression' #75879
Conversation
@@ -20,11 +20,6 @@ internal sealed partial class CSharpUseCollectionExpressionForCreateDiagnosticAn | |||
IDEDiagnosticIds.UseCollectionExpressionForCreateDiagnosticId, | |||
EnforceOnBuildValues.UseCollectionExpressionForCreate) | |||
{ | |||
public const string UnwrapArgument = nameof(UnwrapArgument); | |||
|
|||
private static readonly ImmutableDictionary<string, string?> s_unwrapArgumentProperties = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to common location
var properties = unwrapArgument ? s_unwrapArgumentProperties : ImmutableDictionary<string, string?>.Empty; | ||
if (changesSemantics) | ||
properties = properties.Add(UseCollectionInitializerHelpers.ChangesSemanticsName, ""); | ||
var properties = GetDiagnosticProperties(unwrapArgument, useSpread, changesSemantics); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to common location
nameof(ParallelEnumerable), | ||
nameof(ParallelQuery), | ||
// Special internal runtime interface that is optimized for fast path conversions of collections. | ||
"IIListProvider"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to common location
Implements(type, compilation.IReadOnlyListOfTType()); | ||
} | ||
|
||
bool IsIterable(ExpressionSyntax expression) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to common location
internal sealed partial class CSharpUseCollectionExpressionForNewDiagnosticAnalyzer() | ||
: AbstractCSharpUseCollectionExpressionDiagnosticAnalyzer( | ||
IDEDiagnosticIds.UseCollectionExpressionForNewDiagnosticId, | ||
EnforceOnBuildValues.UseCollectionExpressionForNew) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
supports converting new List<...>(x)
to [.. x]
{ | ||
var argExpression = arguments[0].Expression; | ||
if (argExpression |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to helper.
{ | ||
void M() | ||
{ | ||
const bool shouldParallelize = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this necessary?
} | ||
|
||
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71145")] | ||
public async Task TestNotInParallelEnumerable2() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Not" in the name makes me think it should not convert to a collection expression
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. We didn't update hte parallel one, only the non parallel one :)
Fixes #75870