Skip to content

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

Merged
CyrusNajmabadi merged 10 commits intodotnet:mainfrom
CyrusNajmabadi:useCollectionExpressionArrayInit
Aug 9, 2023
Merged

Support converting arrays, whose elements are assigned afterwards, to a collection expression.#69415
CyrusNajmabadi merged 10 commits intodotnet:mainfrom
CyrusNajmabadi:useCollectionExpressionArrayInit

Conversation

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Aug 5, 2023

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
@ghost ghost 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
StackAllocArrayCreationExpressionSyntax expression,
CancellationToken cancellationToken)
{
// has to either be `stackalloc X[]` or `stackalloc X[const]`.
Copy link
Copy Markdown
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 8, 2023

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
Copy Markdown
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 8, 2023

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
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.UseCollectionInitializer;

namespace Microsoft.CodeAnalysis.UseCollectionExpression;
Copy link
Copy Markdown
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 8, 2023

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
Copy Markdown
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 8, 2023

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
Copy Markdown
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 8, 2023

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
Copy Markdown
Contributor Author

CyrusNajmabadi commented Aug 8, 2023

@akhera99 this is ready for review.

CyrusNajmabadi
CyrusNajmabadi commented Aug 9, 2023
View reviewed changes
...rp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForArrayCodeFixProvider.cs
CodeActionOptionsProvider fallbackOptions,
CancellationToken cancellationToken)
{
var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
Copy link
Copy Markdown
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 9, 2023

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
Copy Markdown
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 9, 2023

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
Copy Markdown
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 9, 2023

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
Copy Markdown
Contributor Author

CyrusNajmabadi commented Aug 9, 2023

@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