docs(workflow): analyzer rule reference + golden real-shape regression tests
docs/modules/workflow/analyzer.md — user-facing reference for
WF001-WF006 + WF010: one section per rule with a "bad" example and
the canonical fix. Covers activation, scope (Spec property is the
entry point; helpers walked transitively), trusted-assembly prefix
rule, cross-project WF010 indirection, and non-goals (no source
generator, no severity config, no escape hatch).
The DiagnosticDescriptors' HelpLinkUri already points at sections in
this doc (e.g., #wf005), so users who hit a build error can click
through to the exact rule explanation.
Golden tests (GoldenWorkflowShapeTests) exercise three patterns
lifted from the Bulstrad corpus:
1. static readonly LegacyRabbitAddress fields + nested
WhenExpression(Gt, Len, ...) + .Call + OnComplete with
WhenExpression(Eq, ...) + ActivateTask/Complete
2. SetBusinessReference(new WorkflowBusinessReferenceDeclaration
{ KeyExpression, Parts = new WorkflowNamedExpressionDefinition[] { ... } })
3. WorkflowExpr.Func("bulstrad.normalizeCustomer", path)
— custom runtime function dispatch
Each asserts zero WF* diagnostics. A regression that rejects these
patterns would break the entire Serdica corpus.
30/30 tests pass.
This commit is contained in:
master
184
docs/modules/workflow/analyzer.md
Normal file
184
docs/modules/workflow/analyzer.md
Normal file
@@ -0,0 +1,184 @@
|
||||
# StellaOps.Workflow.Analyzer
|
||||
|
||||
A Roslyn `DiagnosticAnalyzer` that rejects workflow C# code which cannot be serialized to canonical workflow JSON. Failures are surfaced as compile-time errors at the offending line — no runtime round-trip needed to discover that a workflow is non-canonical.
|
||||
|
||||
## What the analyzer is for
|
||||
|
||||
`IDeclarativeWorkflow<T>` implementations build a `WorkflowSpec<T>` from a fluent builder. At runtime, `WorkflowCanonicalDefinitionCompiler` turns that spec into a canonical JSON document that the engine can execute, persist, diff, and replay.
|
||||
|
||||
For that to work, the spec must be **pure and declarative**: its shape can only depend on compile-time literals and the workflow builder surface, not on build-time state (time, environment, DB reads) or runtime C# semantics (imperative branching, exceptions, async).
|
||||
|
||||
The analyzer enforces this at compile time. Consumer projects pick it up via a `<ProjectReference>` to `StellaOps.Workflow.Analyzer.csproj` (with `OutputItemType="Analyzer"`) or via an `<Analyzer>` item pointing at the built DLL. Once referenced, every `IDeclarativeWorkflow<T>` class in the consumer compilation is checked on every build.
|
||||
|
||||
## Activation
|
||||
|
||||
The analyzer activates **only when the compilation references `StellaOps.Workflow.Abstractions` or `Ablera.Serdica.Workflow.Abstractions`**. Absent those, it is a no-op. No opt-in attribute is needed anywhere.
|
||||
|
||||
## Scope
|
||||
|
||||
The analyzer inspects the `Spec` property of every `IDeclarativeWorkflow<T>`-implementing class in the compilation. From each `Spec` initializer it walks transitively into every method call whose target is available as source:
|
||||
|
||||
- **Trusted leaves** — any method/type in an assembly whose name starts with `StellaOps.Workflow.` or `Ablera.Serdica.Workflow.`. Not walked; always allowed.
|
||||
- **Same compilation / project references** — source walked via `DeclaringSyntaxReferences`. Violations in those methods are reported at their native location (same compilation) or surfaced as **WF010** at the workflow call site with an additional location inside the helper (cross-project).
|
||||
- **Metadata-only (compiled NuGet / framework) references** — not walkable. Invocations, object creations, and non-const field/property reads on such assemblies are **rejected**. If you need a helper routine, put it in a project reference, not a binary dependency.
|
||||
|
||||
## Rules
|
||||
|
||||
All diagnostics are `Error` severity. They fail `dotnet build`.
|
||||
|
||||
### WF001 — Imperative control flow {#wf001}
|
||||
|
||||
**Bad**
|
||||
|
||||
```csharp
|
||||
public WorkflowSpec<R> Spec { get; } = Build();
|
||||
private static WorkflowSpec<R> Build()
|
||||
{
|
||||
if (DateTime.UtcNow.Hour > 12)
|
||||
{
|
||||
return WorkflowSpec.For<R>().InitializeState(...).Build();
|
||||
}
|
||||
return WorkflowSpec.For<R>().InitializeState(...).Build();
|
||||
}
|
||||
```
|
||||
|
||||
**Why it fails** — The shape of the canonical JSON would depend on when the workflow class was constructed. Two callers building the same workflow get different definitions.
|
||||
|
||||
**Fix** — Express branching inside the canonical model:
|
||||
|
||||
```csharp
|
||||
public WorkflowSpec<R> Spec { get; } = WorkflowSpec.For<R>()
|
||||
.StartWith(flow => flow
|
||||
.WhenExpression(
|
||||
"After noon?",
|
||||
WorkflowExpr.Gt(WorkflowExpr.Path("start.hour"), WorkflowExpr.Number(12)),
|
||||
whenTrue => whenTrue.Call(...),
|
||||
whenElse => whenElse.Call(...)))
|
||||
.Build();
|
||||
```
|
||||
|
||||
Forbidden constructs: `if`, `for`, `foreach`, `while`, `do`, `switch` (statement form), `try` / `catch` / `finally`, `throw`, `lock`, `using`, `goto`, `break`, `continue`, `yield`.
|
||||
|
||||
### WF002 — Async/await {#wf002}
|
||||
|
||||
**Bad**
|
||||
|
||||
```csharp
|
||||
public WorkflowSpec<R> Spec { get; } = BuildAsync().GetAwaiter().GetResult();
|
||||
private static async Task<WorkflowSpec<R>> BuildAsync()
|
||||
{
|
||||
await Task.Delay(1);
|
||||
return WorkflowSpec.For<R>().Build();
|
||||
}
|
||||
```
|
||||
|
||||
**Why it fails** — The workflow Spec is constructed once, synchronously. `await` / `async` / `Task.Run` / `Thread` introduce runtime concurrency primitives that cannot be represented in the canonical definition.
|
||||
|
||||
**Fix** — Remove the async machinery. If the work you were doing was async only because it read from a DB or an API, move it out of the Spec: those reads should happen inside workflow steps at runtime (via `builder.Call(...)`), not at definition-construction time.
|
||||
|
||||
### WF003 — Call into non-trusted assembly {#wf003}
|
||||
|
||||
**Bad**
|
||||
|
||||
```csharp
|
||||
public WorkflowSpec<R> Spec { get; } = WorkflowSpec.For<R>()
|
||||
.InitializeState(WorkflowExpr.Obj(
|
||||
WorkflowExpr.Prop("machine", WorkflowExpr.String(Environment.MachineName))))
|
||||
.Build();
|
||||
```
|
||||
|
||||
**Why it fails** — `Environment.MachineName` is a property read on `System.Environment` (in `System.Runtime`, a metadata-only reference). The canonical JSON would bake whatever machine built the workflow.
|
||||
|
||||
**Fix** — For runtime data, bind through a path reference: `WorkflowExpr.Path("start.machineName")`. For literals, use the typed builders: `WorkflowExpr.String("fixed-value")`. For helper routines, put them in a project-referenced class so the analyzer can walk them.
|
||||
|
||||
Lambda invocations via `func()` (where `func` is an `Action`/`Func<T>`/custom delegate) are **not** flagged as WF003 — the lambda body is walked inline. `nameof(...)` expressions are also skipped.
|
||||
|
||||
### WF004 — `new` on non-trusted type {#wf004}
|
||||
|
||||
**Bad**
|
||||
|
||||
```csharp
|
||||
public WorkflowSpec<R> Spec { get; } = WorkflowSpec.For<R>()
|
||||
.InitializeState(WorkflowExpr.Obj(
|
||||
WorkflowExpr.Prop("id", WorkflowExpr.String(new Guid().ToString()))))
|
||||
.Build();
|
||||
```
|
||||
|
||||
**Why it fails** — `new Guid()` instantiates a BCL type at workflow-build time; every class construction produces a different Guid, and the canonical JSON is no longer deterministic.
|
||||
|
||||
**Fix** — Use a workflow expression. Workflow DTOs (`LegacyRabbitAddress`, `WorkflowBusinessReferenceDeclaration`, `WorkflowNamedExpressionDefinition`, etc.) that live in `StellaOps.Workflow.*` / `Ablera.Serdica.Workflow.*` are trusted and may be instantiated freely.
|
||||
|
||||
### WF005 — C# conditional operator in workflow code {#wf005}
|
||||
|
||||
**Bad**
|
||||
|
||||
```csharp
|
||||
WorkflowExpr.Prop("route", WorkflowExpr.String(useFastLane ? "fast" : "slow"))
|
||||
WorkflowExpr.Prop("name", WorkflowExpr.String(input.Name ?? "anonymous"))
|
||||
WorkflowExpr.Prop("length", WorkflowExpr.Number(input?.Name?.Length ?? 0))
|
||||
```
|
||||
|
||||
**Why it fails** — `?:`, `??`, and `?.` resolve at workflow-**build** time. Whichever branch wins when the Spec is constructed is what ends up in the canonical JSON; the runtime condition never enters the workflow at all.
|
||||
|
||||
**Fix** — Use canonical equivalents:
|
||||
|
||||
```csharp
|
||||
WorkflowExpr.Prop("route", WorkflowExpr.Func("if",
|
||||
WorkflowExpr.Path("state.useFastLane"),
|
||||
WorkflowExpr.String("fast"),
|
||||
WorkflowExpr.String("slow")))
|
||||
|
||||
WorkflowExpr.Prop("name", WorkflowExpr.Func("coalesce",
|
||||
WorkflowExpr.Path("start.name"),
|
||||
WorkflowExpr.String("anonymous")))
|
||||
```
|
||||
|
||||
### WF006 — Non-trusted field/property read {#wf006}
|
||||
|
||||
**Bad**
|
||||
|
||||
```csharp
|
||||
WorkflowExpr.Prop("year", WorkflowExpr.Number(DateTime.UtcNow.Year))
|
||||
WorkflowExpr.Prop("cfg", WorkflowExpr.String(MyStaticConfig.CurrentRegion))
|
||||
```
|
||||
|
||||
**Why it fails** — Reading a non-const field or property on a type that isn't in a trusted or source-walkable assembly bakes the read value into the canonical JSON. `DateTime.UtcNow.Year` gives you the year-at-build, not a per-execution value. `MyStaticConfig.CurrentRegion` couples the workflow definition to whatever config was loaded when the plugin assembly initialized.
|
||||
|
||||
**What is allowed**
|
||||
|
||||
- **Compile-time constants** (`const` fields) on any type. `int.MaxValue` and user `public const string Route = "/x"` are always fine.
|
||||
- **Static readonly fields** on types in the current compilation or a project reference — the analyzer can walk the initializer and verify it's canonical. This is how `static readonly LegacyRabbitAddress QueryAddress = new("pas_query");` stays legal.
|
||||
- Any member on a `StellaOps.Workflow.*` / `Ablera.Serdica.Workflow.*` type.
|
||||
|
||||
**Fix** — Use `WorkflowExpr.Path(...)` to bind a runtime value, or a `const` on your own class for compile-time constants.
|
||||
|
||||
### WF010 — Reachable helper violation {#wf010}
|
||||
|
||||
Emitted when a helper method reachable from a workflow's `Spec` (in a project reference) itself contains a WF001–WF006 violation. The primary location points at the call site inside the workflow; the additional location points at the offending line inside the helper.
|
||||
|
||||
If the helper is in the **same compilation** as the workflow, the underlying rule (e.g., WF001) fires directly at the helper's line instead. WF010 is specifically for cross-project indirection where the analyzer cannot emit diagnostics into the referenced project's source.
|
||||
|
||||
**Fix** — Fix the helper. One fix clears the diagnostic at every call site.
|
||||
|
||||
## Non-goals for v1
|
||||
|
||||
- **No source generator.** The analyzer only validates; the runtime `WorkflowCanonicalDefinitionCompiler` remains the single source of truth for canonical JSON emission. Adding a generator would create two sources of truth that will drift.
|
||||
- **No severity configuration.** All rules are `Error`. If a workflow would fail canonicalization at runtime, building it should fail too.
|
||||
- **No escape hatch attribute.** If a real case forces one (e.g., a vetted BCL helper we want to whitelist) it can be added later. Shipping it from day one invites misuse.
|
||||
|
||||
## When the analyzer will NOT catch something
|
||||
|
||||
- **Reflection** (`Type.GetMethod(...)`, `Activator.CreateInstance(...)`): flagged as WF003 (invocation of metadata-only `System.Reflection`).
|
||||
- **Runtime-configured workflows built by a service/DI**: the `Spec` property's initializer is the entry point. If the workflow class has a constructor that fetches data before building `Spec`, the analyzer only walks the `Spec` getter — the constructor runs at runtime and is outside scope. Keep workflow definitions pure (no constructor state dependency).
|
||||
|
||||
## Quick reference
|
||||
|
||||
| ID | Short name | Severity |
|
||||
|----|------------|----------|
|
||||
| WF001 | Imperative control flow | Error |
|
||||
| WF002 | Async/await | Error |
|
||||
| WF003 | Call into non-trusted assembly | Error |
|
||||
| WF004 | `new` on non-trusted type | Error |
|
||||
| WF005 | C# `?:` / `??` / `?.` | Error |
|
||||
| WF006 | Non-trusted field/property read | Error |
|
||||
| WF010 | Reachable helper violation (cross-project) | Error |
|
||||
@@ -0,0 +1,176 @@
|
||||
using FluentAssertions;
|
||||
using NUnit.Framework;
|
||||
|
||||
namespace StellaOps.Workflow.Analyzer.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Regression tests that exercise the patterns used by the real Serdica workflow corpus.
|
||||
/// A failure here means the analyzer has regressed and would reject canonical code that
|
||||
/// was previously accepted.
|
||||
/// </summary>
|
||||
[TestFixture]
|
||||
public class GoldenWorkflowShapeTests
|
||||
{
|
||||
[Test]
|
||||
public async Task RealShape_LegacyRabbitAddressFieldsPlusWhenExpressionPlusCall_IsClean()
|
||||
{
|
||||
const string source = """
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using StellaOps.Workflow.Abstractions;
|
||||
using StellaOps.Workflow.Contracts;
|
||||
|
||||
public sealed class VehicleClaimRequest { public string? SrPolicyId { get; set; } }
|
||||
|
||||
public sealed class VehicleClaim : IDeclarativeWorkflow<VehicleClaimRequest>
|
||||
{
|
||||
private const string EligibilityTaskName = "Eligibility Check";
|
||||
private const string PaymentTaskName = "Payment";
|
||||
private static readonly LegacyRabbitAddress ClaimsQueryAddress = new("pas_claims_cclaims_get");
|
||||
private static readonly LegacyRabbitAddress ConvertToClaimAddress = new("pas_claimprocessing_setclaimsubstatus");
|
||||
|
||||
public string WorkflowName => "VehicleClaim";
|
||||
public string WorkflowVersion => "1.0.0";
|
||||
public string DisplayName => "Vehicle Claim";
|
||||
public IReadOnlyCollection<string> WorkflowRoles => new string[0];
|
||||
public IReadOnlyCollection<WorkflowTaskDescriptor> Tasks => new WorkflowTaskDescriptor[0];
|
||||
|
||||
public WorkflowSpec<VehicleClaimRequest> Spec { get; } = WorkflowSpec.For<VehicleClaimRequest>()
|
||||
.InitializeState(WorkflowExpr.Obj(
|
||||
WorkflowExpr.Prop("lob", WorkflowExpr.String("MOTOR")),
|
||||
WorkflowExpr.Prop("productCode", WorkflowExpr.Func(
|
||||
"coalesce",
|
||||
WorkflowExpr.Path("start.productCode"),
|
||||
WorkflowExpr.String("4704"))),
|
||||
WorkflowExpr.Prop("srPolicyId", WorkflowExpr.Path("start.srPolicyId")),
|
||||
WorkflowExpr.Prop("notifiedAt", WorkflowExpr.Null())))
|
||||
.StartWith(flow => flow
|
||||
.Call(
|
||||
"Query Claims",
|
||||
ClaimsQueryAddress,
|
||||
WorkflowExpr.Obj(
|
||||
WorkflowExpr.Prop("SrPolicyId", WorkflowExpr.Path("state.srPolicyId"))),
|
||||
resultKey: "claimsList")
|
||||
.WhenExpression(
|
||||
"Has prior claim?",
|
||||
WorkflowExpr.Gt(
|
||||
WorkflowExpr.Func("length", WorkflowExpr.Path("result.claimsList")),
|
||||
WorkflowExpr.Number(0)),
|
||||
whenTrue => whenTrue.Call(
|
||||
"Convert To Claim",
|
||||
ConvertToClaimAddress,
|
||||
WorkflowExpr.Obj(
|
||||
WorkflowExpr.Prop("SrPolicyId", WorkflowExpr.Path("state.srPolicyId")))),
|
||||
whenElse => whenElse.ActivateTask(EligibilityTaskName)))
|
||||
.AddTask(WorkflowHumanTask.For<VehicleClaimRequest>(
|
||||
EligibilityTaskName, "VehicleClaimEligibility", "default")
|
||||
.WithRoles("DBA", "APR")
|
||||
.WithPayload(WorkflowExpr.Obj(
|
||||
WorkflowExpr.Prop("srPolicyId", WorkflowExpr.Path("state.srPolicyId"))))
|
||||
.OnComplete(flow => flow
|
||||
.Set("eligibilityAnswer", WorkflowExpr.Path("payload.answer"))
|
||||
.WhenExpression(
|
||||
"Approved?",
|
||||
WorkflowExpr.Eq(
|
||||
WorkflowExpr.Path("state.eligibilityAnswer"),
|
||||
WorkflowExpr.String("approve")),
|
||||
whenTrue => whenTrue.ActivateTask(PaymentTaskName),
|
||||
whenElse => whenElse.Complete())))
|
||||
.AddTask(WorkflowHumanTask.For<VehicleClaimRequest>(
|
||||
PaymentTaskName, "VehicleClaimPayment", "default")
|
||||
.WithRoles("PAY")
|
||||
.OnComplete(flow => flow.Complete()))
|
||||
.Build();
|
||||
}
|
||||
""";
|
||||
|
||||
var diagnostics = await AnalyzerTestHarness.RunAsync(source);
|
||||
var workflowDiagnostics = diagnostics.Where(d => d.Id.StartsWith("WF")).ToArray();
|
||||
workflowDiagnostics.Should().BeEmpty(
|
||||
"a workflow built entirely from fluent builder calls + WorkflowExpr + trusted address " +
|
||||
"literals + SetBusinessReference + WhenExpression + OnComplete must not produce any WF* diagnostic");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task RealShape_SetBusinessReferenceWithNestedDeclaration_IsClean()
|
||||
{
|
||||
const string source = """
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using StellaOps.Workflow.Abstractions;
|
||||
using StellaOps.Workflow.Contracts;
|
||||
|
||||
public sealed class AssistantAddAnnexRequest
|
||||
{
|
||||
public string? SrPolicyId { get; set; }
|
||||
public string? SrAnnexId { get; set; }
|
||||
}
|
||||
|
||||
public sealed class AssistantAddAnnex : IDeclarativeWorkflow<AssistantAddAnnexRequest>
|
||||
{
|
||||
public string WorkflowName => "AssistantAddAnnex";
|
||||
public string WorkflowVersion => "1.0.0";
|
||||
public string DisplayName => "Add Annex";
|
||||
public IReadOnlyCollection<string> WorkflowRoles => new string[0];
|
||||
public IReadOnlyCollection<WorkflowTaskDescriptor> Tasks => new WorkflowTaskDescriptor[0];
|
||||
|
||||
public WorkflowSpec<AssistantAddAnnexRequest> Spec { get; } = WorkflowSpec.For<AssistantAddAnnexRequest>()
|
||||
.InitializeState(WorkflowExpr.Obj(
|
||||
WorkflowExpr.Prop("srPolicyId", WorkflowExpr.Path("start.srPolicyId")),
|
||||
WorkflowExpr.Prop("srAnnexId", WorkflowExpr.Path("start.srAnnexId"))))
|
||||
.StartWith(flow => flow
|
||||
.SetBusinessReference(new WorkflowBusinessReferenceDeclaration
|
||||
{
|
||||
KeyExpression = WorkflowExpr.Path("start.srPolicyId"),
|
||||
Parts = new WorkflowNamedExpressionDefinition[]
|
||||
{
|
||||
new WorkflowNamedExpressionDefinition { Name = "policyId", Expression = WorkflowExpr.Path("start.srPolicyId") },
|
||||
new WorkflowNamedExpressionDefinition { Name = "annexId", Expression = WorkflowExpr.Path("start.srAnnexId") },
|
||||
},
|
||||
})
|
||||
.Complete())
|
||||
.Build();
|
||||
}
|
||||
""";
|
||||
|
||||
var diagnostics = await AnalyzerTestHarness.RunAsync(source);
|
||||
var workflowDiagnostics = diagnostics.Where(d => d.Id.StartsWith("WF")).ToArray();
|
||||
workflowDiagnostics.Should().BeEmpty(
|
||||
"SetBusinessReference(new WorkflowBusinessReferenceDeclaration { ... }) is the canonical pattern " +
|
||||
"for attaching business keys; all types are in the trusted assembly prefix");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task RealShape_CustomFunctionCalls_CustomBulstradFunction_IsClean()
|
||||
{
|
||||
const string source = """
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using StellaOps.Workflow.Abstractions;
|
||||
using StellaOps.Workflow.Contracts;
|
||||
|
||||
public sealed class CustomerChangeRequest { public string? Customer { get; set; } }
|
||||
|
||||
public sealed class CustomerChange : IDeclarativeWorkflow<CustomerChangeRequest>
|
||||
{
|
||||
public string WorkflowName => "CustomerChange";
|
||||
public string WorkflowVersion => "1.0.0";
|
||||
public string DisplayName => "Customer Change";
|
||||
public IReadOnlyCollection<string> WorkflowRoles => new string[0];
|
||||
public IReadOnlyCollection<WorkflowTaskDescriptor> Tasks => new WorkflowTaskDescriptor[0];
|
||||
|
||||
public WorkflowSpec<CustomerChangeRequest> Spec { get; } = WorkflowSpec.For<CustomerChangeRequest>()
|
||||
.InitializeState(WorkflowExpr.Obj(
|
||||
WorkflowExpr.Prop("customer", WorkflowExpr.Func(
|
||||
"bulstrad.normalizeCustomer",
|
||||
WorkflowExpr.Path("start.customer")))))
|
||||
.Build();
|
||||
}
|
||||
""";
|
||||
|
||||
var diagnostics = await AnalyzerTestHarness.RunAsync(source);
|
||||
var workflowDiagnostics = diagnostics.Where(d => d.Id.StartsWith("WF")).ToArray();
|
||||
workflowDiagnostics.Should().BeEmpty(
|
||||
"custom WorkflowExpr.Func(\"bulstrad.*\", ...) dispatches at runtime; the function name is a string literal");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user