Tests: typecheck SafeDITool-generated code against its inputs#256
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96a9ffa120
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #256 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 41 41
Lines 6515 6515
=========================================
Hits 6515 6515 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8aed183888
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 709e849a27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc62861318
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ae5d004c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af97dc3591
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb78ea0051
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b6b64a494
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7a73e895f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9307cca772
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2444abac7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29282acae2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47cb8c80dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deca919db6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fefd28b450
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40e41fde8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 215aacf224
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48cce11bff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d4c12a517
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
dfed
force-pushed
the
claude/compile-verify-codegen-tests
branch
from
3d4c12a to
8970cec
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8970cec020
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
dfed
force-pushed
the
claude/compile-verify-codegen-tests
branch
from
8970cec to
5ac9720
Compare
Self-review on #256 flagged that these two tests' opt-out comments described a cross-module access issue, but the verifier compiles everything in one synthetic module. The actual failure is Swift's same-module rule that a `public init`'s signature cannot reference an `internal` type. Updated comment to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4d53c0146
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61f7ec79ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @Instantiable(generateMock: true, customMockName: "customMock") | ||
| public struct Child: Instantiable { |
There was a problem hiding this comment.
This test now declares customMockName: "customMock" and implements customMock(), but the expected generated output still asserts a Child.mock call and compile verification is skipped, so the case can pass even when the generated code references a nonexistent method. In this state the test no longer validates the custom-mock-name path it is intended to cover and can miss regressions in custom mock routing for error scenarios.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Stale — that test (mock_referencesExpectedMockSignature_whenCustomMockMethodMissingDependencyParameters) was removed from #256 in commit e49f2ad "remove test testing behavior that is unreachable". The macro-rejected Child shape it was pinning can't occur in practice. Current 256 doesn't have this test or any Child.mock assertion in the error-tests file.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 482a1f32b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 305f6bd446
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Runs `swiftc -typecheck` on test inputs plus generated output to catch codegen regressions that were previously only exposed by integration tests. Fixtures that cannot be typechecked in isolation (cross-module placeholders, platform framework dependencies on Linux, etc.) opt out via `skipCompileVerification: true` with an in-place comment explaining the reason. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Self-review on #256 flagged that these two tests' opt-out comments described a cross-module access issue, but the verifier compiles everything in one synthetic module. The actual failure is Swift's same-module rule that a `public init`'s signature cannot reference an `internal` type. Updated comment to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dfed
force-pushed
the
claude/compile-verify-codegen-tests
branch
from
305f6bd to
e49f2ad
Compare
`generateReturnArgumentList` returns a placeholder comment when neither the type's initializer nor its mockInitializer can be resolved — a defensive path for fixtures that slip past the macro's validation. Add a test that exercises it so the path is covered end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ification The fixture inherited from #267 had `@Forwarded let callback` on an `@Instantiable(isRoot: true)` type. The macro rejects that combination (an isRoot type has no parent to forward from), which the compile verifier wired up in this PR now catches. Wrap Root in an App-as-root that instantiates Root via `Instantiator<Root>` and supplies the forwarded closure at builder- call time. The test's Child assertion — that `@Received` closure parameters carry `@escaping` on Child's generated `mock()` — is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex P1 on #256 (r3107957694): when a back-edge made an `erasedToConcreteExistential` child unreachable, the generator set `optionalBuilderPath = nil` while `builderExpression` was assigned a non-optional default builder. Downstream emission still tried to `if let safeDIBuilder = \(overridePath)` (constant-wrapper path) or `\(builderExpression)?(\(argumentList)) ?? …` (leaf path) against that non-optional value — invalid Swift. Split the erased path into two cases: - Override unreachable (`!thisOverrideReachable`): emit a direct construction — `\(wrapperType)(\(node.defaultBuilderCall(arguments: arguments)))` — with no `if let` / optional-chain dance. - Override reachable but `optionalBuilderPath` is nil (sendable context — extracted local IS optional): keep the existing optional-binding behavior. Fixes the bug for cycles whose pruned edge targets an erased child. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex P1 on #256 (stale) / re-filed on #259: the closure literal was marked `@Sendable` only when `typeDescription.propertyType.isSendable` — i.e. when the node itself was a `SendableInstantiator` wrapper. This missed cases where a non-Sendable type is reached through a Sendable ancestor; the closure ends up captured inside an `@Sendable` helper func and Swift 6 rejects the non-Sendable capture. Move the decision to the caller, which knows whether the closure is about to land inside a sendable extraction / `@Sendable func`: `sendableExtractionPrefix != nil || propertyType.isSendable` now drives the attribute. The helper takes an `inSendableContext` bool. Over-annotating is also wrong — a `@Sendable` closure literal in a non-Sendable slot can fail to typecheck at the assignment site — so we don't unconditionally emit `@Sendable`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…om Self (#266) ## Summary - Fix generator to declare `static func mock(...)` with `customMock`'s return type when `customMock` returns an additional type (e.g. a protocol fulfilled via `fulfillingAdditionalTypes`) instead of the concrete type. Previously the method signature returned the concrete type but its body called `customMock(...)` returning the additional type, producing a type error. - Remove `skipCompileVerification: true` from `mock_extensionUserMockReturningAdditionalTypeUsedForProtocolProperty` and `mock_extensionUserMockReturningAdditionalTypeNotUsedForConcreteProperty`, and add explicit expectations for the extension's own mock file. - Document the new behavior in the `customMockName` section of `Documentation/Manual.md`. ## Why FIXMEs at `SafeDIToolMockGenerationCustomMockTests.swift:1480` and `:1565` — both the same root cause. With compile verification now running on every test (#256), these were the last extension-customMock tests still opting out for a real generator bug. ## Fix detail `mockMethodReturnType` in `generateMockRootCode` now picks `mockReturnType.asSource` when the custom mock's declared return type is NOT compatible with the concrete type (i.e. it's not `Self` / the concrete type / the property type). In every other case it stays on the concrete type name, so existing non-protocol-returning customMock tests are unchanged. The macro's `mockMethodIncorrectReturnType` check rejects this shape for concrete (struct) types, so the new code path is only reachable for extension-based Instantiables. ## Self-review pass - Pinned the extension's own mock file on the sibling `NotUsedForConcreteProperty` test so silent drift can't go unnoticed (`083fc16`). - Added a Manual.md paragraph under `customMockName` explaining when the generated `mock()` returns an additional type (`f760b82`). ## Stacked on #256 Base: `claude/compile-verify-codegen-tests`. Three commits on top: `6a77c78`, `083fc16`, `f760b82`. ## Test plan - [x] `swift test --traits sourceBuild` — 841 tests pass - [x] `./CLI/lint.sh` 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Summary - The verifier ran `swiftc -typecheck` twice per test: once on inputs alone, then again on inputs + generated output. The first pass existed purely to disambiguate failure attribution (fixture vs. generated code). - Swap the ordering: run the combined pass first, and re-run the inputs-alone pass only as a diagnostic fallback when the combined pass fails. Green-path tests pay one subprocess instead of two. ## Why Compile verification (added in #256) roughly doubled test-suite wall time. Most of that cost is the per-test `swiftc` startup — forking, loading the compiler, loading the `SafeDIMacros` compiler plugin, typechecking under Swift 6 strict-Sendable. The input-only preemptive pass is paid on every test even though attribution only matters when something is broken. ## Measurements Local `time swift test --traits sourceBuild` on the same 842-test suite: - Before: **19.49s** - After: **11.71s** (~40% wall-time reduction) ## Diagnostic behavior unchanged Failure messages still read "Test inputs failed to compile on their own" vs. "Generated code failed to compile alongside test inputs" depending on whether the fallback input-only run also fails. Attribution only pays the extra subprocess when something is already broken. ## Test plan - [x] `swift test --traits sourceBuild` — 842 tests pass - [x] `./CLI/lint.sh` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…arc set (#259) ## Problem Multi-hop lazy-instantiation cycles (e.g. `A → Instantiator<B> → B → Instantiator<C> → C → Instantiator<A>`) produced `SafeDIMockConfiguration` structs that referenced themselves through the cycle — Swift rejects those as infinite-size value types, so the generated mock code failed to compile. The root cause: the old generator relied on a per-tree `isPropertyCycle` flag that breaks at whichever edge the walker happened to cross last. Different roots walking the same cycle produced different shapes for the same shared config struct. ## Fix Compute cycle back-edges **once**, globally, in `DependencyTreeGenerator.computeCycleEdges` via an alphabetical-DFS feedback arc set. Thread the chosen `cycleEdges` through both: - `SafeDIMockConfiguration` struct generation (so every root emits the same shape for a given type), and - override-path navigation inside the generated `mock()` bodies (so call sites agree on which slot is pruned). With the back-edge chosen globally, every root sees the same cycle break and the shared struct stops recursing into itself. ## Secondary fixes required to make the above compile - **Callee-scope preservation for pruned defaults.** When `resolveBuilderArguments` prunes a back-edge, it used to inline raw default-expression text at the caller. It now emits a closure form (`{ Type(label: $0) }`, or `{ @sendable in … }` in sendable contexts) that captures only deps + required non-deps, so Swift's default-argument thunk fires in the callee's lexical scope — preserving `Self.*`, private members, and file-scoped helpers. - **`ErasedInstantiator` pruned-back-edge branch.** `generateInstantiatorBinding`'s erased arm emitted `if let \(builderExpression)` even when `builderExpression` was a non-optional default. Split the arm so pruned back-edges emit direct construction. - **Narrowed `@Sendable` on config-struct closure fields.** Only marks `@Sendable` on the specific override slot reached through a `SendableInstantiator` / `SendableErasedInstantiator`, not on the whole enclosing struct. ## Tests Compile verification (from #256) typechecks every fixture's generated code. New regressions: - `mock_generatedForFullyLazyInstantiationCycle` — 3-node A↔B↔C↔A, now with compile verification enabled (previously skipped). - `mock_generatedForFourNodeFullyLazyInstantiationCycle` — A→B→C→D→A, SCC handling scales past 3 nodes. - `mock_generatedForCycleWithNonCycleSibling` — A↔B plus A→C, pruning operates per-edge not per-node. - `mock_generatedForLazyCycleBeneathConstantChild` — cycle descendant under the constant-wrapper recursion path. - `mock_generatedForErasedInstantiatorBackEdgeInLazyCycle` — `ErasedInstantiator` property as the selected back-edge. - `mock_generatedForCycleWithDefaultValuedNonDependencyParameter` — labeled non-dep default on a cycle node. - `mock_generatedForCycleWithCustomMockAndLabeledDefaultOnCycleNode` — customMock + labeled default combined. - `mock_generatedForRootSelfInstantiatorCycle` — self-cycle root, pins that the config struct is still emitted. Also fixes the `AnyService` fixture in `mock_usesMockOnly_whenProductionHasNoMockAndBothFulfillSameAdditionalType` (was a concrete class two structs claimed to fulfill — switched to a protocol). ## Test plan - [x] `swift test --traits sourceBuild` — 850 tests pass - [x] `./CLI/lint.sh` - [x] Codecov 100% 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
SafeDICompileVerifier(andSafeDIBuildArtifactLocator) that invokesswiftc -typecheckon the fixture sources plus generated output. Wire it intoexecuteSafeDIToolTestso every test that round-trips through the generator also verifies the output compiles against the builtSafeDImodule andSafeDIMacrosplugin.SAFEDI_SKIP_COMPILE_CHECK=1is an escape hatch for fast local iteration.skipCompileVerification: trueand a comment explaining why. Leaves a paper trail for follow-up work.Instantiableconformance, platform-only types standing in as protocols, etc.).Self-review pass (commit
e5b91c1)skipCompileVerificationreason comment on two internal-Child tests: the verifier compiles everything in one synthetic module, so the real failure is Swift's same-module rule aboutpublic initsignatures referencinginternaltypes — not cross-module inaccessibility.Why
Before this PR, generator regressions that produced malformed-but-plausible Swift slipped through the test suite unless they tripped an integration test. The mock output strings were compared character-for-character, but nothing confirmed that the strings were valid Swift against the rest of the module.
Test plan
swift test --traits sourceBuild— 841 tests pass./CLI/lint.sh🤖 Generated with Claude Code