Skip to content

Commit

Permalink
Review update 1, add support for IDurableEntityContext
Browse files Browse the repository at this point in the history
  • Loading branch information
pavel-mikula-sonarsource committed Jun 2, 2022
1 parent 2ce65a5 commit 83618c0
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 19 deletions.
4 changes: 2 additions & 2 deletions analyzers/rspec/cs/S6424_c#.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
<li> Entity interface methods must return void, Task, or Task&lt;T&gt;. </li>
</ul>
<p>If any of these rules are violated, an <code>InvalidOperationException</code> is thrown at runtime when the interface is used as a type argument to
<code>IDurableClient.SignalEntityAsync&lt;T&gt;</code> or <code>IDurableOrchestrationContext.CreateEntityProxy&lt;T&gt;</code>. The exception message
explains which rule was broken.</p>
<code>IDurableEntityContext.SignalEntity&lt;TEntityInterface&gt;</code>, <code>IDurableEntityClient.SignalEntityAsync&lt;TEntityInterface&gt;</code>
or <code>IDurableOrchestrationContext.CreateEntityProxy&lt;TEntityInterface&gt;</code>. The exception message explains which rule was broken.</p>
<p>This rule raises an issue in case any of the restrictions above is not respected.</p>
<h2>Noncompliant Code Example</h2>
<pre>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public sealed class DurableEntityInterfaceRestrictions : SonarDiagnosticAnalyzer
{
private const string DiagnosticId = "S6424";
private const string MessageFormat = "Use valid entity interface. {0} {1}.";
private const string SignalEntityName = "SignalEntity";
private const string SignalEntityAsyncName = "SignalEntityAsync";
private const string CreateEntityProxyName = "CreateEntityProxy";

Expand All @@ -44,11 +45,10 @@ protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterSyntaxNodeActionInNonGenerated(c =>
{
var name = (GenericNameSyntax)c.Node;
if (name.Identifier.ValueText is SignalEntityAsyncName or CreateEntityProxyName
if (name.Identifier.ValueText is SignalEntityName or SignalEntityAsyncName or CreateEntityProxyName
&& name.TypeArgumentList.Arguments.Count == 1
&& c.SemanticModel.GetSymbolInfo(name).Symbol is IMethodSymbol method
&& (method.Is(KnownType.Microsoft_Azure_WebJobs_Extensions_DurableTask_IDurableEntityClient, SignalEntityAsyncName)
|| method.Is(KnownType.Microsoft_Azure_WebJobs_Extensions_DurableTask_IDurableOrchestrationContext, CreateEntityProxyName))
&& IsRestrictedMethod(method)
&& method.TypeArguments.Single() is INamedTypeSymbol { TypeKind: not TypeKind.Error } entityInterface
&& InterfaceErrorMessage(entityInterface) is { } message)
{
Expand All @@ -57,6 +57,11 @@ protected override void Initialize(SonarAnalysisContext context) =>
},
SyntaxKind.GenericName);

private static bool IsRestrictedMethod(IMethodSymbol method) =>
method.Is(KnownType.Microsoft_Azure_WebJobs_Extensions_DurableTask_IDurableEntityContext, SignalEntityName)
|| method.Is(KnownType.Microsoft_Azure_WebJobs_Extensions_DurableTask_IDurableEntityClient, SignalEntityAsyncName)
|| method.Is(KnownType.Microsoft_Azure_WebJobs_Extensions_DurableTask_IDurableOrchestrationContext, CreateEntityProxyName);

private static string InterfaceErrorMessage(INamedTypeSymbol entityInterface)
{
if (entityInterface.IsGenericType)
Expand Down Expand Up @@ -86,15 +91,15 @@ private static string MemberErrorMessage(ISymbol member)
{
return $@"contains method ""{method.Name}"" with {method.Parameters.Length} parameters. Zero or one are allowed";
}
else if (method.ReturnsVoid
else if (!(method.ReturnsVoid
|| method.ReturnType.Is(KnownType.System_Threading_Tasks_Task)
|| method.ReturnType.Is(KnownType.System_Threading_Tasks_Task_T))
|| method.ReturnType.Is(KnownType.System_Threading_Tasks_Task_T)))
{
return null;
return $@"contains method ""{method.Name}"" with invalid return type. Only ""void"", ""Task"" and ""Task<T>"" are allowed";
}
else
{
return $@"contains method ""{method.Name}"" with invalid return type. Only ""void"", ""Task"" and ""Task<T>"" are allowed";
return null;
}
}
}
Expand Down
1 change: 1 addition & 0 deletions analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ internal sealed class KnownType
internal static readonly KnownType Microsoft_AspNetCore_Mvc_RequestSizeLimitAttribute = new("Microsoft.AspNetCore.Mvc.RequestSizeLimitAttribute");
internal static readonly KnownType Microsoft_AspNetCore_Razor_Hosting_RazorCompiledItemAttribute = new("Microsoft.AspNetCore.Razor.Hosting.RazorCompiledItemAttribute");
internal static readonly KnownType Microsoft_Azure_WebJobs_Extensions_DurableTask_IDurableEntityClient = new("Microsoft.Azure.WebJobs.Extensions.DurableTask.IDurableEntityClient");
internal static readonly KnownType Microsoft_Azure_WebJobs_Extensions_DurableTask_IDurableEntityContext = new("Microsoft.Azure.WebJobs.Extensions.DurableTask.IDurableEntityContext");
internal static readonly KnownType Microsoft_Azure_WebJobs_Extensions_DurableTask_IDurableOrchestrationContext = new("Microsoft.Azure.WebJobs.Extensions.DurableTask.IDurableOrchestrationContext");
internal static readonly KnownType Microsoft_Azure_WebJobs_FunctionNameAttribute = new("Microsoft.Azure.WebJobs.FunctionNameAttribute");
internal static readonly KnownType Microsoft_Data_Sqlite_SqliteCommand = new("Microsoft.Data.Sqlite.SqliteCommand");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ public interface IValid
Task<string> TaskStrArg(int count);
}

public interface IEmpty
{
// This is invalid and will throw
}

public interface IInheritsEmptyWithValid : IEmpty
{
void Valid();
Expand All @@ -33,11 +38,6 @@ public interface IInheritsValidIsEmpty2 : IInheritsValidIsEmpty
// Do not add anything => still valid - another level of nesting
}

public interface IEmpty
{
// This is invalid and will throw
}

public interface IInheritsEmptyIsEmpty : IEmpty
{
// This is invalid and will throw
Expand Down Expand Up @@ -107,16 +107,15 @@ public interface IEvent
event EventHandler<EventArgs> Event;
}

public class UseDurableClient
public class UseDurableEntityClient
{
private readonly IDurableClient client;
private readonly IDurableEntityClient client;
private readonly EntityId id;

public async Task UnrelatedMethods()
{
await client.ReadEntityStateAsync<IInvalid>(id);
await client.StartNewAsync<IInvalid>("name", null);
await client.SignalEntityAsync(id, "name"); // Always compliant, same name but not generic
await client.ReadEntityStateAsync<IInvalid>(id); // T is a type of the returned data. It's not an entity interface.
await client.SignalEntityAsync(id, "name"); // Always compliant, same name but not generic
}

public async Task Compliant()
Expand Down Expand Up @@ -152,6 +151,11 @@ public async Task Reasons()
await client.SignalEntityAsync<IIndexer>(id, x => { }); // Noncompliant {{Use valid entity interface. IIndexer contains property "this[]". Only methods are allowed.}}
await client.SignalEntityAsync<IEvent>(id, x => { }); // Noncompliant {{Use valid entity interface. IEvent contains event "Event". Only methods are allowed.}}
}

public async Task FromDurableClient(IDurableClient inheritedClient)
{
await inheritedClient.SignalEntityAsync<IInvalid>(id, x => { }); // Noncompliant
}
}

public class UseDurableOrchestrationContext
Expand Down Expand Up @@ -179,6 +183,20 @@ public void Errors()
}
}

public class UseDurableEntityContext
{
private readonly IDurableEntityContext context;
private readonly EntityId id;

public void Overloads()
{
context.SignalEntity<IInvalid>(id, x => { }); // Noncompliant
context.SignalEntity<IInvalid>("id", x => { }); // Noncompliant
context.SignalEntity<IInvalid>(id, DateTime.Now, x => { }); // Noncompliant
context.SignalEntity<IInvalid>("id", DateTime.Now, x => { }); // Noncompliant
}
}

public class AnotherType
{
public void SignalEntityAsync<T>() { }
Expand Down

0 comments on commit 83618c0

Please sign in to comment.