Merge pull request #40 from Microsoft/dev/andarno/fix39
Fix recognition of and fixes to delegates within methods
This commit is contained in:
Коммит
96ef16abca
|
@ -62,6 +62,52 @@ class Test {
|
|||
this.VerifyCSharpFix(test, withFix);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void JTFRunInTaskReturningMethod_WithExtraReturn_GeneratesWarning()
|
||||
{
|
||||
var test = @"
|
||||
using System.Threading.Tasks;
|
||||
using Microsoft.VisualStudio.Threading;
|
||||
|
||||
class Test {
|
||||
Task T() {
|
||||
JoinableTaskFactory jtf = null;
|
||||
jtf.Run(() => TplExtensions.CompletedTask);
|
||||
if (false) {
|
||||
return Task.FromResult(2);
|
||||
}
|
||||
|
||||
this.Run();
|
||||
return Task.FromResult(1);
|
||||
}
|
||||
|
||||
void Run() { }
|
||||
}
|
||||
";
|
||||
|
||||
var withFix = @"
|
||||
using System.Threading.Tasks;
|
||||
using Microsoft.VisualStudio.Threading;
|
||||
|
||||
class Test {
|
||||
async Task T() {
|
||||
JoinableTaskFactory jtf = null;
|
||||
await jtf.RunAsync(() => TplExtensions.CompletedTask);
|
||||
if (false) {
|
||||
return;
|
||||
}
|
||||
|
||||
this.Run();
|
||||
}
|
||||
|
||||
void Run() { }
|
||||
}
|
||||
";
|
||||
this.expect.Locations = new[] { new DiagnosticResultLocation("Test0.cs", 8, 13) };
|
||||
this.VerifyCSharpDiagnostic(test, this.expect);
|
||||
this.VerifyCSharpFix(test, withFix);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void JTFRunInAsyncMethodGeneratesWarning()
|
||||
{
|
||||
|
@ -241,6 +287,171 @@ class Test {
|
|||
this.VerifyCSharpFix(test, withFix);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TaskOfTResultInTaskReturningAnonymousMethodWithinSyncMethod_GeneratesWarning()
|
||||
{
|
||||
var test = @"
|
||||
using System;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
class Test {
|
||||
void T() {
|
||||
Func<Task<int>> f = delegate {
|
||||
Task<int> t = null;
|
||||
int result = t.Result;
|
||||
return Task.FromResult(result);
|
||||
};
|
||||
}
|
||||
}
|
||||
";
|
||||
|
||||
var withFix = @"
|
||||
using System;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
class Test {
|
||||
void T() {
|
||||
Func<Task<int>> f = async delegate {
|
||||
Task<int> t = null;
|
||||
int result = await t;
|
||||
return result;
|
||||
};
|
||||
}
|
||||
}
|
||||
";
|
||||
|
||||
this.expect.Locations = new[] { new DiagnosticResultLocation("Test0.cs", 9, 28, 9, 34) };
|
||||
this.VerifyCSharpDiagnostic(test, this.expect);
|
||||
this.VerifyCSharpFix(test, withFix);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TaskOfTResultInTaskReturningSimpleLambdaWithinSyncMethod_GeneratesWarning()
|
||||
{
|
||||
var test = @"
|
||||
using System;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
class Test {
|
||||
void T() {
|
||||
Func<int, Task<int>> f = a => {
|
||||
Task<int> t = null;
|
||||
int result = t.Result;
|
||||
return Task.FromResult(result);
|
||||
};
|
||||
}
|
||||
}
|
||||
";
|
||||
|
||||
var withFix = @"
|
||||
using System;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
class Test {
|
||||
void T() {
|
||||
Func<int, Task<int>> f = async a => {
|
||||
Task<int> t = null;
|
||||
int result = await t;
|
||||
return result;
|
||||
};
|
||||
}
|
||||
}
|
||||
";
|
||||
|
||||
this.expect.Locations = new[] { new DiagnosticResultLocation("Test0.cs", 9, 28, 9, 34) };
|
||||
this.VerifyCSharpDiagnostic(test, this.expect);
|
||||
this.VerifyCSharpFix(test, withFix);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TaskOfTResultInTaskReturningSimpleLambdaExpressionWithinSyncMethod_GeneratesWarning()
|
||||
{
|
||||
var test = @"
|
||||
using System;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
class Test {
|
||||
void T() {
|
||||
Task<int> b = null;
|
||||
Func<int, Task<int>> f = a => Task.FromResult(b.Result);
|
||||
}
|
||||
}
|
||||
";
|
||||
|
||||
var withFix = @"
|
||||
using System;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
class Test {
|
||||
void T() {
|
||||
Task<int> b = null;
|
||||
Func<int, Task<int>> f = async a => await b;
|
||||
}
|
||||
}
|
||||
";
|
||||
|
||||
this.expect.Locations = new[] { new DiagnosticResultLocation("Test0.cs", 8, 57, 8, 63) };
|
||||
this.VerifyCSharpDiagnostic(test, this.expect);
|
||||
this.VerifyCSharpFix(test, withFix);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TaskOfTResultInTaskReturningParentheticalLambdaWithinSyncMethod_GeneratesWarning()
|
||||
{
|
||||
var test = @"
|
||||
using System;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
class Test {
|
||||
void T() {
|
||||
Func<Task<int>> f = () => {
|
||||
Task<int> t = null;
|
||||
int result = t.Result;
|
||||
return Task.FromResult(result);
|
||||
};
|
||||
}
|
||||
}
|
||||
";
|
||||
|
||||
var withFix = @"
|
||||
using System;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
class Test {
|
||||
void T() {
|
||||
Func<Task<int>> f = async () => {
|
||||
Task<int> t = null;
|
||||
int result = await t;
|
||||
return result;
|
||||
};
|
||||
}
|
||||
}
|
||||
";
|
||||
|
||||
this.expect.Locations = new[] { new DiagnosticResultLocation("Test0.cs", 9, 28, 9, 34) };
|
||||
this.VerifyCSharpDiagnostic(test, this.expect);
|
||||
this.VerifyCSharpFix(test, withFix);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TaskOfTResultInTaskReturningMethodAnonymousDelegate_GeneratesNoWarning()
|
||||
{
|
||||
var test = @"
|
||||
using System;
|
||||
using System.Threading.Tasks;
|
||||
|
||||
class Test {
|
||||
Task<int> T() {
|
||||
Task<int> task = null;
|
||||
task.ContinueWith(t => { Console.WriteLine(t.Result); });
|
||||
return Task.FromResult(1);
|
||||
}
|
||||
}
|
||||
";
|
||||
|
||||
this.VerifyCSharpDiagnostic(test);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TaskGetAwaiterGetResultInTaskReturningMethodGeneratesWarning()
|
||||
{
|
||||
|
|
|
@ -207,6 +207,50 @@ namespace Microsoft.VisualStudio.Threading.Analyzers
|
|||
return interfaceImplementations;
|
||||
}
|
||||
|
||||
internal static AnonymousFunctionExpressionSyntax MakeMethodAsync(this AnonymousFunctionExpressionSyntax method, SemanticModel semanticModel, CancellationToken cancellationToken)
|
||||
{
|
||||
if (method.AsyncKeyword.Kind() == SyntaxKind.AsyncKeyword)
|
||||
{
|
||||
// already async
|
||||
return method;
|
||||
}
|
||||
|
||||
var methodSymbol = (IMethodSymbol)semanticModel.GetSymbolInfo(method, cancellationToken).Symbol;
|
||||
bool hasReturnValue = (methodSymbol?.ReturnType as INamedTypeSymbol)?.IsGenericType ?? false;
|
||||
AnonymousFunctionExpressionSyntax updated = null;
|
||||
|
||||
var simpleLambda = method as SimpleLambdaExpressionSyntax;
|
||||
if (simpleLambda != null)
|
||||
{
|
||||
updated = simpleLambda
|
||||
.WithAsyncKeyword(SyntaxFactory.Token(SyntaxKind.AsyncKeyword))
|
||||
.WithBody(UpdateStatementsForAsyncMethod(simpleLambda.Body, semanticModel, hasReturnValue));
|
||||
}
|
||||
|
||||
var parentheticalLambda = method as ParenthesizedLambdaExpressionSyntax;
|
||||
if (parentheticalLambda != null)
|
||||
{
|
||||
updated = parentheticalLambda
|
||||
.WithAsyncKeyword(SyntaxFactory.Token(SyntaxKind.AsyncKeyword))
|
||||
.WithBody(UpdateStatementsForAsyncMethod(parentheticalLambda.Body, semanticModel, hasReturnValue));
|
||||
}
|
||||
|
||||
var anonymousMethod = method as AnonymousMethodExpressionSyntax;
|
||||
if (anonymousMethod != null)
|
||||
{
|
||||
updated = anonymousMethod
|
||||
.WithAsyncKeyword(SyntaxFactory.Token(SyntaxKind.AsyncKeyword))
|
||||
.WithBody(UpdateStatementsForAsyncMethod(anonymousMethod.Body, semanticModel, hasReturnValue));
|
||||
}
|
||||
|
||||
if (updated == null)
|
||||
{
|
||||
throw new NotSupportedException();
|
||||
}
|
||||
|
||||
return updated;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Converts a synchronous method to be asynchronous, if it is not already async.
|
||||
/// </summary>
|
||||
|
@ -227,9 +271,34 @@ namespace Microsoft.VisualStudio.Threading.Analyzers
|
|||
}
|
||||
|
||||
// Fix up any return statements to await on the Task it would have returned.
|
||||
bool hasResultValue = method.ReturnType is GenericNameSyntax;
|
||||
var fixedUpAsyncMethod = method.ReplaceNodes(
|
||||
method.Body.Statements.OfType<ReturnStatementSyntax>(),
|
||||
MethodDeclarationSyntax fixedUpAsyncMethod = method
|
||||
.WithBody(UpdateStatementsForAsyncMethod(method.Body, semanticModel, method.ReturnType is GenericNameSyntax))
|
||||
.AddModifiers(SyntaxFactory.Token(SyntaxKind.AsyncKeyword));
|
||||
|
||||
return fixedUpAsyncMethod;
|
||||
}
|
||||
|
||||
private static CSharpSyntaxNode UpdateStatementsForAsyncMethod(CSharpSyntaxNode body, SemanticModel semanticModel, bool hasResultValue)
|
||||
{
|
||||
var blockBody = body as BlockSyntax;
|
||||
if (blockBody != null)
|
||||
{
|
||||
return UpdateStatementsForAsyncMethod(blockBody, semanticModel, hasResultValue);
|
||||
}
|
||||
|
||||
var expressionBody = body as ExpressionSyntax;
|
||||
if (expressionBody != null)
|
||||
{
|
||||
return SyntaxFactory.AwaitExpression(expressionBody).TrySimplify(expressionBody, semanticModel);
|
||||
}
|
||||
|
||||
throw new NotSupportedException();
|
||||
}
|
||||
|
||||
private static BlockSyntax UpdateStatementsForAsyncMethod(BlockSyntax body, SemanticModel semanticModel, bool hasResultValue)
|
||||
{
|
||||
var fixedUpBlock = body.ReplaceNodes(
|
||||
body.DescendantNodes().OfType<ReturnStatementSyntax>(),
|
||||
(f, n) =>
|
||||
{
|
||||
if (hasResultValue)
|
||||
|
@ -237,7 +306,7 @@ namespace Microsoft.VisualStudio.Threading.Analyzers
|
|||
return n.WithExpression(SyntaxFactory.AwaitExpression(n.Expression).TrySimplify(f.Expression, semanticModel));
|
||||
}
|
||||
|
||||
if (method.Body.Statements.Last() == f)
|
||||
if (body.Statements.Last() == f)
|
||||
{
|
||||
// If it is the last statement in the method, we can remove it since a return is implied.
|
||||
return null;
|
||||
|
@ -246,10 +315,9 @@ namespace Microsoft.VisualStudio.Threading.Analyzers
|
|||
return n
|
||||
.WithExpression(null) // don't return any value
|
||||
.WithReturnKeyword(n.ReturnKeyword.WithTrailingTrivia(SyntaxFactory.TriviaList())); // remove the trailing space after the keyword
|
||||
})
|
||||
.AddModifiers(SyntaxFactory.Token(SyntaxKind.AsyncKeyword));
|
||||
});
|
||||
|
||||
return fixedUpAsyncMethod;
|
||||
return fixedUpBlock;
|
||||
}
|
||||
|
||||
private static ExpressionSyntax TrySimplify(this AwaitExpressionSyntax awaitExpression, ExpressionSyntax originalSyntax, SemanticModel semanticModel)
|
||||
|
|
|
@ -60,16 +60,9 @@
|
|||
|
||||
context.RegisterCodeBlockStartAction<SyntaxKind>(ctxt =>
|
||||
{
|
||||
// We want to scan invocations that occur inside Task and Task<T>-returning methods.
|
||||
// That is: methods that either are or could be made async.
|
||||
var methodSymbol = ctxt.OwningSymbol as IMethodSymbol;
|
||||
var returnType = methodSymbol?.ReturnType;
|
||||
if (returnType != null && returnType.Name == nameof(Task) && returnType.BelongsToNamespace(Namespaces.SystemThreadingTasks))
|
||||
{
|
||||
var methodAnalyzer = new MethodAnalyzer();
|
||||
ctxt.RegisterSyntaxNodeAction(methodAnalyzer.AnalyzeInvocation, SyntaxKind.InvocationExpression);
|
||||
ctxt.RegisterSyntaxNodeAction(methodAnalyzer.AnalyzePropertyGetter, SyntaxKind.SimpleMemberAccessExpression);
|
||||
}
|
||||
var methodAnalyzer = new MethodAnalyzer();
|
||||
ctxt.RegisterSyntaxNodeAction(methodAnalyzer.AnalyzeInvocation, SyntaxKind.InvocationExpression);
|
||||
ctxt.RegisterSyntaxNodeAction(methodAnalyzer.AnalyzePropertyGetter, SyntaxKind.SimpleMemberAccessExpression);
|
||||
});
|
||||
}
|
||||
|
||||
|
@ -78,49 +71,77 @@
|
|||
internal void AnalyzePropertyGetter(SyntaxNodeAnalysisContext context)
|
||||
{
|
||||
var memberAccessSyntax = (MemberAccessExpressionSyntax)context.Node;
|
||||
InspectMemberAccess(context, memberAccessSyntax, CommonInterest.SyncBlockingProperties);
|
||||
if (IsInTaskReturningMethodOrDelegate(context))
|
||||
{
|
||||
InspectMemberAccess(context, memberAccessSyntax, CommonInterest.SyncBlockingProperties);
|
||||
}
|
||||
}
|
||||
|
||||
internal void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
|
||||
{
|
||||
var invocationExpressionSyntax = (InvocationExpressionSyntax)context.Node;
|
||||
var memberAccessSyntax = invocationExpressionSyntax.Expression as MemberAccessExpressionSyntax;
|
||||
if (InspectMemberAccess(context, memberAccessSyntax, CommonInterest.SyncBlockingMethods))
|
||||
if (IsInTaskReturningMethodOrDelegate(context))
|
||||
{
|
||||
// Don't return double-diagnostics.
|
||||
return;
|
||||
}
|
||||
|
||||
// Also consider all method calls to check for Async-suffixed alternatives.
|
||||
SimpleNameSyntax invokedMethodName = memberAccessSyntax?.Name ?? invocationExpressionSyntax.Expression as IdentifierNameSyntax;
|
||||
var symbolInfo = context.SemanticModel.GetSymbolInfo(invocationExpressionSyntax, context.CancellationToken);
|
||||
var methodSymbol = symbolInfo.Symbol as IMethodSymbol;
|
||||
if (symbolInfo.Symbol != null && !symbolInfo.Symbol.Name.EndsWith(VSSDK010AsyncSuffixAnalyzer.MandatoryAsyncSuffix) &&
|
||||
!(methodSymbol?.ReturnType?.Name == nameof(Task) && methodSymbol.ReturnType.BelongsToNamespace(Namespaces.SystemThreadingTasks)))
|
||||
{
|
||||
string asyncMethodName = symbolInfo.Symbol.Name + VSSDK010AsyncSuffixAnalyzer.MandatoryAsyncSuffix;
|
||||
var asyncMethodMatches = context.SemanticModel.LookupSymbols(
|
||||
invocationExpressionSyntax.Expression.GetLocation().SourceSpan.Start,
|
||||
symbolInfo.Symbol.ContainingType,
|
||||
asyncMethodName,
|
||||
includeReducedExtensionMethods: true).OfType<IMethodSymbol>();
|
||||
if (asyncMethodMatches.Any(m => !m.IsObsolete()))
|
||||
var invocationExpressionSyntax = (InvocationExpressionSyntax)context.Node;
|
||||
var memberAccessSyntax = invocationExpressionSyntax.Expression as MemberAccessExpressionSyntax;
|
||||
if (InspectMemberAccess(context, memberAccessSyntax, CommonInterest.SyncBlockingMethods))
|
||||
{
|
||||
// An async alternative exists.
|
||||
var properties = ImmutableDictionary<string, string>.Empty
|
||||
.Add(VSSDK008UseAwaitInAsyncMethodsCodeFix.AsyncMethodKeyName, asyncMethodName);
|
||||
// Don't return double-diagnostics.
|
||||
return;
|
||||
}
|
||||
|
||||
Diagnostic diagnostic = Diagnostic.Create(
|
||||
Descriptor,
|
||||
invokedMethodName.GetLocation(),
|
||||
properties,
|
||||
invokedMethodName.Identifier.Text,
|
||||
asyncMethodName);
|
||||
context.ReportDiagnostic(diagnostic);
|
||||
// Also consider all method calls to check for Async-suffixed alternatives.
|
||||
SimpleNameSyntax invokedMethodName = memberAccessSyntax?.Name ?? invocationExpressionSyntax.Expression as IdentifierNameSyntax;
|
||||
var symbolInfo = context.SemanticModel.GetSymbolInfo(invocationExpressionSyntax, context.CancellationToken);
|
||||
var methodSymbol = symbolInfo.Symbol as IMethodSymbol;
|
||||
if (symbolInfo.Symbol != null && !symbolInfo.Symbol.Name.EndsWith(VSSDK010AsyncSuffixAnalyzer.MandatoryAsyncSuffix) &&
|
||||
!(methodSymbol?.ReturnType?.Name == nameof(Task) && methodSymbol.ReturnType.BelongsToNamespace(Namespaces.SystemThreadingTasks)))
|
||||
{
|
||||
string asyncMethodName = symbolInfo.Symbol.Name + VSSDK010AsyncSuffixAnalyzer.MandatoryAsyncSuffix;
|
||||
var asyncMethodMatches = context.SemanticModel.LookupSymbols(
|
||||
invocationExpressionSyntax.Expression.GetLocation().SourceSpan.Start,
|
||||
symbolInfo.Symbol.ContainingType,
|
||||
asyncMethodName,
|
||||
includeReducedExtensionMethods: true).OfType<IMethodSymbol>();
|
||||
if (asyncMethodMatches.Any(m => !m.IsObsolete()))
|
||||
{
|
||||
// An async alternative exists.
|
||||
var properties = ImmutableDictionary<string, string>.Empty
|
||||
.Add(VSSDK008UseAwaitInAsyncMethodsCodeFix.AsyncMethodKeyName, asyncMethodName);
|
||||
|
||||
Diagnostic diagnostic = Diagnostic.Create(
|
||||
Descriptor,
|
||||
invokedMethodName.GetLocation(),
|
||||
properties,
|
||||
invokedMethodName.Identifier.Text,
|
||||
asyncMethodName);
|
||||
context.ReportDiagnostic(diagnostic);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static bool IsInTaskReturningMethodOrDelegate(SyntaxNodeAnalysisContext context)
|
||||
{
|
||||
// We want to scan invocations that occur inside Task and Task<T>-returning delegates or methods.
|
||||
// That is: methods that either are or could be made async.
|
||||
IMethodSymbol methodSymbol = null;
|
||||
var anonymousFunc = context.Node.FirstAncestorOrSelf<AnonymousFunctionExpressionSyntax>();
|
||||
if (anonymousFunc != null)
|
||||
{
|
||||
var symbolInfo = context.SemanticModel.GetSymbolInfo(anonymousFunc, context.CancellationToken);
|
||||
methodSymbol = symbolInfo.Symbol as IMethodSymbol;
|
||||
}
|
||||
else
|
||||
{
|
||||
var methodDecl = context.Node.FirstAncestorOrSelf<MethodDeclarationSyntax>();
|
||||
methodSymbol = context.SemanticModel.GetDeclaredSymbol(methodDecl, context.CancellationToken);
|
||||
}
|
||||
|
||||
var returnType = methodSymbol?.ReturnType;
|
||||
return returnType?.Name == nameof(Task)
|
||||
&& returnType.BelongsToNamespace(Namespaces.SystemThreadingTasks);
|
||||
}
|
||||
|
||||
private static bool InspectMemberAccess(SyntaxNodeAnalysisContext context, MemberAccessExpressionSyntax memberAccessSyntax, IReadOnlyList<CommonInterest.SyncBlockingMethod> problematicMethods)
|
||||
{
|
||||
if (memberAccessSyntax == null)
|
||||
|
|
|
@ -103,14 +103,25 @@
|
|||
// and that renders the default semantic model broken (even though we've already updated the document's SyntaxRoot?!).
|
||||
// So after acquiring the semantic model, update it with the new method body.
|
||||
var semanticModel = await document.GetSemanticModelAsync(cancellationToken);
|
||||
var originalAnonymousMethodContainerIfApplicable = syncMethodName.FirstAncestorOrSelf<AnonymousFunctionExpressionSyntax>();
|
||||
var originalMethodDeclaration = syncMethodName.FirstAncestorOrSelf<MethodDeclarationSyntax>();
|
||||
if (!semanticModel.TryGetSpeculativeSemanticModelForMethodBody(this.diagnostic.Location.SourceSpan.Start, originalMethodDeclaration, out semanticModel))
|
||||
{
|
||||
throw new InvalidOperationException("Unable to get updated semantic model.");
|
||||
}
|
||||
|
||||
// Ensure that the method is using the async keyword.
|
||||
var updatedMethod = originalMethodDeclaration.MakeMethodAsync(semanticModel);
|
||||
// Ensure that the method or anonymous delegate is using the async keyword.
|
||||
MethodDeclarationSyntax updatedMethod;
|
||||
if (originalAnonymousMethodContainerIfApplicable != null)
|
||||
{
|
||||
updatedMethod = originalMethodDeclaration.ReplaceNode(
|
||||
originalAnonymousMethodContainerIfApplicable,
|
||||
originalAnonymousMethodContainerIfApplicable.MakeMethodAsync(semanticModel, cancellationToken));
|
||||
}
|
||||
else
|
||||
{
|
||||
updatedMethod = originalMethodDeclaration.MakeMethodAsync(semanticModel);
|
||||
}
|
||||
|
||||
if (updatedMethod != originalMethodDeclaration)
|
||||
{
|
||||
|
|
Загрузка…
Ссылка в новой задаче