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.

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

Update "use collection initializer" to use a collection-expression if the user prefers that form. #69219

Merged
merged 44 commits into from
Jul 28, 2023

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented Jul 25, 2023

Part of #69132

Also expands on the prior form. So if the user has:

List<int> list = new List<int>();
list.Add(x);
foreach (var v in y)
    list.Add(v);

This can now be updated to:

List<int> list = [x, .. y];  // We support spreads now!

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 25, 2023
{
var elements = CreateElements<CollectionElementSyntax>(
objectCreation, matches,
static (match, expression) => match?.UseSpread is true ? SpreadElement(expression) : ExpressionElement(expression));
Copy link
Member Author

Choose a reason for hiding this comment

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

similar behavior to normal collection initializers. however, here we may produce spread elements, not just normal expressions. the 'Match' value tells us if we want that. 'match' is nullable as some of the expressions provided here came from the original object creation expression, and thus have no match data.

@@ -253,7 +253,7 @@ private static void AddBracketIndentationOperation(List<IndentBlockOperation> li
return;
}

if (node.IsKind(SyntaxKind.ListPattern) && node.Parent != null)
if (node.Parent != null && node.Kind() is SyntaxKind.ListPattern or SyntaxKind.CollectionExpression)
Copy link
Member Author

Choose a reason for hiding this comment

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

generally following the correspondance that list patterns and collection expressions should be formatted similarly.

=> ImmutableArray.Create(IDEDiagnosticIds.UseCollectionInitializerDiagnosticId);

protected override bool IncludeDiagnosticDuringFixAll(Diagnostic diagnostic)
protected abstract TStatementSyntax GetNewStatement(
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 thsi abstract method from below to here.

BaseObjectCreationExpressionSyntax objectCreation,
ImmutableArray<Match<StatementSyntax>> matches,
Func<Match<StatementSyntax>?, ExpressionSyntax, TElement> createElement)
where TElement : SyntaxNode
Copy link
Member Author

Choose a reason for hiding this comment

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

this is similar cod eto what we had before. Except:

  1. newline handling is pulled out of this to the callers.
  2. this is generified so it can make expression lists (for collection initializers) or element lists (for collection expressions).

else
nodesAndTokens.Add(createElement(null, (ExpressionSyntax)nodeOrToken.AsNode()!));
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

updated as the different features have to be able to customer what sort of lists they get (either expressions or elements)

var list = new List<int>
{
1
};
Copy link
Member Author

Choose a reason for hiding this comment

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

this was actually a bug from before that got fixed. top level statemnets weren't properly wrapping (like what happens in a normal statement).


return;

(ImmutableArray<Match<TStatementSyntax>> matches, bool shouldUseCollectionExpression) GetMatches()
Copy link
Member Author

Choose a reason for hiding this comment

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

new logic that first tries to see if we can convert to a collection expr. otherwise, falls back to original approach of trying a collection-initializer.

allows us to share the majorty of code and concepts.

@CyrusNajmabadi CyrusNajmabadi merged commit 7a592d3 into dotnet:main Jul 28, 2023
22 of 24 checks passed
@CyrusNajmabadi CyrusNajmabadi deleted the useCollectionExpression2 branch July 28, 2023 20:25
@ghost ghost added this to the Next milestone Jul 28, 2023
@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
Labels
Area-IDE New Feature - Collection Expressions untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants