Fix compose() boundary detection under covariant Iso/Lens templates#30
Merged
Conversation
After making Iso/Lens templates covariant (#9), Psalm no longer flagged compose() calls where adjacent isos/lenses don't line up (e.g. compose(Iso<A,B>, Iso<C,C>, Iso<C,D>)) because covariant template inference silently widens the shared boundary to a union. Add a FunctionReturnTypeProvider hook on the existing ComposeProvider classes that walks adjacent args, extracts their TGenericObject template params, and emits InvalidArgument when arg[i]'s right template and arg[i+1]'s left template are not mutually contained (i.e. not equivalent up to subtyping). Fixes #10
4 tasks
veewee
added a commit
that referenced
this pull request
May 26, 2026
Ports Iso and Lens from the two-template form (Iso<S, A> / Lens<S, A>) to the four-template profunctor "STAB" form (Iso<S, T, A, B> / Lens<S, T, A, B>), with a deliberate variance split across the layers: - Concrete Iso / Lens classes are invariant on all four slots. This is where compose boundary detection lives: standard Psalm template inference catches mismatches via invariant unification, no plugin hack needed. The AdjacentTemplateValidator workaround from #30 is deleted. - IsoInterface / LensInterface are @template-covariant on all four slots. This is where storage flexibility lives: Iso<Person, Person, string, string> fits in IsoInterface<mixed, mixed, mixed, mixed> slots, so consumers can keep heterogeneous optics in a single collection without losing the leaf types. Same trade as C#'s IEnumerable<out T>. - Type-changing API exposed: Lens::set(S, B): T, Iso::from(B): T, Lens::update(S, callable(A): B): T. Type-changing optics like Lens<Person, AnonymizedPerson, string, HashedName> are now expressible. - DynamicFunctionStorage compose providers (Iso + Lens) updated to emit 2N+2 templates with STAB pairing. - SA tests rewritten for STAB; added compose_type_changing.php files exercising S != T and A != B end-to-end. - Unit tests gain type-changing set/from tests using lightweight in-test fixtures. Runtime assertions unchanged. - optional() typed as Lens<S|null, T|null, A|null, B|null> to match actual runtime semantics. - properties() InvalidReturnType/InvalidReturnStatement suppress replaced with an inline @var A cast on the return value. New docs/stab-optics.md walks a reader unfamiliar with optics through the four template slots with concrete Person / AnonymizedPerson / HashedName examples, a cheat sheet, and a "Footnote: a note on variance" section explaining the concrete-vs-interface split. Existing lens.md and isomorphisms.md cross-link; README pointer added.
veewee
added a commit
that referenced
this pull request
May 26, 2026
Ports Iso and Lens from the two-template form (Iso<S, A> / Lens<S, A>) to the four-template profunctor "STAB" form (Iso<S, T, A, B> / Lens<S, T, A, B>), with a deliberate variance split across the layers: - Concrete Iso / Lens classes are invariant on all four slots. This is where compose boundary detection lives: standard Psalm template inference catches mismatches via invariant unification, no plugin hack needed. The AdjacentTemplateValidator workaround from #30 is deleted. - IsoInterface / LensInterface are @template-covariant on all four slots. This is where storage flexibility lives: Iso<Person, Person, string, string> fits in IsoInterface<mixed, mixed, mixed, mixed> slots, so consumers can keep heterogeneous optics in a single collection without losing the leaf types. Same trade as C#'s IEnumerable<out T>. - Type-changing API exposed: Lens::set(S, B): T, Iso::from(B): T, Lens::update(S, callable(A): B): T. Type-changing optics like Lens<Person, AnonymizedPerson, string, HashedName> are now expressible. - DynamicFunctionStorage compose providers (Iso + Lens) updated to emit 2N+2 templates with STAB pairing. - SA tests rewritten for STAB; added compose_type_changing.php files exercising S != T and A != B end-to-end. - Unit tests gain type-changing set/from tests using lightweight in-test fixtures. Runtime assertions unchanged. - optional() typed as Lens<S|null, T|null, A|null, B|null> to match actual runtime semantics. - properties() InvalidReturnType/InvalidReturnStatement suppress replaced with an inline @var A cast on the return value. New docs/stab-optics.md walks a reader unfamiliar with optics through the four template slots with concrete Person / AnonymizedPerson / HashedName examples, a cheat sheet, and a "Footnote: a note on variance" section explaining the concrete-vs-interface split. Existing lens.md and isomorphisms.md cross-link; README pointer added.
veewee
added a commit
that referenced
this pull request
May 27, 2026
Ports Iso and Lens from the two-template form (Iso<S, A> / Lens<S, A>) to the four-template profunctor "STAB" form (Iso<S, T, A, B> / Lens<S, T, A, B>), with a deliberate variance split across the layers: - Concrete Iso / Lens classes are invariant on all four slots. This is where compose boundary detection lives: standard Psalm template inference catches mismatches via invariant unification, no plugin hack needed. The AdjacentTemplateValidator workaround from #30 is deleted. - IsoInterface / LensInterface are @template-covariant on all four slots. This is where storage flexibility lives: Iso<Person, Person, string, string> fits in IsoInterface<mixed, mixed, mixed, mixed> slots, so consumers can keep heterogeneous optics in a single collection without losing the leaf types. Same trade as C#'s IEnumerable<out T>. - Type-changing API exposed: Lens::set(S, B): T, Iso::from(B): T, Lens::update(S, callable(A): B): T. Type-changing optics like Lens<Person, AnonymizedPerson, string, HashedName> are now expressible. - DynamicFunctionStorage compose providers (Iso + Lens) updated to emit 2N+2 templates with STAB pairing. - SA tests rewritten for STAB; added compose_type_changing.php files exercising S != T and A != B end-to-end. - Unit tests gain type-changing set/from tests using lightweight in-test fixtures. Runtime assertions unchanged. - optional() typed as Lens<S|null, T|null, A|null, B|null> to match actual runtime semantics. - properties() InvalidReturnType/InvalidReturnStatement suppress replaced with an inline @var A cast on the return value. New docs/stab-optics.md walks a reader unfamiliar with optics through the four template slots with concrete Person / AnonymizedPerson / HashedName examples, a cheat sheet, and a "Footnote: a note on variance" section explaining the concrete-vs-interface split. Existing lens.md and isomorphisms.md cross-link; README pointer added.
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
1 participant
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
FunctionReturnTypeProviderhook on bothIso\Provider\ComposeProviderandLens\Provider\ComposeProviderthat walks adjacentcompose()args and emitsInvalidArgumentwhen arg[i]'s right template is not mutually-contained with arg[i+1]'s left template.src/Psalm/Compose/AdjacentTemplateValidator.php.@psalm-suppress InvalidArgumentmarkers intests/static-analyzer/{Iso,Lens}/compose.php.Why
After #9 made the
Iso/Lenstemplates covariant, Psalm's standard template inference silently widened the shared boundary type between adjacent compose args (e.g. acceptingcompose(Iso<A,B>, Iso<C,C>, Iso<C,D>)by inferring the boundary asB|C), so the existing SA tests stopped catching the broken case. Covariance is intentionally kept (php-soap/encoding depends on storingIso<int,string>inarray<string, Iso<mixed,string>>).Test plan
vendor/bin/psalm --no-cacheis green (compose() suppressions are now used)InvalidArgumenton both Iso and Lens broken casesvendor/bin/phpunit --no-coveragepassesFixes #10
Follow-up (not in this PR)
Per discussion, a future major could port Iso/Lens to profunctor form
Iso<S,T,A,B>with explicit per-slot variance — that would let standard inference handle compose boundary checks without this plugin hook. Tracking separately so the php-soap/encoding migration can be coordinated.