refactor(generator): drop dead InterceptorTarget mirrors, un-shadow factory helpers - #15
Conversation
…actory helpers
Targeted simplifications to the auto-instrumentation source generator. Generated
output is byte-identical (generator-snapshots-ok, source-interceptor-consumer-ok;
full slnx build green at 0 warnings).
- Remove InterceptorTarget.MatcherReceiverTypePattern and MatcherContractKeys:
both were mirrored from the matcher descriptor in TryGetInvocation but never
read anywhere (ReSharper "positional property is never accessed"). Drops two
fields of per-call-site state.
- Rename the outer-class factory helpers Parameters() -> BuildParameters() and
ContractKeys() -> BuildContractKeys() so they no longer shadow the same-named
InterceptorTarget.Parameters / InterceptorMatcherDescriptor.ContractKeys data
properties ("member hides method from outer class").
- Unify EmitDirectLoggerInterceptor onto the
location.GetInterceptsLocationAttributeSyntax() extension form already used by
EmitAttributeAndSignature.
EmitInterceptsLocationAttribute (the `file sealed class InterceptsLocationAttribute`
polyfill) is intentionally KEPT, not deleted: System.Runtime.CompilerServices.
InterceptsLocationAttribute is NOT in the .NET 10 BCL (a bare reference fails
CS0234), so the generated interceptors still need the type declared in the
consumer. It is distinct from GetInterceptsLocationAttributeSyntax(), which emits
only the attribute *usage*, not its declaration.
Also carries a whitespace / `in`-modifier / switch-expression normalization pass
over the same files.
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 (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe generator now uses ChangesReadonly target passing and helper updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
…ariants guard The carried whitespace/`in`-modifier pass changed the generator's descriptor delegation calls to pass the readonly record structs by `in` (e.g. `descriptor.DurationMetric.AppendRecordDurationStatement(builder, in target)`, `EnsureTargetDeclaredByMatcher(descriptor, in target)`, `ValidateMethodShape(in descriptor)`). verify-contract-invariants.py pins these call sites as exact-string architecture anchors and stops at the first miss, so the `verify` leg failed on both runners. Updated the seven affected anchors to the `in` form. The guard's assertions — that the generator delegates emission to descriptor methods and validates matcher-declared kinds via EnsureTargetDeclaredByMatcher — are unchanged; only the incidental argument-passing token is tracked. Generated output is identical (generator-snapshots-ok), so no behavior changed. Verified: python3 tools/verify-aot-autoinstrumentation-goal.py --no-demos --skip "smoketest,webapi aot demo,otlp collector fixtures" -> aot-autoinstrumentation-goal-partial-ok (contract invariants + all legs green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary — complete and verified
Code-quality cleanup of the auto-instrumentation source generator (invoked via
/simplify). The generator's output is byte-identical — every change is internal to the generator, so the emitted interceptors are unchanged.What changed
InterceptorTargetfields —MatcherReceiverTypePatternandMatcherContractKeyswere mirrored from the matcher descriptor inTryGetInvocationbut never read anywhere (the ReSharper "positional property is never accessed" smell). Drops two fields of per-call-site state.Parameters()→BuildParameters()andContractKeys()→BuildContractKeys()so they no longer collide with the same-named data propertiesInterceptorTarget.Parameters/InterceptorMatcherDescriptor.ContractKeys("member hides method from outer class"). Factory verbs now read as factories; the data properties keep their canonical names.EmitDirectLoggerInterceptornow uses thelocation.GetInterceptsLocationAttributeSyntax()extension form already used byEmitAttributeAndSignature(identical output).in-modifier / switch-expression normalization pass over the same files.Investigated and deliberately NOT changed
The prompt asked whether
EmitInterceptsLocationAttribute(thefile sealed class InterceptsLocationAttributepolyfill) is redundant givenGetInterceptsLocationAttributeSyntax. It is not — proven empirically:InterceptsLocationAttributeis not in the .NET 10 BCL, so the generated interceptors still need the type declared in the consumer.GetInterceptsLocationAttributeSyntax()emits only the attribute usage ([InterceptsLocationAttribute(1, "data")]), not its declaration. The two are complementary; the polyfill stays.Verification
dotnet build Qyl.OpenTelemetry.AutoInstrumentation.slnx→ 0 warnings, 0 errors (TreatWarningsAsErrors + full analyzer stack; full consumer compile against the generated interceptors confirms the polyfill is correctly retained).python3 tools/verify-generator-snapshots.py→generator-snapshots-okpython3 tools/verify-source-interceptor-consumer.py→source-interceptor-consumer-okNo public API surface changed (all touched members are
private/internal).🤖 Generated with Claude Code