Skip to content

Commit

Permalink
Merge pull request #4841 from ryzngard/feature/task_when_wait_all_ana…
Browse files Browse the repository at this point in the history
…lyzer

Add WhenAll and WaitAll analyzer for single task argument
  • Loading branch information
ryzngard authored Apr 22, 2021
2 parents 138ab76 + dd00427 commit 535cbe7
Show file tree
Hide file tree
Showing 24 changed files with 1,179 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Composition;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.NetCore.Analyzers.Tasks;

namespace Microsoft.NetCore.CSharp.Analyzers.Tasks
{
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
public sealed class CSharpDoNotUseWhenAllOrWaitAllWithSingleArgumentFixer : DoNotUseWhenAllOrWaitAllWithSingleArgumentFixer
{
protected override SyntaxNode GetSingleArgumentSyntax(IInvocationOperation operation)
{
var invocationSyntax = (InvocationExpressionSyntax)operation.Syntax;
return invocationSyntax.ArgumentList.Arguments.Single().Expression;
}
}
}
2 changes: 2 additions & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ CA1418 | Interoperability | Warning | UseValidPlatformString, [Documentation](ht
CA1839 | Performance | Info | UseEnvironmentMembers, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1839)
CA1840 | Performance | Info | UseEnvironmentMembers, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1840)
CA1841 | Performance | Info | PreferDictionaryContainsMethods, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1841)
CA1842 | Performance | Info | DoNotUseWhenAllOrWaitAllWithSingleArgument, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1842)
CA1843 | Performance | Info | DoNotUseWhenAllOrWaitAllWithSingleArgument, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1843)

### Removed Rules

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1571,4 +1571,22 @@
<data name="UseValidPlatformStringNoVersion" xml:space="preserve">
<value>Version '{0}' is not valid for platform '{1}'. Do not use versions for this platform.</value>
</data>
</root>
<data name="DoNotUseWaitAllWithSingleTaskDescription" xml:space="preserve">
<value>Using 'WaitAll' with a single task may result in performance loss, await or return the task instead.</value>
</data>
<data name="DoNotUseWaitAllWithSingleTaskTitle" xml:space="preserve">
<value>Do not use 'WaitAll' with a single task</value>
</data>
<data name="DoNotUseWhenAllWithSingleTaskDescription" xml:space="preserve">
<value>Using 'WhenAll' with a single task may result in performance loss, await or return the task instead.</value>
</data>
<data name="DoNotUseWhenAllWithSingleTaskTitle" xml:space="preserve">
<value>Do not use 'WhenAll' with a single task</value>
</data>
<data name="DoNotUseWaitAllWithSingleTaskFix" xml:space="preserve">
<value>Replace 'WaitAll' with single 'Wait'</value>
</data>
<data name="DoNotUseWhenAllWithSingleTaskFix" xml:space="preserve">
<value>Replace 'WhenAll' call with argument</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Linq;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.NetCore.Analyzers.Tasks
{
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public class DoNotUseWhenAllOrWaitAllWithSingleArgument : DiagnosticAnalyzer
{
internal const string WhenAllRuleId = "CA1842";
internal const string WaitAllRuleId = "CA1843";

internal static readonly DiagnosticDescriptor WhenAllRule = DiagnosticDescriptorHelper.Create(WhenAllRuleId,
new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.DoNotUseWhenAllWithSingleTaskTitle), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources)),
new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.DoNotUseWhenAllWithSingleTaskTitle), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources)),
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.DoNotUseWhenAllWithSingleTaskDescription), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources)),
isPortedFxCopRule: false,
isDataflowRule: false);

internal static readonly DiagnosticDescriptor WaitAllRule = DiagnosticDescriptorHelper.Create(WaitAllRuleId,
new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.DoNotUseWaitAllWithSingleTaskTitle), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources)),
new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.DoNotUseWaitAllWithSingleTaskTitle), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources)),
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.DoNotUseWaitAllWithSingleTaskDescription), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources)),
isPortedFxCopRule: false,
isDataflowRule: false);

public sealed override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(WhenAllRule, WaitAllRule);

public sealed override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(compilationContext =>
{
if (!compilationContext.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksTask, out var taskType) ||
!compilationContext.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksTask1, out var genericTaskType))
{
return;
}

