Docs: document SafeDI cycle mechanics, erasure matrix, and selected init#270
Merged
Docs: document SafeDI cycle mechanics, erasure matrix, and selected init#270
Conversation
Capture convention/mechanism knowledge that wasn't load-bearing in the code but tripped up recent work: **CLAUDE.md additions** - Cycle mechanisms table (`isPropertyCycle`, `cycleEdges`, `throwIfInvalidCycle`, `validateMockRootScopeForCycles`) naming scope and role of each so future work doesn't conflate them. - `erasedToConcreteExistential` × property-type matrix under Common Pitfalls. The flag is orthogonal to `Property.PropertyType.isConstant`; reasoning about cycle reachability has to check the property type. - Two-layer mock return type note (inner `builderClosureType` vs outer `mock()` return) with PR refs. - "Selected construction initializer" named + pointed at the new `canonicalConstructionInitializer` helper. - Testing Philosophy: "structurally unreachable" claims need a failing test, not a structural argument. Past session burned a round trip on a wrong argument. **Code changes** - Add `InstantiableVisitor.canonicalConstructionInitializer` and route the two existing `initializers.first(where: isValid(forFulfilling:))` callers through it. Removes drift risk — the macro used to recompute the selector separately from the visitor. - Add /// comments on `MockParameterNode.isPropertyCycle` and `MockContext.cycleEdges` cross-referencing each other so the per-root-vs-global distinction is readable from the definitions. - Expand `throwIfInvalidCycle` docstring: explicit rejection classes, explicit acceptance condition, pointer to the erasure matrix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #270 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 41 41
Lines 6796 6796
=========================================
Hits 6796 6796
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Capture convention/mechanism knowledge that wasn't load-bearing in the code but has tripped up recent work. All additions are docs + one small helper extraction — no behavior change.
What's documented
Cycle mechanisms table (CLAUDE.md). Four related things —
isPropertyCycle,cycleEdges,throwIfInvalidCycle,validateMockRootScopeForCycles— each doing a different job with overlapping names. Table names them, says where each is set, and what it does. Also explicit: ifthrowIfInvalidCycleaccepts a cycle, every hop is non-constant.erasedToConcreteExistential× property-type matrix (CLAUDE.md Common Pitfalls). The flag doesn't imply a constant property type — it's orthogonal. Four rows cover all combinations and whether each can appear in an accepted cycle. This mistake cost a round trip in #259 when defensive branches were removed on a wrong "erased = constant" assumption.Two-layer mock return type (CLAUDE.md Mock generation specifics). Inner closure type (
builderClosureType, #251) vs outermock()return type (#266) are distinct concerns when acustomMockreturns a type other thanSelf. Both layers have to agree."Selected construction initializer" (CLAUDE.md). SafeDI picks a single canonical initializer per type. Validation that only matters for the path SafeDI uses (e.g.,
Self.*-in-default-expression check) should gate on this, not walk every initializer. Was #268's P1 bug class."Structurally unreachable" claims need a failing test (CLAUDE.md Testing Philosophy). Codifies the discipline we're already trying to hold — structural arguments are easy to get wrong.
Inline /// comments:
MockParameterNode.isPropertyCyclecross-referencescycleEdges(per-root syntactic vs global feedback arc set).MockContext.cycleEdgescross-referencesisPropertyCycle(opposite direction).throwIfInvalidCycledocstring expanded with explicit rejection classes and acceptance condition.Code change
Add
InstantiableVisitor.canonicalConstructionInitializercomputed property. Route the two existinginitializers.first(where: { $0.isValid(forFulfilling: dependencies) })call sites through it:InstantiableVisitor.swift:377(buildingInstantiable.initializer)InstantiableMacro.swift:259(thehasMemberwiseInitializerForInjectablePropertiesguard)Removes drift risk — previously the macro recomputed the selector separately.
Test plan
swift test --traits sourceBuild— 885 tests pass./CLI/lint.sh🤖 Generated with Claude Code