fix(generator): don't intercept RequestDelegate.Invoke (delegate invocation → CS9207) - #17
Conversation
…cation → CS9207)
Hand-written ASP.NET Core middleware invokes the next hop as `next(context)` /
`_next(context)`, which the semantic model resolves to RequestDelegate.Invoke — a
MethodKind.DelegateInvoke. The C# interceptors feature can only intercept ordinary
member methods, so emitting an interceptor for a delegate invocation is rejected with
CS9207 ("Cannot intercept 'next' because it is not an invocation of an ordinary member
method").
The AspNetCoreRequestDelegate matcher had no MethodKind guard, so it emitted an
un-interceptable interceptor for every middleware hop. The package's own demos never
hand-write middleware, so CI stayed green — while real consumers that do (qyl.collector's
OtlpApiKeyMiddleware / OtlpCorsMiddleware / EmbeddedDashboardMiddleware) failed to build.
Guard the matcher on MethodKind.Ordinary, as the HttpClient/Db/Azure/Elastic matchers
already do; RequestDelegate.Invoke is never Ordinary, so the broken interceptor is no
longer emitted. Request-pipeline trace coverage already comes from the ASP.NET Core
hosting instrumentation, not from wrapping each middleware hop.
Adds tools/verify-aspnetcore-middleware-delegate.py — a consumer with `next(context)`
middleware must build clean and emit no RequestDelegate interceptor while still emitting a
control HttpClient interceptor (proving the generator stayed selective, not silent) —
wired into the verify goal suite.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (2)**⚙️ CodeRabbit configuration file
Files:
⚙️ CodeRabbit configuration file
Files:
src/**/*.cs⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe source generator's ChangesDelegate Invocation Guard and Verification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
1 participant
Problem
Hand-written ASP.NET Core middleware invokes the next hop as
next(context)/_next(context). The C# semantic model resolves that toRequestDelegate.Invoke, whoseMethodKindisDelegateInvoke. The interceptors feature can only intercept ordinary member methods, so emitting an interceptor for a delegate invocation is rejected:The
AspNetCoreRequestDelegatematcher had noMethodKindguard (unlike the HttpClient/Db/Azure/Elastic matchers), so it emitted an un-interceptable interceptor for every middleware hop. This package's own demos never hand-write middleware, so CI stayed green — but every real consumer that does breaks. Surfaced inqyl.collector(OtlpApiKeyMiddleware,OtlpCorsMiddleware,EmbeddedDashboardMiddleware), and unmasked once theBuild()opt-out (#16) removed the earlier CS9153.This was pre-existing since v3.1.0 —
cde22004(the qyl-side bump to 3.1.0) introduced both CS9153 and CS9207; only CS9153 was diagnosed at the time.Fix
Guard
TryGetAspNetCoreRequestDelegateInvocationonMethodKind.Ordinary.RequestDelegate.Invokeis neverOrdinary, so the broken interceptor is no longer emitted. Request-pipeline trace coverage already comes from the ASP.NET Core hosting instrumentation, not from wrapping each middleware hop.Regression test
tools/verify-aspnetcore-middleware-delegate.py(wired into the verify goal suite): a consumer withnext(context)middleware must build clean, emit no RequestDelegate interceptor, and still emit a control HttpClient interceptor (proving the generator stayed selective, not silent). Confirmed it fails with CS9207 on the pre-fix generator and passes after.Verification
verify-aspnetcore-middleware-delegate(new)verify-build-interceptor-optout(no regression to feat(generator): opt out of WebApplicationBuilder.Build() interception #16)verify-generator-snapshots(no committed snapshot changed — surgical)verify-contract-invariants(inert matcher keeps all structural counts)qyl.collectorRelease build against a locally-packed fixed package: 0 warnings, 0 errors (both CS9153 and CS9207 gone)🤖 Generated with Claude Code