compilationContext.RegisterOperationAction(operationContext =>
{
var invocation = (IInvocationOperation)operationContext.Operation;
if (IsWhenOrWaitAllMethod(invocation.TargetMethod, taskType) &&
IsSingleTaskArgument(invocation, taskType, genericTaskType))
{
switch (invocation.TargetMethod.Name)
{
case nameof(Task.WhenAll):
operationContext.ReportDiagnostic(invocation.CreateDiagnostic(WhenAllRule));
break;

case nameof(Task.WaitAll):
operationContext.ReportDiagnostic(invocation.CreateDiagnostic(WaitAllRule));
break;

default:
throw new InvalidOperationException($"Unexpected method name: {invocation.TargetMethod.Name}");
}
}
}, OperationKind.Invocation);
});
}

private static bool IsWhenOrWaitAllMethod(IMethodSymbol targetMethod, INamedTypeSymbol taskType)
{
var nameMatches = targetMethod.Name is (nameof(Task.WhenAll)) or (nameof(Task.WaitAll));
var parameters = targetMethod.Parameters;

return nameMatches &&
targetMethod.IsStatic &&
SymbolEqualityComparer.Default.Equals(targetMethod.ContainingType, taskType) &&
parameters.Length == 1 &&
parameters[0].IsParams;
}

