Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,28 @@ Mock generation follows this pipeline:

The production code path (`generatePropertyCode` with `.dependencyTree`) and mock path (`.mock`) share the same `ScopeGenerator` but diverge at `generatePropertyCode`. Mock fields (`mockInitializer`, `mockReturnType`, `customMockName`) are only accessed in mock code paths — never in production paths.

### Cycle mechanisms

Multiple cycle-related mechanisms coexist — keep them distinct:

| Name | Scope | Set by / when | Role |
|------|-------|---------------|------|
| `isPropertyCycle` on a scope/node | Per-root, syntactic | `Scope.createScopeGenerator` when a property label repeats in the ancestor stack | Terminates infinite descent during a single root's walk; differs between roots for the same underlying cycle. |
| `cycleEdges` (set of `CycleEdge`) | Global | `DependencyTreeGenerator.computeCycleEdges`, alphabetical-DFS feedback arc set | Chooses a single back-edge per cycle consistently across all roots — required because shared `SafeDIMockConfiguration` structs must have one shape. Consumed by struct generation AND override-path navigation in mock bodies. |
| `throwIfInvalidCycle` | Shared classifier | Called from `validatePropertiesAreFulfillable` (production) and `validateMockRootScopeForCycles` (mock) | Rejects constant + partially-lazy + received-lazy cycles. Accepts fully-lazy non-received cycles (those compile as long as pruning breaks the config-struct recursion). |
| `validateMockRootScopeForCycles` | Mock graph | Called after `@Received` promotion in `createMockRootScopeGenerator` | Walks the post-promotion scope to catch cycles that only surface once received deps are promoted to mock-root children. |

Key consequence: if `throwIfInvalidCycle` accepts a cycle, every hop in it is non-constant (Instantiator/ErasedInstantiator-typed). Any code that assumes "a cycle node reached through a constant-typed property" is reasoning about something impossible.

### Mock generation specifics

