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

Don't offer 'use collection initializer' if the collection being initialized is referenced during initialization. #17825

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,57 @@ static void Main(string[] args)
var myStringList = myStringArray?.ToList() ?? new [||]List<string>();
myStringList.Add(""Done"");
}
}");
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseCollectionInitializer)]
[WorkItem(17823, "https://github.com/dotnet/roslyn/issues/17823")]
public async Task TestMissingWhenReferencedInInitializer()
{
await TestMissingInRegularAndScriptAsync(
@"
using System.Collections.Generic;

class C
{
static void M()
{
var items = new [||]List<object>();
items[0] = items[0];
}
}");
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseCollectionInitializer)]
[WorkItem(17823, "https://github.com/dotnet/roslyn/issues/17823")]
public async Task TestWhenReferencedInInitializer()
{
await TestInRegularAndScript1Async(
@"
using System.Collections.Generic;

class C
{
static void M()
{
var items = new [||]List<object>();
items[0] = 1;
items[1] = items[0];
}
}",
@"
using System.Collections.Generic;

class C
{
static void M()
{
var items = new [||]List<object>
{
[0] = 1
};
items[1] = items[0];
}
}");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
return SpecializedTasks.EmptyTask;
}

protected override Task FixAllAsync(
protected override async Task FixAllAsync(
Document document, ImmutableArray<Diagnostic> diagnostics,
SyntaxEditor editor, CancellationToken cancellationToken)
{
Expand Down Expand Up @@ -78,13 +78,17 @@ protected override Task FixAllAsync(
// care about so we can find them across each edit.
var currentRoot = originalRoot.TrackNodes(originalObjectCreationNodes);

var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
(semanticModel, currentRoot) = GetCurrentSemanticModelAndRoot(
semanticModel, currentRoot, cancellationToken);

while (originalObjectCreationNodes.Count > 0)
{
var originalObjectCreation = originalObjectCreationNodes.Pop();
var objectCreation = currentRoot.GetCurrentNodes(originalObjectCreation).Single();

var analyzer = new ObjectCreationExpressionAnalyzer<TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TVariableDeclaratorSyntax>(
syntaxFacts, objectCreation);
semanticModel, syntaxFacts, objectCreation, cancellationToken);
var matches = analyzer.Analyze();
if (matches == null || matches.Value.Length == 0)
{
Expand All @@ -103,11 +107,27 @@ protected override Task FixAllAsync(
subEditor.RemoveNode(match);
}

currentRoot = subEditor.GetChangedRoot();
var newRoot = subEditor.GetChangedRoot();
(semanticModel, currentRoot) = GetCurrentSemanticModelAndRoot(
semanticModel, newRoot, cancellationToken);
}

editor.ReplaceNode(originalRoot, currentRoot);
return SpecializedTasks.EmptyTask;
}

private static (SemanticModel, SyntaxNode) GetCurrentSemanticModelAndRoot(
SemanticModel semanticModel, SyntaxNode newRoot, CancellationToken cancellationToken)
{
var oldSyntaxTree = semanticModel.SyntaxTree;
var newSyntaxTree = semanticModel.SyntaxTree.WithRootAndOptions(
newRoot, oldSyntaxTree.Options);

var newCompilation = semanticModel.Compilation.ReplaceSyntaxTree(oldSyntaxTree, newSyntaxTree);

var currentSemanticModel = newCompilation.GetSemanticModel(newSyntaxTree);
var currentRoot = currentSemanticModel.SyntaxTree.GetRoot(cancellationToken);

return (currentSemanticModel, currentRoot);
}

protected abstract TStatementSyntax GetNewStatement(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,12 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien
return;
}

var semanticModel = context.SemanticModel;
var objectCreationExpression = (TObjectCreationExpressionSyntax)context.Node;
var language = objectCreationExpression.Language;
var syntaxTree = objectCreationExpression.SyntaxTree;
var cancellationToken = context.CancellationToken;

var optionSet = context.Options.GetDocumentOptionSetAsync(syntaxTree, cancellationToken).GetAwaiter().GetResult();
if (optionSet == null)
{
Expand All @@ -89,7 +91,7 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context, INamedTypeSymbol ien
}

var analyzer = new ObjectCreationExpressionAnalyzer<TExpressionSyntax, TStatementSyntax, TObjectCreationExpressionSyntax, TMemberAccessExpressionSyntax, TInvocationExpressionSyntax, TExpressionStatementSyntax, TVariableDeclaratorSyntax>(
GetSyntaxFactsService(), objectCreationExpression);
semanticModel, GetSyntaxFactsService(), objectCreationExpression, cancellationToken);
var matches = analyzer.Analyze();
if (matches == null || matches.Value.Length == 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

using System.Collections;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.Shared.Extensions;

namespace Microsoft.CodeAnalysis.UseCollectionInitializer
{
Expand All @@ -22,18 +25,25 @@ internal struct ObjectCreationExpressionAnalyzer<
where TExpressionStatementSyntax : TStatementSyntax
where TVariableDeclaratorSyntax : SyntaxNode
{
private readonly SemanticModel _semanticModel;
private readonly ISyntaxFactsService _syntaxFacts;
private readonly TObjectCreationExpressionSyntax _objectCreationExpression;
private readonly CancellationToken _cancellationToken;

private TStatementSyntax _containingStatement;
private SyntaxNodeOrToken _valuePattern;
private ISymbol _variableSymbol;

public ObjectCreationExpressionAnalyzer(
SemanticModel semanticModel,
ISyntaxFactsService syntaxFacts,
TObjectCreationExpressionSyntax objectCreationExpression) : this()
TObjectCreationExpressionSyntax objectCreationExpression,
CancellationToken cancellationToken) : this()
{
_semanticModel = semanticModel;
_syntaxFacts = syntaxFacts;
_objectCreationExpression = objectCreationExpression;
_cancellationToken = cancellationToken;
}

internal ImmutableArray<TExpressionStatementSyntax>? Analyze()
Expand Down Expand Up @@ -146,6 +156,21 @@ private bool TryAnalyzeIndexAssignment(
return false;
}

// If we're initializing a variable, then we can't reference that variable on the right
// side of the initialization. Rewriting this into a collection initializer would lead
// to a definite-assignment error.
if (_variableSymbol != null)
{
foreach (var child in right.DescendantNodesAndSelf().OfType<TExpressionSyntax>())
{
if (ValuePatternMatches(child) &&
_variableSymbol.Equals(_semanticModel.GetSymbolInfo(child, _cancellationToken).GetAnySymbol()))
{
return false;
}
}
}

instance = _syntaxFacts.GetExpressionOfElementAccessExpression(left);
return true;
}
Expand Down Expand Up @@ -251,6 +276,7 @@ private bool TryInitializeVariableDeclarationCase()
}

_valuePattern = _syntaxFacts.GetIdentifierOfVariableDeclarator(containingDeclarator);
_variableSymbol = _semanticModel.GetDeclaredSymbol(containingDeclarator);
return true;
}
}
Expand Down