Azure-functions-durable-extension: Analyzer - Cater for activity constants and/or suppress DF109 warnings

[wahyuen]

Description

A clear and concise description of what the bug is. Please make an effort to fill in all the sections below; the information will help us investigate your issue.

We currently have a number of functions where we make reference to the activity name via a constant

eg.

await context.CallActivityAsync(SendComputerVisionWorkToMonitor.EnqueuePerformComputerVisionRequestMessageActivityName,...

This currently raises a warning from the analyzer with

ProcessComputerVision.cs(49,29): warning DF0109: Activity function named 'SendComputerVisionWorkToMonitor.EnqueuePerformComputerVisionRequestMessageActivityName' does not exist. Did you mean

Expected behavior

A clear and concise description of what you expected to happen.

Ability to resolve constant names when performing this check

Actual behavior

A clear and concise description of what actually happened.

Warning is thrown

Relevant source code snippets

await context.CallActivityAsync(SendComputerVisionWorkToMonitor.EnqueuePerformComputerVisionRequestMessageActivityName,

public const string EnqueuePerformComputerVisionRequestMessageActivityName = "EnqueuePerformComputerVisionRequestMessage";

Known workarounds

Provide a description of any known workarounds you used.

If we can't achieve this check with constants, would there be a way to provide a way to suppress or exclude this check on a case by case basis? (some opt out attribute?)

App Details

• Durable Functions extension version (e.g. v1.8.3): 2.2.0
• Azure Functions runtime version (1.0 or 2.0): 3
• Programming language used: .Net

[amdeel]

Thanks for bringing this up!

You should be able to disable this entire analyzer under dependencies -> analyzers -> Microsoft....DurableTask.Analyzers -> DF109. Right click to set custom severity to none.

Alternatively, if you want to only disable this analyzer for specific cases, you can right click on the warning in the visual studio ErrorList to suppress the warning. It will generate code for you around the flagged code.

[ghost]

Same goes for other static resolvers such as nameof(FunctionName). 😄

[sebader]

observed the same as @jublair. I would actually not like to supress this since IMHO using nameof() to reference Functions is actually best practice over using constant strings. The analyzer should correctly detect the use of nameof() - and maybe even discourage the use of strings?!

[wahyuen]

Out of interest...why the preference of nameof vs constant strings? Do they not both evaluate to a compile time constant anyway?

[sebader]

because then you still need to make sure that the constant is always up-to-date with the name of the actual function. Using nameof the name of the function is the single point of truth. (happy for somebody to tell me that there is a better way)

[wahyuen]

public const string FooConstant = "Example";

[FunctionName(FooConstant)]
public async Task BlahFunction(...) {}

We generally have followed the above pattern, still allowing for single point of truth, but allowing us to better name our Functions for debugging in App Insights. Still interesting thought regarding nameof(), have never really considered using that before! :)

[cgillum]

We actually did have a PR submitted that was intended to handle usage of nameof(...): https://github.com/Azure/azure-functions-durable-extension/pull/1263. For the folks running into problems with nameof, are you using v0.2.0 of the analyzer? A code snippet of what isn't working would be helpful in case the implementation of the analyzer is working for some cases but not others.

[ghost]

Ah - this seems to be tripping for CallActivityAsync on my end:

await context.CallActivityAsync(nameof(FunctionName), input);

We use the nameof() extensively in our durable functions do ensure that function renames do not break any dependent function calls.

[ghost]

We also receive this warning when generating the activity name dynamically, for instance:

await context.CallActivityAsync(function, comb);

I reckon that dynamically named variables should have this warning suppressed too, since right now the warning assumes in the above example that the function name is literally "function", and the onus should be on the developer to ensure they're getting the function name right in this instance.

[sebader]

@cgillum I'm using 0.2.0:

//edit: It does not complain where I start a new Orchestrator in the same way:
await starter.StartNewAsync(nameof(UpdateAvailabilityOrchestrator), hutIds);

[amdeel]

@sebader The suggestion to suppress the warning was only to temporarily eliminate the warnings if desired. This will be fixed in our next release of the analyzer. 0.2.0 does not have the PR mentioned above.

[marcduiker]

Yes, nameof(...) should be detected properly in the next release of the Analyzer :).

I think the check of using constants will be a nice one for me to pick up if you are ok with that @amdeel.

[amdeel]

@marcduiker Sure, feel free to work on this! Thank you! :)

[DenisBalan]

Same issue, with nameof, as we treat warning as errors, this causes ci build failing fixed via additional steps to disable analyzer.