- **Share logic between production and mock paths where possible.** Validation, scope population, and other shared concerns should live in common helpers rather than being duplicated between `createTypeDescriptionToScopeMapping` and `createMockTypeDescriptionToScopeMapping`. When adding validation to one path, check whether the other path needs it too.
- **Selected construction initializer.** `InstantiableVisitor.canonicalConstructionInitializer` is the single initializer SafeDI selects for construction (first init that fulfills the type's dependencies). `Instantiable.initializer` is populated from it, and mock generation emits against its signature. Validation that only matters for the path SafeDI uses (e.g., diagnosing `Self.*` in default expressions) should gate on this — walking every entry in `visitor.initializers` false-positives on unused overloads.
- **Two-layer mock return type** (extension-based `customMockName` only). When a `customMock` method returns a type different from `Self` (permitted via `fulfillingAdditionalTypes`), two concerns have to agree:
1. **Inner closure type** (`MockParameterNode.builderClosureType`) — the type of the override closure stored in the parent's `SafeDIMockConfiguration`. Uses the property type so callers writing overrides match the receiving slot. Fixed in #251.
2. **Outer `mock()` return type** — the generated `mock()` wrapper must return whatever the user's `customMock` returns. Fixed in #266.

When touching extension-based types with `customMockName`, confirm both layers. `mockReturnTypeIsCompatible(withPropertyType: concreteInstantiable)` distinguishes genuine hand-written mocks from mockOnly-inherited ones.
- `MockParameterIdentifier` (propertyLabel + sourceType) is the key type for tracking parameters throughout mock gen
- `resolvedParameters` tracks which deps are already bound — prevents duplicate bindings across scopes
- `parameterLabelMap` maps identifiers to disambiguated parameter names
Expand Down Expand Up @@ -93,6 +112,7 @@ The `Instantiable` struct conforms to `Codable` and is serialized as JSON in `.s
- **Code generation tests should only test paths where the macro would not emit an error.** Test inputs must be valid under `@Instantiable` macro validation — every `mockOnly: true` type needs a `mock()` method (or `customMockName` method), mock method names on the same type must not collide, and production types need `init` or `instantiate()`.
- **Test titles must be observably verified.** When a test title says "prefers X over Y", the assertion must verify X is used — not just that the test doesn't crash. Add a parent type with `generateMock: true` that instantiates the type under test, and assert the full mock output shows X's method name.
- **If code can't be covered by a test with real parsed input, remove the code.** Dead branches and defensive fallbacks for structurally unreachable paths should not exist.
- **"Structurally unreachable" claims need a failing test.** Before removing defensive branches on the grounds that they can't be reached, write a test that tries to reach them with valid input. If the test can't be written, cite the specific rejection (file:line) and the input shape it rejects — a structural argument alone is not evidence. Structural arguments are easy to get wrong: e.g., `erasedToConcreteExistential: true` does NOT imply a constant property type, so "erased + cycle is rejected as constant" was wrong and cost a round trip when the branches had to be restored.
- **Test fixture file naming affects processing order.** `executeSafeDIToolTest` names fixture files by extracting the type name from `@Instantiable` in the source content. Files are processed alphabetically, so the order types appear in `resolveSafeDIFulfilledTypes` depends on these names. When testing ordering-sensitive behavior (e.g., duplicate detection), be aware that a `struct MyService` file sorts differently than an `extension MyService` file (which falls back to `"File"`).

## Common Pitfalls
Expand All @@ -105,3 +125,13 @@ The `Instantiable` struct conforms to `Codable` and is serialized as JSON in `.s
- `typeDescriptionToFulfillingInstantiableMap` is a dictionary — iterating `.values` has non-deterministic order. Any code that reduces, sorts, or builds state from `.values` must produce the same result regardless of iteration order. CI runs tests 5 times specifically to catch non-determinism.
- `resolveSafeDIFulfilledTypes` can produce multiple map entries with the same `concreteInstantiable` under different keys (e.g., a merged entry under `MyService` and an unmerged entry under `ServiceProtocol`). These entries may have different mock info. The mock scope map uses scope-reuse by `concreteInstantiable` and a deterministic sort to handle this.
- `mockInitializer` on an `Instantiable` can be inherited from a mockOnly merge — it doesn't always mean the type declared its own mock. `generateMock` is never inherited. Use `mockReturnTypeIsCompatible(withPropertyType: concreteInstantiable)` to distinguish genuine hand-written mocks from inherited ones.
- **`erasedToConcreteExistential: true` is orthogonal to the property's `PropertyType`.** The flag controls whether SafeDI wraps the concrete in the property's existential type at the call site; it does not determine whether the property itself is constant or lazy. The four combinations:

| Decorator + property | `Property.PropertyType` | `isConstant` | Can be in an accepted cycle? |
|----------------------|-------------------------|--------------|------------------------------|
| `@Instantiated let x: Concrete` | `.constant` | `true` | No — `throwIfInvalidCycle` rejects. |
| `@Instantiated let x: Instantiator<T>` | `.instantiator` | `false` | Yes. |
| `@Instantiated(erasedToConcreteExistential: true) let x: AnyProto` | `.constant` | `true` | No — `throwIfInvalidCycle` rejects. |
| `@Instantiated(erasedToConcreteExistential: true) let x: ErasedInstantiator<A, AnyP>` | `.erasedInstantiator` | `false` | Yes. |

When reasoning about whether an erased node can appear on a pruned back-edge, check `propertyType.isConstant` — don't short-circuit on the erasure flag.
25 changes: 22 additions & 3 deletions Sources/SafeDICore/Generators/DependencyTreeGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -704,9 +704,28 @@ public actor DependencyTreeGenerator {
}
}

