-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Share more code in the 'use collection expression' fixers #69452
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
4560dfe
add base type
CyrusNajmabadi 2670641
Use helper
CyrusNajmabadi 4be6264
Use helper
CyrusNajmabadi 9f6ac57
Use helper
CyrusNajmabadi 2e6770a
Merge remote-tracking branch 'upstream/main' into forkedFixProvider
CyrusNajmabadi 594819d
Delete
CyrusNajmabadi 7f6e173
Docs
CyrusNajmabadi 4fcf048
Docs
CyrusNajmabadi e92c615
Docs and simpliciatin
CyrusNajmabadi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,130 +25,99 @@ namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; | |
using static UseCollectionExpressionHelpers; | ||
|
||
[ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.UseCollectionExpressionForArray), Shared] | ||
internal partial class CSharpUseCollectionExpressionForArrayCodeFixProvider : SyntaxEditorBasedCodeFixProvider | ||
internal partial class CSharpUseCollectionExpressionForArrayCodeFixProvider | ||
: ForkingSyntaxEditorBasedCodeFixProvider<ExpressionSyntax> | ||
{ | ||
[ImportingConstructor] | ||
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
public CSharpUseCollectionExpressionForArrayCodeFixProvider() | ||
: base(CSharpCodeFixesResources.Use_collection_expression, | ||
IDEDiagnosticIds.UseCollectionExpressionForArrayDiagnosticId) | ||
{ | ||
} | ||
|
||
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(IDEDiagnosticIds.UseCollectionExpressionForArrayDiagnosticId); | ||
|
||
protected override bool IncludeDiagnosticDuringFixAll(Diagnostic diagnostic) | ||
=> !diagnostic.Descriptor.ImmutableCustomTags().Contains(WellKnownDiagnosticTags.Unnecessary); | ||
|
||
public override Task RegisterCodeFixesAsync(CodeFixContext context) | ||
{ | ||
RegisterCodeFix(context, CSharpCodeFixesResources.Use_collection_expression, IDEDiagnosticIds.UseCollectionExpressionForArrayDiagnosticId); | ||
return Task.CompletedTask; | ||
} | ||
|
||
protected sealed override async Task FixAllAsync( | ||
protected sealed override async Task FixAsync( | ||
Document document, | ||
ImmutableArray<Diagnostic> diagnostics, | ||
SyntaxEditor editor, | ||
CodeActionOptionsProvider fallbackOptions, | ||
ExpressionSyntax arrayCreationExpression, | ||
ImmutableDictionary<string, string?> properties, | ||
CancellationToken cancellationToken) | ||
{ | ||
var services = document.Project.Solution.Services; | ||
var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>(); | ||
var originalRoot = editor.OriginalRoot; | ||
|
||
var arrayExpressions = new Stack<ExpressionSyntax>(); | ||
foreach (var diagnostic in diagnostics) | ||
if (arrayCreationExpression | ||
is not ArrayCreationExpressionSyntax | ||
and not ImplicitArrayCreationExpressionSyntax | ||
and not InitializerExpressionSyntax) | ||
{ | ||
var expression = (ExpressionSyntax)originalRoot.FindNode( | ||
diagnostic.AdditionalLocations[0].SourceSpan, getInnermostNodeForTie: true); | ||
arrayExpressions.Push(expression); | ||
return; | ||
} | ||
|
||
// We're going to be continually editing this tree. Track all the nodes we | ||
// care about so we can find them across each edit. | ||
var semanticDocument = await SemanticDocument.CreateAsync( | ||
document.WithSyntaxRoot(originalRoot.TrackNodes(arrayExpressions)), | ||
cancellationToken).ConfigureAwait(false); | ||
|
||
while (arrayExpressions.Count > 0) | ||
if (arrayCreationExpression is InitializerExpressionSyntax initializer) | ||
{ | ||
var arrayCreationExpression = semanticDocument.Root.GetCurrentNodes(arrayExpressions.Pop()).Single(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all this logic moved into the base class. |
||
if (arrayCreationExpression | ||
is not ArrayCreationExpressionSyntax | ||
and not ImplicitArrayCreationExpressionSyntax | ||
and not InitializerExpressionSyntax) | ||
{ | ||
continue; | ||
} | ||
|
||
var subEditor = new SyntaxEditor(semanticDocument.Root, services); | ||
|
||
if (arrayCreationExpression is InitializerExpressionSyntax initializer) | ||
{ | ||
subEditor.ReplaceNode( | ||
var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); | ||
editor.ReplaceNode( | ||
initializer, | ||
ConvertInitializerToCollectionExpression( | ||
initializer, | ||
ConvertInitializerToCollectionExpression( | ||
initializer, | ||
IsOnSingleLine(semanticDocument.Text, initializer))); | ||
} | ||
else | ||
{ | ||
var matches = GetMatches(semanticDocument, arrayCreationExpression); | ||
if (matches.IsDefault) | ||
continue; | ||
|
||
var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( | ||
semanticDocument.Document, | ||
fallbackOptions, | ||
arrayCreationExpression, | ||
matches, | ||
static e => e switch | ||
{ | ||
ArrayCreationExpressionSyntax arrayCreation => arrayCreation.Initializer, | ||
ImplicitArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.Initializer, | ||
_ => throw ExceptionUtilities.Unreachable(), | ||
}, | ||
static (e, i) => e switch | ||
{ | ||
ArrayCreationExpressionSyntax arrayCreation => arrayCreation.WithInitializer(i), | ||
ImplicitArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.WithInitializer(i), | ||
_ => throw ExceptionUtilities.Unreachable(), | ||
}, | ||
cancellationToken).ConfigureAwait(false); | ||
|
||
subEditor.ReplaceNode(arrayCreationExpression, collectionExpression); | ||
foreach (var match in matches) | ||
subEditor.RemoveNode(match.Statement); | ||
} | ||
|
||
semanticDocument = await semanticDocument.WithSyntaxRootAsync( | ||
subEditor.GetChangedRoot(), cancellationToken).ConfigureAwait(false); | ||
IsOnSingleLine(text, initializer))); | ||
} | ||
else | ||
{ | ||
var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); | ||
var matches = GetMatches(semanticModel, arrayCreationExpression); | ||
if (matches.IsDefault) | ||
return; | ||
|
||
var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( | ||
document, | ||
fallbackOptions, | ||
arrayCreationExpression, | ||
matches, | ||
static e => e switch | ||
{ | ||
ArrayCreationExpressionSyntax arrayCreation => arrayCreation.Initializer, | ||
ImplicitArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.Initializer, | ||
_ => throw ExceptionUtilities.Unreachable(), | ||
}, | ||
static (e, i) => e switch | ||
{ | ||
ArrayCreationExpressionSyntax arrayCreation => arrayCreation.WithInitializer(i), | ||
ImplicitArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.WithInitializer(i), | ||
_ => throw ExceptionUtilities.Unreachable(), | ||
}, | ||
cancellationToken).ConfigureAwait(false); | ||
|
||
editor.ReplaceNode(arrayCreationExpression, collectionExpression); | ||
foreach (var match in matches) | ||
editor.RemoveNode(match.Statement); | ||
} | ||
|
||
editor.ReplaceNode(originalRoot, semanticDocument.Root); | ||
return; | ||
|
||
static bool IsOnSingleLine(SourceText sourceText, SyntaxNode node) | ||
=> sourceText.AreOnSameLine(node.GetFirstToken(), node.GetLastToken()); | ||
|
||
ImmutableArray<CollectionExpressionMatch> GetMatches(SemanticDocument document, ExpressionSyntax expression) | ||
{ | ||
switch (expression) | ||
ImmutableArray<CollectionExpressionMatch> GetMatches(SemanticModel semanticModel, ExpressionSyntax expression) | ||
=> expression switch | ||
{ | ||
case ImplicitArrayCreationExpressionSyntax: | ||
// if we have `new[] { ... }` we have no subsequent matches to add to the collection. All values come | ||
// from within the initializer. | ||
return ImmutableArray<CollectionExpressionMatch>.Empty; | ||
|
||
case ArrayCreationExpressionSyntax arrayCreation: | ||
// we have `stackalloc T[...] ...;` defer to analyzer to find the items that follow that may need to | ||
// be added to the collection expression. | ||
return CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.TryGetMatches( | ||
document.SemanticModel, arrayCreation, cancellationToken); | ||
|
||
default: | ||
// We validated this is unreachable in the caller. | ||
throw ExceptionUtilities.Unreachable(); | ||
} | ||
} | ||
// if we have `new[] { ... }` we have no subsequent matches to add to the collection. All values come | ||
// from within the initializer. | ||
ImplicitArrayCreationExpressionSyntax | ||
=> ImmutableArray<CollectionExpressionMatch>.Empty, | ||
|
||
// we have `stackalloc T[...] ...;` defer to analyzer to find the items that follow that may need to | ||
// be added to the collection expression. | ||
ArrayCreationExpressionSyntax arrayCreation | ||
=> CSharpUseCollectionExpressionForArrayDiagnosticAnalyzer.TryGetMatches(semanticModel, arrayCreation, cancellationToken), | ||
|
||
// We validated this is unreachable in the caller. | ||
_ => throw ExceptionUtilities.Unreachable(), | ||
}; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,8 @@ | |
// See the LICENSE file in the project root for more information. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.Composition; | ||
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.CodeActions; | ||
|
@@ -15,117 +13,81 @@ | |
using Microsoft.CodeAnalysis.Diagnostics; | ||
using Microsoft.CodeAnalysis.Editing; | ||
using Microsoft.CodeAnalysis.Host.Mef; | ||
using Microsoft.CodeAnalysis.LanguageService; | ||
using Microsoft.CodeAnalysis.Shared.Extensions; | ||
using Roslyn.Utilities; | ||
|
||
namespace Microsoft.CodeAnalysis.CSharp.UseCollectionExpression; | ||
[ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.UseCollectionExpressionForStackAlloc), Shared] | ||
internal partial class CSharpUseCollectionExpressionForStackAllocCodeFixProvider : SyntaxEditorBasedCodeFixProvider | ||
internal partial class CSharpUseCollectionExpressionForStackAllocCodeFixProvider | ||
: ForkingSyntaxEditorBasedCodeFixProvider<ExpressionSyntax> | ||
{ | ||
[ImportingConstructor] | ||
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
public CSharpUseCollectionExpressionForStackAllocCodeFixProvider() | ||
: base(CSharpCodeFixesResources.Use_collection_expression, | ||
IDEDiagnosticIds.UseCollectionExpressionForStackAllocDiagnosticId) | ||
{ | ||
} | ||
|
||
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(IDEDiagnosticIds.UseCollectionExpressionForStackAllocDiagnosticId); | ||
|
||
protected override bool IncludeDiagnosticDuringFixAll(Diagnostic diagnostic) | ||
=> !diagnostic.Descriptor.ImmutableCustomTags().Contains(WellKnownDiagnosticTags.Unnecessary); | ||
|
||
public override Task RegisterCodeFixesAsync(CodeFixContext context) | ||
{ | ||
RegisterCodeFix(context, CSharpCodeFixesResources.Use_collection_expression, IDEDiagnosticIds.UseCollectionExpressionForStackAllocDiagnosticId); | ||
return Task.CompletedTask; | ||
} | ||
|
||
protected sealed override async Task FixAllAsync( | ||
protected sealed override async Task FixAsync( | ||
Document document, | ||
ImmutableArray<Diagnostic> diagnostics, | ||
SyntaxEditor editor, | ||
CodeActionOptionsProvider fallbackOptions, | ||
ExpressionSyntax stackAllocExpression, | ||
ImmutableDictionary<string, string?> properties, | ||
CancellationToken cancellationToken) | ||
{ | ||
var services = document.Project.Solution.Services; | ||
var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>(); | ||
var originalRoot = editor.OriginalRoot; | ||
|
||
var stackallocExpressions = new Stack<ExpressionSyntax>(); | ||
foreach (var diagnostic in diagnostics) | ||
{ | ||
var expression = (ExpressionSyntax)originalRoot.FindNode( | ||
diagnostic.AdditionalLocations[0].SourceSpan, getInnermostNodeForTie: true); | ||
stackallocExpressions.Push(expression); | ||
} | ||
|
||
// We're going to be continually editing this tree. Track all the nodes we | ||
// care about so we can find them across each edit. | ||
var semanticDocument = await SemanticDocument.CreateAsync( | ||
document.WithSyntaxRoot(originalRoot.TrackNodes(stackallocExpressions)), | ||
if (stackAllocExpression is not StackAllocArrayCreationExpressionSyntax and not ImplicitStackAllocArrayCreationExpressionSyntax) | ||
return; | ||
|
||
var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); | ||
var matches = GetMatches(); | ||
if (matches.IsDefault) | ||
return; | ||
|
||
var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( | ||
document, | ||
fallbackOptions, | ||
stackAllocExpression, | ||
matches, | ||
static e => e switch | ||
{ | ||
StackAllocArrayCreationExpressionSyntax arrayCreation => arrayCreation.Initializer, | ||
ImplicitStackAllocArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.Initializer, | ||
_ => throw ExceptionUtilities.Unreachable(), | ||
}, | ||
static (e, i) => e switch | ||
{ | ||
StackAllocArrayCreationExpressionSyntax arrayCreation => arrayCreation.WithInitializer(i), | ||
ImplicitStackAllocArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.WithInitializer(i), | ||
_ => throw ExceptionUtilities.Unreachable(), | ||
}, | ||
cancellationToken).ConfigureAwait(false); | ||
|
||
while (stackallocExpressions.Count > 0) | ||
{ | ||
var stackAllocExpression = semanticDocument.Root.GetCurrentNodes(stackallocExpressions.Pop()).Single(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all moved into base class. |
||
if (stackAllocExpression is not StackAllocArrayCreationExpressionSyntax and not ImplicitStackAllocArrayCreationExpressionSyntax) | ||
continue; | ||
|
||
var matches = GetMatches(semanticDocument, stackAllocExpression); | ||
if (matches.IsDefault) | ||
continue; | ||
|
||
var collectionExpression = await CSharpCollectionExpressionRewriter.CreateCollectionExpressionAsync( | ||
semanticDocument.Document, | ||
fallbackOptions, | ||
stackAllocExpression, | ||
matches, | ||
static e => e switch | ||
{ | ||
StackAllocArrayCreationExpressionSyntax arrayCreation => arrayCreation.Initializer, | ||
ImplicitStackAllocArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.Initializer, | ||
_ => throw ExceptionUtilities.Unreachable(), | ||
}, | ||
static (e, i) => e switch | ||
{ | ||
StackAllocArrayCreationExpressionSyntax arrayCreation => arrayCreation.WithInitializer(i), | ||
ImplicitStackAllocArrayCreationExpressionSyntax implicitArrayCreation => implicitArrayCreation.WithInitializer(i), | ||
_ => throw ExceptionUtilities.Unreachable(), | ||
}, | ||
cancellationToken).ConfigureAwait(false); | ||
|
||
var subEditor = new SyntaxEditor(semanticDocument.Root, services); | ||
subEditor.ReplaceNode(stackAllocExpression, collectionExpression); | ||
editor.ReplaceNode(stackAllocExpression, collectionExpression); | ||
|
||
foreach (var match in matches) | ||
subEditor.RemoveNode(match.Statement); | ||
foreach (var match in matches) | ||
editor.RemoveNode(match.Statement); | ||
|
||
semanticDocument = await semanticDocument.WithSyntaxRootAsync( | ||
subEditor.GetChangedRoot(), cancellationToken).ConfigureAwait(false); | ||
} | ||
|
||
editor.ReplaceNode(originalRoot, semanticDocument.Root); | ||
return; | ||
|
||
ImmutableArray<CollectionExpressionMatch> GetMatches(SemanticDocument document, ExpressionSyntax stackAllocExpression) | ||
{ | ||
switch (stackAllocExpression) | ||
ImmutableArray<CollectionExpressionMatch> GetMatches() | ||
=> stackAllocExpression switch | ||
{ | ||
case ImplicitStackAllocArrayCreationExpressionSyntax: | ||
// if we have `stackalloc[] { ... }` we have no subsequent matches to add to the collection. All values come | ||
// from within the initializer. | ||
return ImmutableArray<CollectionExpressionMatch>.Empty; | ||
|
||
case StackAllocArrayCreationExpressionSyntax arrayCreation: | ||
// we have `stackalloc T[...] ...;` defer to analyzer to find the items that follow that may need to | ||
// be added to the collection expression. | ||
return CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.TryGetMatches( | ||
document.SemanticModel, arrayCreation, cancellationToken); | ||
|
||
default: | ||
// We validated this is unreachable in the caller. | ||
throw ExceptionUtilities.Unreachable(); | ||
} | ||
} | ||
// if we have `stackalloc[] { ... }` we have no subsequent matches to add to the collection. All values come | ||
// from within the initializer. | ||
ImplicitStackAllocArrayCreationExpressionSyntax | ||
=> ImmutableArray<CollectionExpressionMatch>.Empty, | ||
|
||
// we have `stackalloc T[...] ...;` defer to analyzer to find the items that follow that may need to | ||
// be added to the collection expression. | ||
StackAllocArrayCreationExpressionSyntax arrayCreation | ||
=> CSharpUseCollectionExpressionForStackAllocDiagnosticAnalyzer.TryGetMatches(semanticModel, arrayCreation, cancellationToken), | ||
|
||
// We validated this is unreachable in the caller. | ||
_ => throw ExceptionUtilities.Unreachable(), | ||
}; | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
view with whitespace off.