Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support converting arrays, whose elements are assigned afterwards, to a collection expression. #69415

Merged
merged 10 commits into from
Aug 9, 2023
Merged

Support converting arrays, whose elements are assigned afterwards, to a collection expression. #69415

merged 10 commits into from
Aug 9, 2023

Conversation

CyrusNajmabadi
Copy link
Member

Part of #69132

Followup to #69414. That PR should go in first before reviewing this.

Updates the cases we handle to support:

int[] x = new int[3];
x[0] = a;
x[1] = b;
x[2] = c;

// And converts it to
int[] x = [a, b, c];

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner August 5, 2023 21:46
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 5, 2023
@CyrusNajmabadi CyrusNajmabadi mentioned this pull request Aug 5, 2023
Closed
14 tasks
@CyrusNajmabadi CyrusNajmabadi changed the title Support converting arrays, whose elements are added afterwards, to a collection expression. Support converting arrays, whose elements are assigned afterwards, to a collection expression. Aug 5, 2023
@CyrusNajmabadi CyrusNajmabadi requested a review from akhera99 August 5, 2023 21:54
@CyrusNajmabadi CyrusNajmabadi marked this pull request as draft August 5, 2023 21:54
@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review August 8, 2023 22:05
CyrusNajmabadi
CyrusNajmabadi commented Aug 8, 2023
View reviewed changes
...zers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs
@@ -158,94 +154,11 @@ private static void AnalyzeExplicitStackAllocExpression(SyntaxNodeAnalysisContex
StackAllocArrayCreationExpressionSyntax expression,
CancellationToken cancellationToken)
{
// has to either be `stackalloc X[]` or `stackalloc X[const]`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to common helper code, since we'll use it for stackalloc and new array creations.

CyrusNajmabadi
CyrusNajmabadi commented Aug 8, 2023
View reviewed changes
src/Analyzers/CSharp/Analyzers/UseCollectionExpression/UseCollectionExpressionHelpers.cs
CancellationToken cancellationToken)
where TArrayCreationExpressionSyntax : ExpressionSyntax
{
Contract.ThrowIfFalse(expression is ArrayCreationExpressionSyntax or StackAllocArrayCreationExpressionSyntax);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a move.

CyrusNajmabadi
CyrusNajmabadi commented Aug 8, 2023
View reviewed changes
src/Analyzers/CSharp/Analyzers/UseCollectionExpression/CollectionExpressionMatch.cs
@@ -5,7 +5,7 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.UseCollectionInitializer;

namespace Microsoft.CodeAnalysis.UseCollectionExpression;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few types were in the wrong namespace. fixed up with downstream consequences.

CyrusNajmabadi
CyrusNajmabadi commented Aug 8, 2023
View reviewed changes
...Analyzers/UseCollectionExpression/CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.cs
expression,
static e => e.Type,
static e => e.Initializer,
cancellationToken);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calls into new helper that shares logic with stackalloc creation analysis.

CyrusNajmabadi
CyrusNajmabadi commented Aug 8, 2023
View reviewed changes
...zers/UseCollectionExpression/CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.cs
}

return matches.ToImmutable();
return UseCollectionExpressionHelpers.TryGetMatches(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calls into the shared helper now.

@CyrusNajmabadi
Update src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpU…
6b2758c 
…seCollectionExpressionForArrayCodeFixProvider.cs
@CyrusNajmabadi
Copy link
Member Author

@akhera99 this is ready for review.

CyrusNajmabadi
CyrusNajmabadi commented Aug 9, 2023
View reviewed changes
...rp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs
Document document,
ImmutableArray<Diagnostic> diagnostics,
SyntaxEditor editor,
CodeActionOptionsProvider fallbackOptions,
CancellationToken cancellationToken)
{
var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed this up to follow the pattern we use for all the rest of the rewriters. important, this handles successive rewrites with arrays within arrays.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trying to decide if this pattern should be extracted out somewhere as it's now done in like 4 places. may do a followup where that happens.

CyrusNajmabadi
CyrusNajmabadi commented Aug 9, 2023
View reviewed changes
src/Analyzers/CSharp/Tests/UseCollectionExpression/UseCollectionExpressionForArrayTests.cs
{
void M(int i, int j)
{
int[][] r = [[1], [2]];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

demonstrates recursive fixall working.

@CyrusNajmabadi
Copy link
Member Author

@akhera99 ptal :)

@CyrusNajmabadi CyrusNajmabadi merged commit c3ac4e1 into dotnet:main Aug 9, 2023
@CyrusNajmabadi CyrusNajmabadi deleted the useCollectionExpressionArrayInit branch August 9, 2023 19:15
@ghost ghost added this to the Next milestone Aug 9, 2023
@allisonchou allisonchou mentioned this pull request Aug 16, 2023
Closed
@dibarbet dibarbet modified the milestones: Next, 17.8 P2 Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akhera99 akhera99 akhera99 approved these changes

Assignees
No one assigned
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Milestone
17.8 P2
Development

Successfully merging this pull request may close these issues.
3 participants
@CyrusNajmabadi @akhera99 @dibarbet