/// Throws the appropriate cycle error based on the property types in the cycle.
/// For fully-lazy cycles (all Instantiator hops, no constant entry point), does nothing —
/// those are valid and produce compilable code.
/// Classifies a detected cycle and throws the appropriate error, or
/// returns silently if the cycle is valid. Called from both
/// `validatePropertiesAreFulfillable` (production graph) and
/// `validateMockRootScopeForCycles` (mock graph after `@Received`
/// promotion) — the two validation entry points share this classifier.
///
/// Rejected:
/// - Constant entry point in a fully-constant cycle → `.constantDependencyCycleDetected`.
/// - Constant entry point with any lazy hop → `.partiallyLazyDependencyCycleDetected`.
/// - Non-constant entry point that originated from an `@Received` → `.receivedInstantiatorDependencyCycleDetected`.
///
/// Accepted (returns without throwing):
/// - Fully-lazy cycles entered via an `Instantiator` that is NOT received.
/// The cycle compiles because each hop defers construction; pruning via
/// `cycleEdges` is what breaks the self-referencing config-struct shape
/// so the generated code typechecks.
///
/// Key implication for downstream code: any mock tree node reached
/// through a constant-typed property CANNOT be on a pruned back-edge —
/// this function would have rejected the cycle before code generation.
/// See the `erasedToConcreteExistential` matrix in CLAUDE.md's "Common
/// Pitfalls" for how this interacts with erased properties.
private static func throwIfInvalidCycle(
typesInCycle: [TypeDescription],
property: Property,
Expand Down
25 changes: 21 additions & 4 deletions Sources/SafeDICore/Generators/ScopeGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,18 @@ actor ScopeGenerator: CustomStringConvertible, Sendable {
/// Maps types with hand-written mock methods to their mock method name (e.g. "mock" or a custom name).
/// Used to provide default values for forwarded dependencies whose type has a hand-written mock.
let forwardedParameterMockDefaults: [TypeDescription: String]
/// Type-graph back-edges to skip when emitting `safeDIOverrides` paths in
/// builder code. Computed once globally so the override-path navigation
/// matches the configuration-struct shape produced by `generateConfigurationStruct`.
/// Globally-computed feedback arc set (see
/// `DependencyTreeGenerator.computeCycleEdges`). Alphabetical-DFS
/// selection makes every root walking the same graph agree on which
/// back-edges are pruned — required because shared
/// `SafeDIMockConfiguration` structs are emitted once per type and
/// must have a consistent shape regardless of which root is being
/// generated. Consumed by both struct generation and override-path
/// navigation inside `mock()` bodies.
///
/// Contrast with `MockParameterNode.isPropertyCycle`, which is a
/// per-root syntactic flag and differs between roots for the same
/// underlying cycle.
let cycleEdges: Set<CycleEdge>
}

Expand Down Expand Up @@ -1015,7 +1024,15 @@ actor ScopeGenerator: CustomStringConvertible, Sendable {
let concreteType: TypeDescription
/// Forwarded properties on this type (relevant for Instantiator edges).
let forwardedProperties: Set<Property>
/// Whether this node is part of a property cycle.
/// Set when this property's label repeats in the ancestor stack during
/// scope construction (`Scope.createScopeGenerator`). Per-root and
/// syntactic — different roots walking the same underlying cycle can
/// see it at different edges. Terminates infinite descent by zeroing
/// the cycle-closer's `propertiesToGenerate`.
///
/// For cross-root consistency (e.g., shared `SafeDIMockConfiguration`
/// struct shapes), use `cycleEdges` — the globally-computed feedback
/// arc set — instead.
let isPropertyCycle: Bool
/// Whether this node is inside a sendable scope (descendant of SendableInstantiator).
/// When `true`, the `safeDIBuilder` closure on `SafeDIMockConfiguration` is `@Sendable`.
Expand Down
13 changes: 12 additions & 1 deletion Sources/SafeDICore/Visitors/InstantiableVisitor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,17 @@ public final class InstantiableVisitor: SyntaxVisitor {
public private(set) var diagnostics = [Diagnostic]()
public private(set) var uninitializedNonOptionalPropertyNames = [String]()

/// The initializer SafeDI selects for construction — the first in source
/// order that fulfills this type's dependencies. `Instantiable.initializer`
/// is populated from this, and mock generation emits construction code
/// against this initializer's signature. Validation that only matters for
/// the path SafeDI actually uses (e.g., diagnosing `Self.*` in default
/// expressions) should gate on this initializer rather than walking
/// `initializers`, which would false-positive on unused overloads.
public var canonicalConstructionInitializer: Initializer? {
initializers.first(where: { $0.isValid(forFulfilling: dependencies) })
}

public static let macroName = "Instantiable"
public static let instantiateMethodName = "instantiate"
public static let mockMethodName = "mock"
Expand Down Expand Up @@ -374,7 +385,7 @@ public final class InstantiableVisitor: SyntaxVisitor {
Instantiable(
instantiableType: instantiableType,
isRoot: isRoot,
initializer: initializers.first(where: { $0.isValid(forFulfilling: dependencies) }),
initializer: canonicalConstructionInitializer,
additionalInstantiables: additionalInstantiables,
dependencies: dependencies,
declarationType: instantiableDeclarationType.asDeclarationType,
Expand Down
4 changes: 1 addition & 3 deletions Sources/SafeDIMacros/Macros/InstantiableMacro.swift
Original file line number Diff line number Diff line change
Expand Up @@ -260,9 +260,7 @@ public struct InstantiableMacro: MemberMacro {
)
}

let hasMemberwiseInitializerForInjectableProperties = isMockOnly || visitor
.initializers
.contains(where: { $0.isValid(forFulfilling: visitor.dependencies) })
let hasMemberwiseInitializerForInjectableProperties = isMockOnly || visitor.canonicalConstructionInitializer != nil
guard hasMemberwiseInitializerForInjectableProperties else {
func associatedError(for initializer: Initializer) -> (initializer: Initializer, syntax: InitializerDeclSyntax, error: Initializer.FixableError)? {
do {
Expand Down
Loading