Fix Anaylzers treating passed in variable argument name as null

[YunchuWang]

Summary

What changed?

• Added a null check in RoslynExtensions.GetConstantValueFromAttribute to handle cases where the name argument to AddOrchestratorFunc is not a string literal (e.g., a loop variable in a foreach).
• Added 5 new unit tests across DateTimeOrchestrationAnalyzerTests, DelayOrchestrationAnalyzerTests, and GuidOrchestrationAnalyzerTests to cover the foreach + variable name scenario.

Why is this change needed?

All orchestration analyzers (DateTimeOrchestrationAnalyzer, DelayOrchestrationAnalyzer, GuidOrchestrationAnalyzer, etc.) throw System.ArgumentNullException with message Value cannot be null. (Parameter 'node') when AddOrchestratorFunc is called with an inline lambda inside a foreach loop that captures a loop-scoped variable. This surfaces as AD0001 errors and breaks builds with TreatWarningsAsErrors.

The root cause is that GetConstantValueFromAttribute searches for a LiteralExpressionSyntax descendant in the argument's syntax. When the name is a variable rather than a string literal, FirstOrDefault() returns null, which is passed directly to SemanticModel.GetConstantValue(), causing the crash.

Issues / work items

• Resolves https://github.com/kshyju/DurableTaskAnalyzersBugRepro

---

Project checklist

• Release notes are not required for the next release
  • Otherwise: Notes added to release_notes.md
• Backport is not required
  • Otherwise: Backport tracked by issue/PR #issue_or_pr
• All required tests have been added/updated (unit tests, E2E tests)
• Breaking change?
  • No — this is a bugfix only

---

AI-assisted code disclosure (required)

Was an AI tool used? (select one)

• No
• Yes, AI helped write parts of this PR (e.g., GitHub Copilot)
• Yes, an AI agent generated most of this PR

If AI was used:

• Tool(s): GitHub Copilot
• AI-assisted areas/files:
  • src/Analyzers/RoslynExtensions.cs — null guard in GetConstantValueFromAttribute
  • test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs — 2 new tests
  • test/Analyzers.Tests/Orchestration/DelayOrchestrationAnalyzerTests.cs — 2 new tests
  • test/Analyzers.Tests/Orchestration/GuidOrchestrationAnalyzerTests.cs — 1 new test
• What you changed after AI output: Reviewed all changes, verified edge cases, confirmed all 142 tests pass

AI verification (required if AI was used):

• I understand the code and can explain it
• I verified referenced APIs/types exist and are correct
• I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
• I reviewed concurrency/async behavior
• I checked for unintended breaking or behavior changes

---

Testing

Automated tests

• Result: Passed — 142/142 tests pass (0 failures, 0 skipped)

New tests added:

Test	File	Purpose
FuncOrchestratorInForEachWithVariableNameNoDiag	DateTimeOrchestrationAnalyzerTests.cs	No crash, no false positive when name is a foreach variable
FuncOrchestratorInForEachWithVariableNameHasDiag	DateTimeOrchestrationAnalyzerTests.cs	Still detects DateTime.Now inside foreach lambda
FuncOrchestratorInForEachWithVariableNameNoDiag	DelayOrchestrationAnalyzerTests.cs	No crash for Delay analyzer
FuncOrchestratorInForEachWithVariableNameHasDiag	DelayOrchestrationAnalyzerTests.cs	Still detects Thread.Sleep inside foreach lambda
FuncOrchestratorInForEachWithVariableNameNoDiag	GuidOrchestrationAnalyzerTests.cs	No crash for Guid analyzer

Manual validation (only if runtime/behavior changed)

• N/A — analyzer-only change, validated through automated tests

---

Notes for reviewers

• The fix is a single null guard in RoslynExtensions.GetConstantValueFromAttribute. When the name argument is not a string literal, the method now returns default instead of crashing. The caller in OrchestrationAnalyzer.cs (line 169) already handles this gracefully via name.Value?.ToString() ?? methodSymbol.Name.
• All 137 pre-existing tests continue to pass unchanged.

[Copilot]

Pull request overview

This PR fixes a critical bug where all orchestration analyzers (DateTime, Delay, Guid, etc.) throw ArgumentNullException when AddOrchestratorFunc is called with a non-literal name argument (e.g., a variable from a foreach loop). The fix adds a simple null check in RoslynExtensions.GetConstantValueFromAttribute to return default when the argument is not a string literal, allowing the caller to gracefully handle the case.

Changes:

• Added null guard in GetConstantValueFromAttribute to prevent crashes when name argument is not a literal
• Added 5 new unit tests across DateTime, Delay, and Guid analyzer tests to cover the foreach + variable name scenario

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/Analyzers/RoslynExtensions.cs	Added null check to prevent ArgumentNullException when argument is not a string literal
test/Analyzers.Tests/Orchestration/DateTimeOrchestrationAnalyzerTests.cs	Added NoDiag and HasDiag tests for foreach with variable name scenario
test/Analyzers.Tests/Orchestration/DelayOrchestrationAnalyzerTests.cs	Added NoDiag and HasDiag tests for foreach with variable name scenario
test/Analyzers.Tests/Orchestration/GuidOrchestrationAnalyzerTests.cs	Added NoDiag test for foreach with variable name scenario

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.