microsoft / durabletask-dotnet

Out-of-process .NET SDK for the Durable Task Framework
MIT License
114 stars 33 forks source link

Add code analyzer #253

Open jviau opened 11 months ago

jviau commented 11 months ago

This issue is copied from https://github.com/Azure/azure-functions-durable-extension/issues/2682. Original author is @Alan-Hinton

Is your feature request related to a problem? Please describe. Durable functions are very useful, however there are multiple gotchas that the compiler doesn't catch. We use isolated mode c# function apps so we don't have access to the Analyzer to help us avoid these issue at build time. These issues therefore take much longer to find and fix at runtime.

Are there any existing GitHub discussions or issues filed that help give some context to this proposal? Not that I can see

Describe the solution you'd like I would like the durable function Analyzer to work for isolated mode c# function apps.

Describe alternatives you've considered At the moment we are hitting errors at runtime and then fixing them. Rather than seeing them at build time.

Additional context Common durable function gotchas (not sure which of these the Analyzer handles)

allantargino commented 4 months ago

Hey @jviau, is it ok if we close this issue in favor of more specific ones? I just created 2 new issues - the rest of the features asked in the description is already covered by the current implementation.

Alan-Hinton commented 4 months ago

It doesn't seem that the Analyzer is catching any of these errors e.g.

[Function(nameof(Activity))]
public async Task<string> Activity([ActivityTrigger] string param)  => await Task.Run(() => param);

[Function(nameof(Orchestrator))]
public async Task Orchestrator([OrchestrationTrigger] TaskOrchestrationContext context)
{
    var param= DateTime.Now;  // expect error due to using DateTime.Now
    var result = await context.CallActivityAsync<int>(nameof(Activity), param);  // Expect error because param is a DateTime, not a string; also expect an error because result is an int, not a string
    var result2 = await Activity("test") // expect an error due to calling an async function directly from an orchestrator
}
allantargino commented 4 months ago

Hey @Alan-Hinton, how are you using the Analyzer? Did you manually add a reference to this assembly? I am asking because it wasn't released it (it should be soon).

I tested your snippet, and the analyzer were able to catch the 3/4 expected errors: image

The direct async function call detection is not supported yet, you are right.

Alan-Hinton commented 4 months ago

Hey @Alan-Hinton, how are you using the Analyzer? Did you manually add a reference to this assembly? I am asking because it wasn't released it (it should be soon).

I followed the instructions, in the readme of this repo, to add the 'preview-1' version of the analyzer. I look forward to the release of the new version with these improvements. Hopefully this issue will be closed when the new version is released, so I get a notification.