private static bool IsSingleTaskArgument(IInvocationOperation invocation, INamedTypeSymbol taskType, INamedTypeSymbol genericTaskType)
{
if (invocation.Arguments.Length != 1)
{
return false;
}

var argument = invocation.Arguments.Single();

// Task.WhenAll and Task.WaitAll have params arguments, which are implicit
// array creation for cases where params were passed in without explicitly
// being an array already.
if (argument.Value is not IArrayCreationOperation
{
IsImplicit: true,
Initializer: { ElementValues: { Length: 1 } initializerValues }
})
{
return false;
}

if (initializerValues.Single().Type is not INamedTypeSymbol namedTypeSymbol)
{
return false;
}

return namedTypeSymbol.Equals(taskType) || namedTypeSymbol.ConstructedFrom.Equals(genericTaskType);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.NetCore.Analyzers.Tasks
{
public abstract class DoNotUseWhenAllOrWaitAllWithSingleArgumentFixer : CodeFixProvider
{
public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(
DoNotUseWhenAllOrWaitAllWithSingleArgument.WaitAllRule.Id,
DoNotUseWhenAllOrWaitAllWithSingleArgument.WhenAllRule.Id);

public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var cancellationToken = context.CancellationToken;
var root = await context.Document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var nodeToFix = root.FindNode(context.Span, getInnermostNodeForTie: true);
if (nodeToFix is null)
{
return;
}

var semanticModel = await context.Document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
if (semanticModel.GetOperation(nodeToFix, cancellationToken) is not IInvocationOperation operation)
{
return;
}

if (operation.TargetMethod.Name == nameof(Task.WaitAll))
{
var title = MicrosoftNetCoreAnalyzersResources.DoNotUseWaitAllWithSingleTaskFix;
context.RegisterCodeFix(new MyCodeAction(title,
async ct =>
{
var editor = await DocumentEditor.CreateAsync(context.Document, ct).ConfigureAwait(false);
editor.ReplaceNode(nodeToFix,
editor.Generator.InvocationExpression(
editor.Generator.MemberAccessExpression(
GetSingleArgumentSyntax(operation),
nameof(Task.Wait))
).WithTriviaFrom(nodeToFix)
);

return editor.GetChangedDocument();
},
equivalenceKey: title),
context.Diagnostics);
}
else if (!IsValueStored(operation) && operation.TargetMethod.Name == nameof(Task.WhenAll))
{
var title = MicrosoftNetCoreAnalyzersResources.DoNotUseWhenAllWithSingleTaskFix;
context.RegisterCodeFix(new MyCodeAction(title,
async ct =>
{
var editor = await DocumentEditor.CreateAsync(context.Document, ct).ConfigureAwait(false);
var newNode = GetSingleArgumentSyntax(operation).WithTriviaFrom(nodeToFix);

// The original invocation already returns a Task,
// so we can just replace directly with the argument
editor.ReplaceNode(nodeToFix, newNode);

return editor.GetChangedDocument();
},
equivalenceKey: title),
context.Diagnostics);
}
}

/// <summary>
/// Returns true if the invocation is part of an assignment or variable declaration
/// </summary>
private static bool IsValueStored(IInvocationOperation operation)
{
var currentOperation = operation.Parent;
while (currentOperation is not null)
{
if (currentOperation is IAssignmentOperation or
IVariableDeclarationOperation)
{
return true;
}

currentOperation = currentOperation.Parent;
}

return false;
}

protected abstract SyntaxNode GetSingleArgumentSyntax(IInvocationOperation operation);
public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

// Needed for Telemetry (https://github.com/dotnet/roslyn-analyzers/issues/192)
private sealed class MyCodeAction : DocumentChangeAction
{
public MyCodeAction(string title, Func<CancellationToken, Task<Document>> createChangedDocument, string equivalenceKey) :
base(title, createChangedDocument, equivalenceKey)
{
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,21 @@
<target state="translated">Použití nebezpečné hodnoty DllImportSearchPath {0}</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWaitAllWithSingleTaskDescription">
<source>Using 'WaitAll' with a single task may result in performance loss, await or return the task instead.</source>
<target state="new">Using 'WaitAll' with a single task may result in performance loss, await or return the task instead.</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWaitAllWithSingleTaskFix">
<source>Replace 'WaitAll' with single 'Wait'</source>
<target state="new">Replace 'WaitAll' with single 'Wait'</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWaitAllWithSingleTaskTitle">
<source>Do not use 'WaitAll' with a single task</source>
<target state="new">Do not use 'WaitAll' with a single task</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWeakCryptographicAlgorithms">
<source>Do Not Use Weak Cryptographic Algorithms</source>
<target state="translated">Nepoužívejte slabé kryptografické algoritmy</target>
Expand Down Expand Up @@ -977,6 +992,21 @@
<target state="translated">Při odvozování kryptografických klíčů z uživatelem zadaných vstupů, jako je například heslo, používejte dostatečný počet iterací (alespoň 100 000).</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWhenAllWithSingleTaskDescription">
<source>Using 'WhenAll' with a single task may result in performance loss, await or return the task instead.</source>
<target state="new">Using 'WhenAll' with a single task may result in performance loss, await or return the task instead.</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWhenAllWithSingleTaskFix">
<source>Replace 'WhenAll' call with argument</source>
<target state="new">Replace 'WhenAll' call with argument</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWhenAllWithSingleTaskTitle">
<source>Do not use 'WhenAll' with a single task</source>
<target state="new">Do not use 'WhenAll' with a single task</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseXslTransform">
<source>Do Not Use XslTransform</source>
<target state="translated">Nepoužívat XslTransform</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,21 @@
<target state="translated">Verwendung eines unsicheren DllImportSearchPath-Werts "{0}"</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWaitAllWithSingleTaskDescription">
<source>Using 'WaitAll' with a single task may result in performance loss, await or return the task instead.</source>
<target state="new">Using 'WaitAll' with a single task may result in performance loss, await or return the task instead.</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWaitAllWithSingleTaskFix">
<source>Replace 'WaitAll' with single 'Wait'</source>
<target state="new">Replace 'WaitAll' with single 'Wait'</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWaitAllWithSingleTaskTitle">
<source>Do not use 'WaitAll' with a single task</source>
<target state="new">Do not use 'WaitAll' with a single task</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWeakCryptographicAlgorithms">
<source>Do Not Use Weak Cryptographic Algorithms</source>
<target state="translated">Keine schwachen kryptografischen Algorithmen verwenden</target>
Expand Down Expand Up @@ -977,6 +992,21 @@
<target state="translated">Verwenden Sie beim Ableiten kryptografischer Schlüssel aus Benutzereingaben (z. B. Kennwort) eine ausreichende Anzahl von Iterationen (mindestens 100.000).</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWhenAllWithSingleTaskDescription">
<source>Using 'WhenAll' with a single task may result in performance loss, await or return the task instead.</source>
<target state="new">Using 'WhenAll' with a single task may result in performance loss, await or return the task instead.</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWhenAllWithSingleTaskFix">
<source>Replace 'WhenAll' call with argument</source>
<target state="new">Replace 'WhenAll' call with argument</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseWhenAllWithSingleTaskTitle">
<source>Do not use 'WhenAll' with a single task</source>
<target state="new">Do not use 'WhenAll' with a single task</target>
<note />
</trans-unit>
<trans-unit id="DoNotUseXslTransform">
<source>Do Not Use XslTransform</source>
<target state="translated">Kein XslTransform verwenden</target>
Expand Down
Loading

0 comments on commit 535cbe7

Please sign in to comment.