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

Add support for generating the .. x ? [y] : [] pattern in a collection-expression #69280

Merged
merged 12 commits into from
Aug 2, 2023
Merged

Add support for generating the .. x ? [y] : [] pattern in a collection-expression #69280

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
00eadd6
Simplify
CyrusNajmabadi Jul 28, 2023
51e266a
In progress
CyrusNajmabadi Jul 28, 2023
a96d399
Merge branch 'useCollectionExpression3' into useCollectionExpression4
CyrusNajmabadi Jul 28, 2023
7fe6746
in progress
CyrusNajmabadi Jul 28, 2023
e6481fb
Typesafe
CyrusNajmabadi Jul 28, 2023
a302e5d
testS
CyrusNajmabadi Jul 28, 2023
e9858ea
Fixes and tests
CyrusNajmabadi Jul 28, 2023
cde8879
Merge branch 'useCollectionExpression3' into useCollectionExpression4
CyrusNajmabadi Jul 28, 2023
bfc5393
Merge branch 'main' into useCollectionExpression4
CyrusNajmabadi Jul 28, 2023
6764a5a
Merge branch 'useCollectionExpression3' into useCollectionExpression4
CyrusNajmabadi Jul 31, 2023
d073f13
Merge branch 'main' into useCollectionExpression4
CyrusNajmabadi Jul 31, 2023
027d00c
formattign
CyrusNajmabadi Jul 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
In progress
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi committed Jul 28, 2023
commit 51e266a5a4b56a0b4ee49c8c96f818547fb73aae
14 changes: 12 additions & 2 deletions .../Analyzers/UseCollectionInitializer/AbstractUseCollectionInitializerDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@

