Skip to content
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

Add Roslyn analyzer for workflows #1440

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ngruson
Copy link

@ngruson ngruson commented Jan 9, 2025

Description

This PR adds a Roslyn analyzer that checks if workflows and activities are registered properly.
For workflows, the analyzer checks if a workflow is referenced in DaprWorkflowClient.ScheduleNewWorkflowAsync calls (other logic may be added to determine if a workflow is referenced?).
For workflow activities, the analyzer checks if activities are referenced in workflows in WorkflowContext.CallActivityAsync calls.

For both scenarios, code fix providers have been added to add RegisterWorkflow or RegisterActivity calls to AddDaprWorkflow code.

We have to come up with a general approach for Roslyn analyzers. Maybe we can move these remarks to the issue.

  • Class library per building block (i.e. Dapr.Workflow.Analyzers).
  • Define a range for diagnostic rule IDs. For now, I have used DAPR1001 and DAPR1002 for the diagnostics described above. For each building block, we might define a different range for the first digit following DAPR.
  • The analyzers should be included in the NuGet package for the building block (Dapr.Workflow).

Other open questions:

  • If a workflow and its activities are defined in a class library, it should not require workflow and activity registrations to be in the class library. This is usually done in the hosted application.

Issue reference

This PR belongs to issue #1426.

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@ngruson ngruson requested review from a team as code owners January 9, 2025 16:02
Introduced a new diagnostic analyzer `WorkflowActivityAnalyzer` to check if workflow activities are registered in the DI container. Updated the solution to include new projects for analyzers and their tests. Added unit tests to verify analyzer functionality and updated documentation to reflect the new release and rules. Included necessary package references and streamlined testing setup.

Signed-off-by: Nils Gruson <[email protected]>
Introduce `WorkflowRegistrationAnalyzer` and `WorkflowActivityAnalyzer` to validate workflow and activity registrations in the DI container. Implement corresponding code fix providers to suggest automatic fixes. Add tests to ensure functionality and robustness of the Dapr workflow framework.

Signed-off-by: Nils Gruson <[email protected]>
Signed-off-by: Nils Gruson <[email protected]>
@ngruson ngruson force-pushed the 1426-ng-analyzers branch from c2ac5e7 to 5e9abec Compare January 9, 2025 16:44
@WhitWaldo WhitWaldo added this to the v1.16 milestone Jan 11, 2025
ngruson and others added 3 commits January 11, 2025 15:19
Added `<IncludeBuildOutput>false</IncludeBuildOutput>` to
`Dapr.Workflow.Analyzers.csproj` to prevent the build output
from being included as a library dependency. This change
includes a comment for clarity and aligns with existing
NuGet package properties.

Signed-off-by: Nils Gruson <[email protected]>
Updated the WorkflowRegistrationCodeFixProvider to include logic for obtaining the semantic model and extracting workflow type names. Added methods for finding and creating the AddDaprWorkflow invocation. Modified the Utilities class to reference WebApplication. Expanded test coverage in WorkflowRegistrationCodeFixProviderTests with new cases for handling AddDaprWorkflow invocations, including standard and top-level statements.

Signed-off-by: Nils Gruson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants