Repository navigation
Return never from sealed ObjectType::tryRemove() when a removed union member is a supertype, and from TemplateType::tryRemove() when the bound is emptied - #6802
Open
phpstan-bot wants to merge 1 commit into
Conversation
…nion member is a supertype, and from `TemplateType::tryRemove()` when the bound is emptied - ObjectType::tryRemove(): the sealed-hierarchy branch that removes a whole union at once only collected members `$this` is a supertype of and subtracted them, so removing `BaseEx|OtherEx` from a sealed `BaseEx` produced `BaseEx~BaseEx` instead of `never` (unlike `TypeCombinator::remove()` with a single type). A multi-catch `catch (BaseEx | OtherEx $e)` therefore did not catch a `BaseEx` throw point. Return `never` when a member is a supertype of `$this`. Enums take the same path (`SomeEnum|stdClass`). - TemplateTypeTrait::tryRemove(): removing the same union from `T of SomeEnum` now empties the bound, and rebuilding the template with a `never` bound turned it into `T of mixed`. Return `never` instead: T is a subset of its bound. - Both changes are mirrored in the turbo extension (turbo-ext/src/ObjectType.cpp, turbo-ext/src/TypeTraits.cpp). - Probed and found correct: union `fromType`s (`SomeEnum|int`), `StaticType`, a sealed type with a subtracted case, multi-catch in methods, catching a parent class or every allowed subtype, and narrowing with `instanceof ... || instanceof ...`, which removes members one at a time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Since 2.3.0, a multi-catch such as
catch (BaseEx | OtherEx $e)stopped catchingBaseExthrow points whenBaseExis@phpstan-sealed. The result was falsethrows.void/missingType.checkedExceptionerrors.TypeCombinator::remove(BaseEx, BaseEx|OtherEx)returnedBaseEx~BaseExinstead ofnever. Enums had the same problem:remove(SomeEnum, SomeEnum|stdClass).Changes
src/Type/ObjectType.php(tryRemove(), sealed-hierarchy branch): returnneverwhen a member of the removed union is a supertype of$this.src/Type/Generic/TemplateTypeTrait.php(tryRemove()): returnneverwhen removing the type from the bound leavesnever. Without this,T of SomeEnumminusSomeEnum|stdClasswas rebuilt asT of mixed. The template case reaches the same all-at-once path throughTypeCombinator::canRemoveUnionAtOnce().turbo-ext/src/ObjectType.cpp,turbo-ext/src/TypeTraits.cpp.fromTypes (SomeEnum|int),StaticType, a sealed type with a subtracted case, multi-catch inside methods, catching a parent class or every allowed subtype, andinstanceof ... || instanceof ...narrowing (which removes members one at a time).Root cause
TypeCombinator::doRemove()removes a whole union from a sealed hierarchy in onetryRemove()call instead of member by member (introduced in #6443). The member-by-member path checks first whether the removed type is a supertype of the source type and returnsneverif so. The newObjectType::tryRemove()branch skipped that check. It only subtracted the members that are subtypes of$this, and subtracting a type from itself producesX~X, notnever. Template types bounded by a sealed class or enum delegate to the bound, so they took the same path. They also turned an emptied bound intomixed.Test
tests/PHPStan/Rules/Exceptions/data/bug-15392.phpwithThrowsVoidFunctionWithExplicitThrowPointRuleTest::testBug15392: the reproducer from the issue, with the catch union in both orders.TypeCombinatorTest::dataRemove: removingSomeEnum|stdClassandstdClass|SomeEnumfromSomeEnumgivesnever, and so does removing it fromT of SomeEnum.stdClass|SomeEnumInterfaceis kept as a guard for the interface (supertype) case.Fixes phpstan/phpstan#15392
🤖 Generated with Claude Code