提交 c2d4bac0 编写于 作者: M Manish Vasani

Address feedback

上级 3839fe55
......@@ -344,7 +344,7 @@ void M(int p)
{
if (p > 0)
{
var unused = M2();
var unused1 = M2();
}
else
{
......
......@@ -11,7 +11,6 @@
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;
namespace Microsoft.CodeAnalysis.AddRequiredParentheses
{
......@@ -21,7 +20,7 @@ internal class AddRequiredParenthesesCodeFixProvider : SyntaxEditorBasedCodeFixP
public override ImmutableArray<string> FixableDiagnosticIds
=> ImmutableArray.Create(IDEDiagnosticIds.AddRequiredParenthesesDiagnosticId);
protected override bool IncludeDiagnosticDuringFixAll(FixAllState state, Diagnostic diagnostic)
protected override bool IncludeDiagnosticDuringFixAll(FixAllState state, Diagnostic diagnostic, CancellationToken cancellationToken)
=> diagnostic.Properties.ContainsKey(AddRequiredParenthesesConstants.IncludeInFixAll) &&
diagnostic.Properties[AddRequiredParenthesesConstants.EquivalenceKey] == state.CodeActionEquivalenceKey;
......
......@@ -56,7 +56,7 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
}
}
protected override bool IncludeDiagnosticDuringFixAll(FixAllState state, Diagnostic diagnostic)
protected override bool IncludeDiagnosticDuringFixAll(FixAllState state, Diagnostic diagnostic, CancellationToken cancellationToken)
=> diagnostic.Properties.ContainsKey(PreferFrameworkTypeConstants.PreferFrameworkType);
private class PreferFrameworkTypeCodeAction : CodeAction.DocumentChangeAction
......
......@@ -301,8 +301,8 @@ private void AnalyzeOperationBlockEnd(OperationBlockAnalysisContext context)
}
// Now perform the slower, precise, CFG based dataflow analysis to identify the actual unused symbol writes.
var cfg = context.GetControlFlowGraph(operationBlock);
var symbolUsageResult = SymbolUsageAnalysis.Run(cfg, context.OwningSymbol, context.CancellationToken);
var controlFlowGraph = context.GetControlFlowGraph(operationBlock);
var symbolUsageResult = SymbolUsageAnalysis.Run(controlFlowGraph, context.OwningSymbol, context.CancellationToken);
symbolUsageResultsBuilder.Add(symbolUsageResult);
foreach (var (symbol, unreadWriteOperation) in symbolUsageResult.GetUnreadSymbolWrites())
......
......@@ -108,7 +108,7 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context)
switch (preference)
{
case UnusedValuePreference.DiscardVariable:
if (IsForEachIterationVariableDiagnostic(diagnostic, context.Document))
if (IsForEachIterationVariableDiagnostic(diagnostic, context.Document, context.CancellationToken))
{
// Do not offer a fix to replace unused foreach iteration variable with discard.
// User should probably replace it with a for loop based on the collection length.
......@@ -137,12 +137,12 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context)
return Task.CompletedTask;
}
private static bool IsForEachIterationVariableDiagnostic(Diagnostic diagnostic, Document document)
private static bool IsForEachIterationVariableDiagnostic(Diagnostic diagnostic, Document document, CancellationToken cancellationToken)
{
// Do not offer a fix to replace unused foreach iteration variable with discard.
// User should probably replace it with a for loop based on the collection length.
var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
return syntaxFacts.IsForEachStatement(diagnostic.Location.FindNode(CancellationToken.None));
return syntaxFacts.IsForEachStatement(diagnostic.Location.FindNode(cancellationToken));
}
private static string GetEquivalenceKey(UnusedValuePreference preference, bool isRemovableAssignment)
......@@ -167,10 +167,10 @@ private static string GetEquivalenceKey(Diagnostic diagnostic)
private static bool NeedsToMoveNewLocalDeclarationsNearReference(string diagnosticId)
=> diagnosticId == IDEDiagnosticIds.ValueAssignedIsUnusedDiagnosticId;
protected override bool IncludeDiagnosticDuringFixAll(FixAllState fixAllState, Diagnostic diagnostic)
protected override bool IncludeDiagnosticDuringFixAll(FixAllState fixAllState, Diagnostic diagnostic, CancellationToken cancellationToken)
{
return fixAllState.CodeActionEquivalenceKey == GetEquivalenceKey(diagnostic) &&
!IsForEachIterationVariableDiagnostic(diagnostic, fixAllState.Document);
!IsForEachIterationVariableDiagnostic(diagnostic, fixAllState.Document, cancellationToken);
}
private IEnumerable<IGrouping<SyntaxNode, Diagnostic>> GetDiagnosticsGroupedByMember(
......@@ -221,6 +221,7 @@ protected sealed override async Task FixAllAsync(Document document, ImmutableArr
document = await PreprocessDocumentAsync(document, diagnostics, cancellationToken).ConfigureAwait(false);
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
var semanticFacts = document.GetLanguageService<ISemanticFactsService>();
var originalEditor = editor;
editor = new SyntaxEditor(root, editor.Generator);
......@@ -248,7 +249,7 @@ protected sealed override async Task FixAllAsync(Document document, ImmutableArr
foreach (var diagnosticsToFix in diagnosticsGroupedByMember)
{
var orderedDiagnostics = diagnosticsToFix.OrderBy(d => d.Location.SourceSpan.Start);
using (var nameGenerator = new UniqueVariableNameGenerator(semanticModel, syntaxFacts))
using (var nameGenerator = new UniqueVariableNameGenerator(diagnosticsToFix.Key, semanticModel, semanticFacts, cancellationToken))
{
FixAll(diagnosticId, orderedDiagnostics, semanticModel, root, preference,
removeAssignments, nameGenerator, editor, syntaxFacts, cancellationToken);
......@@ -439,10 +440,10 @@ protected sealed override async Task FixAllAsync(Document document, ImmutableArr
// For example, "x = MethodCall();" is replaced with "_ = MethodCall();" or "var unused = MethodCall();"
// Replace the flagged variable's indentifier token with new named, based on user's preference.
newLocalNameOpt = preference == UnusedValuePreference.DiscardVariable
? AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.DiscardVariableName
var newNameToken = preference == UnusedValuePreference.DiscardVariable
? editor.Generator.Identifier(AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.DiscardVariableName)
: nameGenerator.GenerateUniqueNameAtSpanStart(node);
var newNameToken = editor.Generator.Identifier(newLocalNameOpt);
newLocalNameOpt = newNameToken.ValueText;
var newNameNode = TryUpdateNameForFlaggedNode(node, newNameToken);
if (newNameNode == null)
{
......@@ -761,50 +762,31 @@ private sealed class MyCodeAction : CodeAction.DocumentChangeAction
protected sealed class UniqueVariableNameGenerator: IDisposable
{
private readonly SyntaxNode _memberDeclaration;
private readonly SemanticModel _semanticModel;
private readonly ISyntaxFactsService _syntaxFacts;
private readonly ISemanticFactsService _semanticFacts;
private readonly CancellationToken _cancellationToken;
private readonly PooledHashSet<string> _usedNames;
public UniqueVariableNameGenerator(SemanticModel semanticModel, ISyntaxFactsService syntaxFacts)
public UniqueVariableNameGenerator(
SyntaxNode memberDeclaration,
SemanticModel semanticModel,
ISemanticFactsService semanticFacts,
CancellationToken cancellationToken)
{
_memberDeclaration = memberDeclaration;
_semanticModel = semanticModel;
_syntaxFacts = syntaxFacts;
_usedNames = PooledHashSet<string>.GetInstance();
}
_semanticFacts = semanticFacts;
_cancellationToken = cancellationToken;
public string GenerateUniqueNameAtSpanStart(SyntaxNode node)
{
var localsInNestedScope = PooledHashSet<string>.GetInstance();
try
{
// Add local names for all variable declarations in nested scopes for this node.
// This helps prevent name clashes with locals declared in nested block scopes.
AddLocalsInNestedScope(node, localsInNestedScope);
var name = NameGenerator.GenerateUniqueName("unused",
n => !_usedNames.Contains(n) &&
!localsInNestedScope.Contains(n) &&
_semanticModel.LookupSymbols(node.SpanStart, name: n).IsEmpty);
_usedNames.Add(name);
return name;
}
finally
{
localsInNestedScope.Free();
}
_usedNames = PooledHashSet<string>.GetInstance();
}
private void AddLocalsInNestedScope(SyntaxNode node, PooledHashSet<string> localsInNestedScope)
public SyntaxToken GenerateUniqueNameAtSpanStart(SyntaxNode node)
{
var blockAncestor = node.FirstAncestorOrSelf<SyntaxNode>(n => _syntaxFacts.IsExecutableBlock(n));
if (blockAncestor != null)
{
foreach (var variableDeclarator in blockAncestor.DescendantNodes().OfType<TVariableDeclaratorSyntax>())
{
var name = _syntaxFacts.GetIdentifierOfVariableDeclarator(variableDeclarator).ValueText;
localsInNestedScope.Add(name);
}
}
var nameToken = _semanticFacts.GenerateUniqueName(_semanticModel, node, _memberDeclaration, "unused", _usedNames, _cancellationToken);
_usedNames.Add(nameToken.ValueText);
return nameToken;
}
public void Dispose() => _usedNames.Free();
......
......@@ -64,7 +64,7 @@ public sealed override async Task<CodeAction> GetFixAsync(FixAllContext fixAllCo
// Ensure that diagnostics for this document are always in document location
// order. This provides a consistent and deterministic order for fixers
// that want to update a document.
var filteredDiagnostics = diagnostics.WhereAsArray(d => _codeFixProvider.IncludeDiagnosticDuringFixAll(fixAllState, d))
var filteredDiagnostics = diagnostics.WhereAsArray(d => _codeFixProvider.IncludeDiagnosticDuringFixAll(fixAllState, d, cancellationToken))
.Sort((d1, d2) => d1.Location.SourceSpan.Start - d2.Location.SourceSpan.Start);
return _codeFixProvider.FixAllAsync(document, filteredDiagnostics, cancellationToken);
}
......
......@@ -65,7 +65,7 @@ public sealed override FixAllProvider GetFixAllProvider()
/// Only one of these two overloads needs to be overridden if you want to customize
/// behavior.
/// </summary>
protected virtual bool IncludeDiagnosticDuringFixAll(FixAllState fixAllState, Diagnostic diagnostic)
protected virtual bool IncludeDiagnosticDuringFixAll(FixAllState fixAllState, Diagnostic diagnostic, CancellationToken cancellationToken)
=> IncludeDiagnosticDuringFixAll(diagnostic);
/// <summary>
......@@ -79,7 +79,7 @@ protected virtual bool IncludeDiagnosticDuringFixAll(FixAllState fixAllState, Di
/// here. If only the diagnostic needs to be queried to make this determination, only this
/// overload needs to be overridden. However, if information from <see cref="FixAllState"/>
/// is needed (for example <see cref="FixAllState.CodeActionEquivalenceKey"/>), then <see
/// cref="IncludeDiagnosticDuringFixAll(FixAllState, Diagnostic)"/>
/// cref="IncludeDiagnosticDuringFixAll(FixAllState, Diagnostic, CancellationToken)"/>
/// should be overridden instead.
///
/// Only one of these two overloads needs to be overridden if you want to customize
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册