Skip to content

Commit

Permalink
CC0122 Replace Task.Result with await Task
Browse files Browse the repository at this point in the history
  • Loading branch information
AmadeusW committed Aug 12, 2016
1 parent 9080d64 commit 9fea23d
Show file tree
Hide file tree
Showing 9 changed files with 372 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/CSharp/CodeCracker/CodeCracker.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
<Compile Include="Design\InconsistentAccessibility\InconsistentAccessibilityInPropertyType.cs" />
<Compile Include="Design\MakeMethodStaticAnalyzer.cs" />
<Compile Include="Design\MakeMethodStaticCodeFixProvider.cs" />
<Compile Include="Design\ResultInAsyncAnalyzer.cs" />
<Compile Include="Design\ResultInAsyncCodeFixProvider.cs" />
<Compile Include="Extensions\CSharpAnalyzerExtensions.cs" />
<Compile Include="Extensions\CSharpGeneratedCodeAnalysisExtensions.cs" />
<Compile Include="Extensions\InitializerState.cs" />
Expand Down
66 changes: 66 additions & 0 deletions src/CSharp/CodeCracker/Design/ResultInAsyncAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
using CodeCracker.Properties;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using System;
using System.Collections.Immutable;
using System.Linq;

namespace CodeCracker.CSharp.Design
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class ResultInAsyncAnalyzer : DiagnosticAnalyzer
{
internal static readonly LocalizableString Title = new LocalizableResourceString(nameof(Resources.ResultInAsyncAnalyzer_Title), Resources.ResourceManager, typeof(Resources));
internal static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(Resources.ResultInAsync_MessageFormat), Resources.ResourceManager, typeof(Resources));
internal static readonly LocalizableString Description = new LocalizableResourceString(nameof(Resources.ResultInAsync_Description), Resources.ResourceManager, typeof(Resources));
internal const string Category = SupportedCategories.Design;

internal static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
DiagnosticId.ResultInAsync.ToDiagnosticId(),
Title,
MessageFormat,
Category,
DiagnosticSeverity.Warning,
true,
description: Description,
helpLinkUri: HelpLink.ForDiagnostic(DiagnosticId.ResultInAsync));

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

public override void Initialize(AnalysisContext context) =>
context.RegisterSyntaxNodeAction(Analyzer, SyntaxKind.InvocationExpression);

