-
Notifications
You must be signed in to change notification settings - Fork 2
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
Improve GracefulStop()
analyzer and code fix
#60
Improve GracefulStop()
analyzer and code fix
#60
Conversation
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.
Self review
public MyActor() | ||
{ | ||
ReceiveAsync<string>(async str => | ||
{ | ||
await GracefulStop(TimeSpan.FromSeconds(3)); | ||
}); | ||
|
||
ReceiveAnyAsync(async _ => | ||
{ | ||
await GracefulStop(TimeSpan.FromSeconds(3)); | ||
}); | ||
} | ||
|
||
public Task GracefulStop(TimeSpan timeout) | ||
{ | ||
return Task.CompletedTask; | ||
} |
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.
We're not responsible for these kind of shenanigans.
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.
100%.
return methodSymbol is { Name: "GracefulStop", IsExtensionMethod: true } | ||
&& SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingType, akkaContext.AkkaCore.GracefulStopSupportType); |
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.
This checks that the method in question is defined within the GracefulStopSupport
static class.
return; | ||
|
||
var diagnostic = Diagnostic.Create(RuleDescriptors.Ak1002DoNotAwaitOnGracefulStop, awaitExpression.GetLocation()); | ||
ctx.ReportDiagnostic(diagnostic); | ||
}, SyntaxKind.InvocationExpression); | ||
} | ||
|
||
private static bool IsInsideReceiveAsyncLambda(SyntaxNode node, SemanticModel semanticModel, AkkaContext akkaContext) |
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.
Method is moved to CodeAnalysisExtensions.cs, will be used by other analyzers.
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.
Yes, great idea.
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.
Very well done
public MyActor() | ||
{ | ||
ReceiveAsync<string>(async str => | ||
{ | ||
await GracefulStop(TimeSpan.FromSeconds(3)); | ||
}); | ||
|
||
ReceiveAnyAsync(async _ => | ||
{ | ||
await GracefulStop(TimeSpan.FromSeconds(3)); | ||
}); | ||
} | ||
|
||
public Task GracefulStop(TimeSpan timeout) | ||
{ | ||
return Task.CompletedTask; | ||
} |
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.
100%.
return; | ||
|
||
var diagnostic = Diagnostic.Create(RuleDescriptors.Ak1002DoNotAwaitOnGracefulStop, awaitExpression.GetLocation()); | ||
ctx.ReportDiagnostic(diagnostic); | ||
}, SyntaxKind.InvocationExpression); | ||
} | ||
|
||
private static bool IsInsideReceiveAsyncLambda(SyntaxNode node, SemanticModel semanticModel, AkkaContext akkaContext) |
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.
Yes, great idea.
Changes
Make sure that the
GracefulStop()
method being invoked are defined insideGracefulStopSupport
static class and not a custom method defined by the user.