namespace Microsoft.CodeAnalysis.UseCollectionInitializer
{
/// <summary>
/// Represents statements following an object initializer that should be converted into
/// collection-initializer/expression elements.
/// </summary>
/// <param name="Statement">The statement that follows that contains the values to add to the new
/// collection-initializer or collection-expression</param>
/// <param name="UseSpread">Whether or not a spread (<c>.. x</c>) element should be created for this statement. This
/// is needed as the statement could be cases like <c>expr.Add(x)</c> vs. <c>expr.AddRange(x)</c>. This property
/// indicates that the latter should become a spread, without the consumer having to reexamine the statement to see
/// what form it is.</param>
internal readonly record struct Match<TStatementSyntax>(
TStatementSyntax Statement,
bool UseSpread) where TStatementSyntax : SyntaxNode;
Expand Down Expand Up @@ -158,7 +168,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien
(ImmutableArray<Match<TStatementSyntax>> matches, bool shouldUseCollectionExpression)? GetCollectionInitializerMatches()
{
var matches = UseCollectionInitializerAnalyzer<
TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TVariableDeclaratorSyntax>.Analyze(
TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, TVariableDeclaratorSyntax>.Analyze(
semanticModel, GetSyntaxFacts(), objectCreationExpression, areCollectionExpressionsSupported: false, cancellationToken);

// If analysis failed, we can't change this, no matter what.
Expand All @@ -181,7 +191,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien
return null;

var matches = UseCollectionInitializerAnalyzer<
TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TVariableDeclaratorSyntax>.Analyze(
TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, TVariableDeclaratorSyntax>.Analyze(
semanticModel, GetSyntaxFacts(), objectCreationExpression, areCollectionExpressionsSupported: true, cancellationToken);

// If analysis failed, we can't change this, no matter what.
Expand Down
206 changes: 140 additions & 66 deletions src/Analyzers/Core/Analyzers/UseCollectionInitializer/UseCollectionInitializerAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ internal sealed class UseCollectionInitializerAnalyzer<
TInvocationExpressionSyntax,
TExpressionStatementSyntax,
TForeachStatementSyntax,
TIfStatementSyntax,
TVariableDeclaratorSyntax> : AbstractObjectCreationExpressionAnalyzer<
TExpressionSyntax,
TStatementSyntax,
Expand All @@ -34,10 +35,11 @@ internal sealed class UseCollectionInitializerAnalyzer<
where TInvocationExpressionSyntax : TExpressionSyntax
where TExpressionStatementSyntax : TStatementSyntax
where TForeachStatementSyntax : TStatementSyntax
where TIfStatementSyntax : TStatementSyntax
where TVariableDeclaratorSyntax : SyntaxNode
{
private static readonly ObjectPool<UseCollectionInitializerAnalyzer<TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TVariableDeclaratorSyntax>> s_pool
= SharedPools.Default<UseCollectionInitializerAnalyzer<TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TVariableDeclaratorSyntax>>();
private static readonly ObjectPool<UseCollectionInitializerAnalyzer<TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, TVariableDeclaratorSyntax>> s_pool
= SharedPools.Default<UseCollectionInitializerAnalyzer<TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, TVariableDeclaratorSyntax>>();

public static ImmutableArray<Match<TStatementSyntax>> Analyze(
SemanticModel semanticModel,
Expand Down Expand Up @@ -107,86 +109,158 @@ protected override void AddMatches(ArrayBuilder<Match<TStatementSyntax>> matches
continue;
}

if (extractedChild is TExpressionStatementSyntax expressionStatement)
if (extractedChild is TExpressionStatementSyntax expressionStatement &&
TryProcessExpressionStatement(expressionStatement))
{
if (!seenIndexAssignment)
continue;
}
else if (extractedChild is TForeachStatementSyntax foreachStatement &&
TryProcessForeachStatement(foreachStatement))
{
continue;
}
else if (extractedChild is TIfStatementSyntax ifStatement &&
TryProcessIfStatement(ifStatement))
{
continue;
}

// Something we didn't understand. Stop here.
return;
}

bool TryProcessExpressionStatement(TExpressionStatementSyntax expressionStatement)
{
// Can't mix Adds and indexing.
if (!seenIndexAssignment)
{
// Look for a call to Add or AddRange
if (TryAnalyzeInvocation(
expressionStatement,
addName: WellKnownMemberNames.CollectionInitializerAddMethodName,
requiredArgumentName: null,
out var instance) &&
ValuePatternMatches(instance))
{
// Look for a call to Add or AddRange
if (TryAnalyzeInvocation(
expressionStatement,
addName: WellKnownMemberNames.CollectionInitializerAddMethodName,
requiredArgumentName: null,
out var instance) &&
ValuePatternMatches(instance))
{
seenInvocation = true;
matches.Add(new Match<TStatementSyntax>(expressionStatement, UseSpread: false));
continue;
}
else if (
_analyzeForCollectionExpression &&
TryAnalyzeInvocation(
expressionStatement,
addName: nameof(List<int>.AddRange),
requiredArgumentName: null,
out instance))
{
seenInvocation = true;
seenInvocation = true;
matches.Add(new Match<TStatementSyntax>(expressionStatement, UseSpread: false));
return true;
}
else if (
_analyzeForCollectionExpression &&
TryAnalyzeInvocation(
expressionStatement,
addName: nameof(List<int>.AddRange),
requiredArgumentName: null,
out instance))
{
seenInvocation = true;

// AddRange(x) will become `..x` when we make it into a collection expression.
matches.Add(new Match<TStatementSyntax>(expressionStatement, UseSpread: true));
continue;
}
// AddRange(x) will become `..x` when we make it into a collection expression.
matches.Add(new Match<TStatementSyntax>(expressionStatement, UseSpread: true));
return true;
}
}

// Can't mix Adds and indexing. Can't use indexing with a collection expression.
if (!seenInvocation && !_analyzeForCollectionExpression)
{
if (TryAnalyzeIndexAssignment(expressionStatement, out var instance))
{
seenIndexAssignment = true;
matches.Add(new Match<TStatementSyntax>(expressionStatement, UseSpread: false));
return true;
}
}

return false;
}

if (!seenInvocation && !_analyzeForCollectionExpression)
bool TryProcessForeachStatement(TForeachStatementSyntax foreachStatement)
{
// if we're not producing a collection expression, then we cannot convert any foreach'es into
// `[..expr]` elements.
if (_analyzeForCollectionExpression)
{
_syntaxFacts.GetPartsOfForeachStatement(foreachStatement, out var identifier, out _, out var foreachStatements);
if (identifier != default)
{
if (TryAnalyzeIndexAssignment(expressionStatement, out var instance))
// must be of the form:
//
// foreach (var x in expr)
// dest.Add(x)
//
// By passing 'x' into TryAnalyzeInvocation below, we ensure that it is an enumerated value from `expr`
// being added to `dest`.
if (foreachStatements.ToImmutableArray() is [TExpressionStatementSyntax childExpressionStatement] &&
TryAnalyzeInvocation(
childExpressionStatement,
addName: WellKnownMemberNames.CollectionInitializerAddMethodName,
requiredArgumentName: identifier.Text,
out var instance) &&
ValuePatternMatches(instance))
{
seenIndexAssignment = true;
matches.Add(new Match<TStatementSyntax>(expressionStatement, UseSpread: false));
continue;
// `foreach` will become `..expr` when we make it into a collection expression.
matches.Add(new Match<TStatementSyntax>(foreachStatement, UseSpread: true));
return true;
}
}

return;
}
else if (extractedChild is TForeachStatementSyntax foreachStatement)

return false;
}

bool TryProcessIfStatement(TIfStatementSyntax ifStatement)
{
if (!_analyzeForCollectionExpression)
return false;

// look for the form:
//
// if (x)
// expr.Add(y)
//
// or
//
// if (x)
// expr.Add(y)
// else
// expr.Add(z)

_syntaxFacts.GetPartsOfIfStatement(ifStatement, out _, out var whenTrue, out var whenFalse);
var whenTrueStatements = whenTrue.ToImmutableArray();
var whenFalseStatements = whenFalse.ToImmutableArray();

if (whenTrueStatements is [TExpressionStatementSyntax trueChildStatement] &&
TryAnalyzeInvocation(
trueChildStatement,
addName: WellKnownMemberNames.CollectionInitializerAddMethodName,
requiredArgumentName: null,
out var instance) &&
ValuePatternMatches(instance))
{
// if we're not producing a collection expression, then we cannot convert any foreach'es into
// `[..expr]` elements.
if (!_analyzeForCollectionExpression)
return;
if (whenTrueStatements.Length == 0)
{
// add the form `.. x ? [y] : []` to the result
matches.Add(new Match<TStatementSyntax>(ifStatement, UseSpread: true));
return true;
}

_syntaxFacts.GetPartsOfForeachStatement(foreachStatement, out var identifier, out _, out var foreachStatements);
if (identifier == default)
return;

// must be of the form:
//
// foreach (var x in expr)
// dest.Add(x)
//
// By passing 'x' into TryAnalyzeInvocation below, we ensure that it is an enumerated value from `expr`
// being added to `dest`.
if (foreachStatements.ToImmutableArray() is not [TExpressionStatementSyntax childExpressionStatement] ||
!TryAnalyzeInvocation(
childExpressionStatement,
if (whenFalseStatements is [TExpressionStatementSyntax falseChildStatement] &&
TryAnalyzeInvocation(
falseChildStatement,
addName: WellKnownMemberNames.CollectionInitializerAddMethodName,
requiredArgumentName: identifier.Text,
out var instance) ||
!ValuePatternMatches(instance))
requiredArgumentName: null,
out instance) &&
ValuePatternMatches(instance))
{
return;
// add the form `x ? y : z` to the result
matches.Add(new Match<TStatementSyntax>(ifStatement, UseSpread: false));
return true;
}

// `foreach` will become `..expr` when we make it into a collection expression.
matches.Add(new Match<TStatementSyntax>(foreachStatement, UseSpread: true));
}
else
{
return;
}

return false;
}
}

Expand Down
4 changes: 3 additions & 1 deletion ...ore/CodeFixes/UseCollectionInitializer/AbstractUseCollectionInitializerCodeFixProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider<
TInvocationExpressionSyntax,
TExpressionStatementSyntax,
TForeachStatementSyntax,
TIfStatementSyntax,
TVariableDeclaratorSyntax>
: SyntaxEditorBasedCodeFixProvider
where TSyntaxKind : struct
Expand All @@ -40,6 +41,7 @@ internal abstract class AbstractUseCollectionInitializerCodeFixProvider<
where TInvocationExpressionSyntax : TExpressionSyntax
where TExpressionStatementSyntax : TStatementSyntax
where TForeachStatementSyntax : TStatementSyntax
where TIfStatementSyntax : TStatementSyntax
where TVariableDeclaratorSyntax : SyntaxNode
{
public sealed override ImmutableArray<string> FixableDiagnosticIds
Expand Down Expand Up @@ -102,7 +104,7 @@ protected sealed override async Task FixAllAsync(
var (originalObjectCreation, useCollectionExpression) = originalObjectCreationNodes.Pop();
var objectCreation = currentRoot.GetCurrentNodes(originalObjectCreation).Single();

var matches = UseCollectionInitializerAnalyzer<TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TVariableDeclaratorSyntax>.Analyze(
var matches = UseCollectionInitializerAnalyzer<TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TForeachStatementSyntax, TIfStatementSyntax, TVariableDeclaratorSyntax>.Analyze(
semanticModel, syntaxFacts, objectCreation, useCollectionExpression, 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.

no need for consumption sides of the helper type ot have to have so much noise. this was getting particularly awful as i needded to add 'IfStatementSyntax' here (and all the other places).


if (matches.IsDefaultOrEmpty)
Expand Down
2 changes: 2 additions & 0 deletions ...orkspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ internal interface ISyntaxFacts
bool IsExpressionOfForeach([NotNullWhen(true)] SyntaxNode? node);
void GetPartsOfForeachStatement(SyntaxNode statement, out SyntaxToken identifier, out SyntaxNode expression, out IEnumerable<SyntaxNode> statements);

void GetPartsOfIfStatement(SyntaxNode statement, out SyntaxNode condition, out IEnumerable<SyntaxNode> whenTrueStatements, out IEnumerable<SyntaxNode> whenFalseStatements);

void GetPartsOfTupleExpression<TArgumentSyntax>(SyntaxNode node,
out SyntaxToken openParen, out SeparatedSyntaxList<TArgumentSyntax> arguments, out SyntaxToken closeParen) where TArgumentSyntax : SyntaxNode;

Expand Down