private static void Analyzer(SyntaxNodeAnalysisContext context)
{
if (context.IsGenerated()) return;
var invocation = (InvocationExpressionSyntax)context.Node;
var parentMethod = invocation.Ancestors().OfType<MethodDeclarationSyntax>().FirstOrDefault();
if (parentMethod == null) return;
var parentIsAsync = parentMethod.Modifiers.Any(n => n.IsKind(SyntaxKind.AsyncKeyword));
if (!parentIsAsync) return;
// We now know that we are in async method

var memberAccess = invocation.Parent as MemberAccessExpressionSyntax;
if (memberAccess == null) return;
var member = memberAccess.Name;
if (member.ToString() != "Result") return;
// We now know that we are accessing .Result

var identifierSymbol = context.SemanticModel.GetSymbolInfo(memberAccess, context.CancellationToken).Symbol;
if (identifierSymbol.OriginalDefinition.ToString() != "System.Threading.Tasks.Task<TResult>.Result") return;
// We now know that we are accessing System.Threading.Tasks.Task<TResult>.Result

SimpleNameSyntax identifier;
identifier = invocation.Expression as IdentifierNameSyntax;
if (identifier == null)
{
var transient = invocation.Expression as MemberAccessExpressionSyntax;
identifier = transient.Name;
}
if (identifier == null) return; // It's not supposed to happen. Don't throw an exception, though.
context.ReportDiagnostic(Diagnostic.Create(Rule, identifier.GetLocation(), identifier.Identifier.Text));
}
}
}
75 changes: 75 additions & 0 deletions src/CSharp/CodeCracker/Design/ResultInAsyncCodeFixProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
using CodeCracker.Properties;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Simplification;
using System;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace CodeCracker.CSharp.Design
{
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ResultInAsyncCodeFixProvider)), Shared]
public class ResultInAsyncCodeFixProvider : CodeFixProvider
{
public sealed override ImmutableArray<string> FixableDiagnosticIds =>
ImmutableArray.Create(DiagnosticId.ResultInAsync.ToDiagnosticId());

public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

public async sealed override Task RegisterCodeFixesAsync(CodeFixContext context)
{
var diagnostic = context.Diagnostics.First();
var compilation = (CSharpCompilation)await context.Document.Project.GetCompilationAsync();
context.RegisterCodeFix(CodeAction.Create(
Resources.ResultInAsyncCodeFixProvider_Title,
ct => ReplaceResultWithAwaitAsync(context.Document, diagnostic, ct),
nameof(ResultInAsyncCodeFixProvider)
), diagnostic);
}

private async static Task<Document> ReplaceResultWithAwaitAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
{
var root = (await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false));
var sourceSpan = diagnostic.Location.SourceSpan;
var invocation = root.FindToken(sourceSpan.Start).Parent.AncestorsAndSelf().OfType<InvocationExpressionSyntax>().First();
var memberAccess = invocation.Parent as MemberAccessExpressionSyntax;

// Replace memberAccess with the async invocation
SyntaxNode newRoot;

// See if the member access expression is a part of something bigger
// i.e. something.Result.something. Then we need to produce (await something.Result).something
var parentAccess = memberAccess.Parent as MemberAccessExpressionSyntax;
if (parentAccess != null)
{
var rewritten =
SyntaxFactory.ParenthesizedExpression(
SyntaxFactory.AwaitExpression(
invocation)
)
.WithLeadingTrivia(invocation.GetLeadingTrivia())
.WithTrailingTrivia(invocation.GetTrailingTrivia());
var subExpression = parentAccess.Expression;
newRoot = root.ReplaceNode(subExpression, rewritten);
}
else
{
var rewritten =
SyntaxFactory.AwaitExpression(
invocation)
.WithLeadingTrivia(invocation.GetLeadingTrivia())
.WithTrailingTrivia(invocation.GetTrailingTrivia());
newRoot = root.ReplaceNode(memberAccess, rewritten);
}

return document.WithSyntaxRoot(newRoot);
}
}
}
1 change: 1 addition & 0 deletions src/Common/CodeCracker.Common/DiagnosticId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,6 @@ public enum DiagnosticId
NameOf_External = 108,
StringFormatArgs_ExtraArgs = 111,
AlwaysUseVarOnPrimitives = 105,
ResultInAsync = 122,
}
}
36 changes: 36 additions & 0 deletions src/Common/CodeCracker.Common/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions src/Common/CodeCracker.Common/Properties/Resources.fr.resx
Original file line number Diff line number Diff line change
Expand Up @@ -214,4 +214,16 @@ Si l'erreur est attendu considérer ajouter du logging ou modifier le flow de co
<data name="EmptyCatchBlockCodeFixProvider_RemoveAndDocumentation" xml:space="preserve">
<value>Enlever le block Catch vide et ajouter un lien vers la documentation des bonnes pratiques de Try...Catch</value>
</data>
<data name="ResultInAsyncAnalyzer_Title" xml:space="preserve">
<value>Replace Task.Result with await Task</value>
</data>
<data name="ResultInAsyncCodeFixProvider_Title" xml:space="preserve">
<value>Await the asynchronous call</value>
</data>
<data name="ResultInAsync_Description" xml:space="preserve">
<value>Calling Task.Result in an awaited method may lead to a deadlock. Obtain the result of the task with the await keyword to avoid deadlocks.</value>
</data>
<data name="ResultInAsync_MessageFormat" xml:space="preserve">
<value>await '{0}' rather than calling its Result.</value>
</data>
</root>
12 changes: 12 additions & 0 deletions src/Common/CodeCracker.Common/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -216,4 +216,16 @@
<data name="EmptyCatchBlockCodeFixProvider_RemoveTry" xml:space="preserve">
<value>Remove wrapping Try Block</value>
</data>
<data name="ResultInAsyncAnalyzer_Title" xml:space="preserve">
<value>Replace Task.Result with await Task</value>
</data>
<data name="ResultInAsyncCodeFixProvider_Title" xml:space="preserve">
<value>Await the asynchronous call</value>
</data>
<data name="ResultInAsync_Description" xml:space="preserve">
<value>Calling Task.Result in an awaited method may lead to a deadlock. Obtain the result of the task with the await keyword to avoid deadlocks.</value>
</data>
<data name="ResultInAsync_MessageFormat" xml:space="preserve">
<value>await '{0}' rather than calling its Result.</value>
</data>
</root>
1 change: 1 addition & 0 deletions test/CSharp/CodeCracker.Test/CodeCracker.Test.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@
<Compile Include="Design\InconsistentAccessibilityTests.MethodIndexerParameter.cs" />
<Compile Include="Design\InconsistentAccessibilityTests.MethodIndexerReturnType.cs" />
<Compile Include="Design\MakeMethodStaticTests.cs" />
<Compile Include="Design\ResultInAsyncTest.cs" />
<Compile Include="GeneratedCodeAnalysisExtensionsTests.cs" />
<Compile Include="GlobalSuppressions.cs" />
<Compile Include="Performance\UseStaticRegexIsMatchTests.cs" />
Expand Down

0 comments on commit 9fea23d

Please sign in to comment.