Skip DAPR1301 analyzer when AddDaprWorkflowVersioning is present#1778
Skip DAPR1301 analyzer when AddDaprWorkflowVersioning is present#1778
Conversation
… versioning references Agent-Logs-Url: https://github.com/dapr/dotnet-sdk/sessions/ea469658-b9f0-4c16-9227-6fdb2a6b3f4b Co-authored-by: WhitWaldo <2238529+WhitWaldo@users.noreply.github.com>
…1301 versioning check Agent-Logs-Url: https://github.com/dapr/dotnet-sdk/sessions/ea469658-b9f0-4c16-9227-6fdb2a6b3f4b Co-authored-by: WhitWaldo <2238529+WhitWaldo@users.noreply.github.com>
…l not apply when Dapr.Workflow.Versioning is installed. Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1778 +/- ##
=======================================
Coverage 61.18% 61.18%
=======================================
Files 285 285
Lines 8405 8405
Branches 974 974
=======================================
Hits 5143 5143
Misses 3026 3026
Partials 236 236
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WhitWaldo
left a comment
There was a problem hiding this comment.
There are a few changes I'd like to see addressed
| foreach (var invocation in root.DescendantNodes().OfType<InvocationExpressionSyntax>()) | ||
| { | ||
| if (invocation.Expression is MemberAccessExpressionSyntax memberAccess && | ||
| memberAccess.Name.Identifier.Text == "AddDaprWorkflowVersioning") |
There was a problem hiding this comment.
While a minor point, I would like for this to look for the AddDaprWorkflowVersioning method from the Dapr.Workflow.Versioning assembly, not just any at all (though we're unlikely to have such a conflict).
There was a problem hiding this comment.
Done in a80f353. The check now uses the deferred-diagnostics pattern to get a proper semantic model: when the versioning type is present in the compilation, a RegisterSyntaxNodeAction handler fires on every InvocationExpression and calls semanticModel.GetSymbolInfo(invocation) to verify the resolved method's ContainingType equals WorkflowVersioningServiceCollectionExtensions. Only if that exact Dapr method is confirmed does the RegisterCompilationEndAction suppress the collected diagnostics.
| context.RegisterSyntaxNodeAction(AnalyzeWorkflowRegistration, SyntaxKind.InvocationExpression); | ||
| context.RegisterCompilationStartAction(compilationContext => | ||
| { | ||
| if (CheckIfWorkflowVersioningIsRegistered(compilationContext.Compilation)) |
There was a problem hiding this comment.
While workflow versioning will automatically pick up all the workflows, I would also like for the analyzer to still run in case the user happens to define their own Workflow DI registrations, so can you ensure this scenario is covered as well?
There was a problem hiding this comment.
Done in a80f353. The deferred-diagnostics pattern keeps the per-workflow CheckIfWorkflowIsRegistered logic fully active even when versioning is present. For every ScheduleNewWorkflowAsync(nameof(X)) call, a node action checks explicit RegisterWorkflow<X> registrations first — if found, no diagnostic is added. If not found, a pending diagnostic is collected and later suppressed only because AddDaprWorkflowVersioning was confirmed. I also added a test VerifyWorkflowRegisteredWithVersioningPresent covering the combination of versioning + explicit registration.
…ks active; fix cross-tree semantic model crash Agent-Logs-Url: https://github.com/dapr/dotnet-sdk/sessions/6e1d6688-99e4-4438-aed2-56f6e529ecf3 Co-authored-by: WhitWaldo <2238529+WhitWaldo@users.noreply.github.com>
With Dapr 1.17 workflow versioning (
Dapr.Workflow.Versioning), workflows are auto-registered by a source generator — manualRegisterWorkflow<T>()calls are not required. The DAPR1301 analyzer was unaware of this, producing spurious warnings (and AD0001 crashes) in projects usingAddDaprWorkflowVersioning().Description
Analyzer (
WorkflowRegistrationAnalyzer): uses the deferred-diagnostics pattern to suppress DAPR1301 whenAddDaprWorkflowVersioningfrom the Dapr versioning package is detected.RegisterCompilationStartActionchecks once per compilation whetherWorkflowVersioningServiceCollectionExtensionsis present in the referenced assemblies viaGetTypeByMetadataName, preventing a user-defined method with the same name from accidentally suppressing the diagnostic.RegisterSyntaxNodeActionhandlers are registered: one semantically verifies thatAddDaprWorkflowVersioningresolves toWorkflowVersioningServiceCollectionExtensions.AddDaprWorkflowVersioning(the actual Dapr method), and one collects potential DAPR1301 diagnostics — keeping explicitRegisterWorkflow<T>checks active per workflow call.RegisterCompilationEndActionsuppresses the collected diagnostics only if the Dapr versioning call was confirmed; otherwise they are reported as normal.CheckIfWorkflowIsRegistered: same-tree invocations use semantic symbol comparison; invocations in other files fall back to syntactic name comparison (avoids RS1030).Tests (
Dapr.Workflow.Analyzers.Test):VerifyWorkflowNotRegisteredButVersioningPresent— verifies no DAPR1301 is emitted whenservices.AddDaprWorkflowVersioning()is present and no manualRegisterWorkflow<T>()exists.VerifyWorkflowRegisteredWithVersioningPresent— verifies no DAPR1301 is emitted when bothAddDaprWorkflowVersioningand an explicitRegisterWorkflow<T>are present.Dapr.IntegrationTest.Workflow.Versioning(Dapr.Workflow.Versioning.Abstractions+Dapr.Workflow.Versioning.Runtime).Issue reference
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: