Skip to content

Do not let method_exists() check fall through to general logic when object type contains template types#5605

Open
phpstan-bot wants to merge 8 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-7ulxx4w
Open

Do not let method_exists() check fall through to general logic when object type contains template types#5605
phpstan-bot wants to merge 8 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-7ulxx4w

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When using method_exists() on a template type narrowed to object&T (e.g., after an is_object() check), PHPStan incorrectly reports "will always evaluate to true". This is a false positive because an arbitrary object&T does not necessarily have any given method.

Changes

  • Added an early return null in src/Rules/Comparison/ImpossibleCheckTypeHelper.php at the end of the method_exists specific handling block, when the object type contains template or late-resolvable types
  • Added regression test in tests/PHPStan/Rules/Comparison/data/bug-8217.php reproducing the exact scenario from the issue
  • Added test method testBug8217() in tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php

Root cause

The method_exists specific handling in ImpossibleCheckTypeHelper has checks for:

  1. Concrete class names (getObjectClassNames() !== []) — checks hasMethod() directly
  2. GenericClassStringType — checks on the generic type

For object&T where T is a bare template type, neither check applies (no class names, no GenericClassStringType), so the code falls through to the general type specifier logic. The general logic uses HasMethodType::isSuperTypeOf(), which calls $type->hasMethod() on the argument. For IntersectionType([ObjectWithoutClassType, TemplateMixedType]), hasMethod() returns Yes because TemplateMixedType inherits MixedType::hasMethod() which always returns Yes. This causes the general logic to conclude the check "will always evaluate to true".

The fix adds an explicit return null when the object type contains template or late-resolvable types, correctly indicating that method existence is uncertain.

Analogous cases investigated

  • property_exists: Not affected — PropertyExistsTypeSpecifyingExtension returns empty SpecifiedTypes when finding a non-native property (which MixedType provides), so the general logic has nothing to check
  • is_callable: Not affected — tested with is_callable($object) on object&T, no false positive
  • class-string<T> templates: Not affected — handled correctly by the existing GenericClassStringType block in the specific method_exists handling
  • T of object bound: Not affected — TemplateObjectWithoutClassType::hasMethod() correctly returns Maybe (via ObjectTypeTrait), not Yes

Test

Regression test reproduces the exact scenario from the issue: a function with @template T parameter, narrowed via is_object($object), then checked with method_exists($object, 'method'). The test expects no errors (previously reported a false positive).

Fixes phpstan/phpstan#8217

… object type contains template types

- When the specific `method_exists` handling in `ImpossibleCheckTypeHelper` cannot
  determine the result (no concrete class names, no GenericClassStringType), it falls
  through to the general type specifier logic
- The general logic uses `HasMethodType::isSuperTypeOf()` which calls `hasMethod()` on
  the argument type; for `object&T` where T is a TemplateMixedType, this returns Yes
  (inherited from MixedType), causing a false positive "will always evaluate to true"
- Add early `return null` when `$objectType->hasTemplateOrLateResolvableType()` is true,
  preventing the general logic from running on types where method existence is uncertain
- Verified analogous cases: `property_exists` is not affected (its type specifying
  extension returns empty SpecifiedTypes for non-native properties); `is_callable` is
  not affected; `class-string<T>` template bounds are correctly handled by the existing
  GenericClassStringType block
Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the issue seems related to TemplateMixedType, why it doesn't currently occur for MixedType ?

Demonstrates that the false positive only occurs with template types
(object&T), not with plain mixed or object. For mixed,
ObjectWithoutClassType::isSuperTypeOf(MixedType) returns Maybe (mixed
is not necessarily an object), preventing the false positive. For plain
object, ObjectWithoutClassType::hasMethod() returns Maybe (via
ObjectTypeTrait), also preventing it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Pushed. Here's the answer to the reviewer's question:

Why doesn't MixedType trigger the same false positive?

The false positive requires two conditions to align in the general type specifier logic:

  1. ObjectWithoutClassType::isSuperTypeOf($argumentType) returns Yes (the type is definitely an object)
  2. HasMethodType::isSuperTypeOf($argumentType) returns Yes (the type definitely has the method)

For plain mixed: condition 1 fails — ObjectWithoutClassType::isSuperTypeOf(MixedType) returns Maybe (mixed isn't necessarily an object). So the overall result is Maybe, no false positive.

For plain object: condition 2 fails — HasMethodType::isSuperTypeOf(ObjectWithoutClassType) calls ObjectWithoutClassType::hasMethod() which returns Maybe (via ObjectTypeTrait). So the overall result is Maybe, no false positive.

For object&T (the bug case): both conditions are Yes — the type is already known to be an object (condition 1), AND IntersectionType::hasMethod() returns Yes because TemplateMixedType inherits MixedType::hasMethod() which returns Yes, dominating the intersection (condition 2). This makes the general logic conclude the check "will always evaluate to true."

I added test cases for the mixed and object scenarios to the test data file to demonstrate they work correctly.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should/could the fix be done in the general specifier ?

…o return Maybe

A template type T represents a specific-but-unknown type. MixedType::hasMethod()
returns Yes (mixed encompasses all types including those with any method), but for
a template T we don't know whether the specific type has a given method.

This fixes the root cause instead of working around it in ImpossibleCheckTypeHelper.
The previous workaround checked hasTemplateOrLateResolvableType() in the
method_exists-specific handling to bail out early. With this fix, the general type
specifier logic works correctly because HasMethodType::isSuperTypeOf() now gets
Maybe (not Yes) from TemplateMixedType, so it no longer concludes that
method_exists() on object&T "will always evaluate to true."

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Done. The fix now addresses the reviewer's suggestion: instead of working around the issue in ImpossibleCheckTypeHelper, the fix is at the type system level by overriding hasMethod() on TemplateMixedType to return Maybe.

Why this is the right fix: MixedType::hasMethod() returns Yes because mixed encompasses all types. But TemplateMixedType represents a specific-but-unknown type T — we can't know whether that specific type has a given method. Returning Maybe makes the general type specifier logic in ImpossibleCheckTypeHelper work correctly without needing a special case: HasMethodType::isSuperTypeOf() gets Maybe from the template type, so the intersection object & T no longer claims to definitely have every method.

All 11,995 tests pass and make phpstan reports no errors.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List other possible fixes cause this one is not ok.

MixedType:: and TemplateMixedType methods shouldn't have different result.

…lper

Revert the hasMethod() override on TemplateMixedType so that it
inherits MixedType::hasMethod() (returning Yes) as before — the two
should not diverge.

Instead, prevent the method_exists specific handling in
ImpossibleCheckTypeHelper from falling through to the general type
specifier logic when the object type contains template or
late-resolvable types. The general logic incorrectly concludes
"always true" because MixedType::hasMethod() returns Yes, which
dominates the IntersectionType result via lazyMaxMin.

Alternatives considered:
- HasMethodType::isSuperTypeOf(): could check for template types,
  but would incorrectly return Maybe for bounded templates like
  T of SomeClass where the method genuinely exists on the bound.
- IntersectionType::hasMethod(): changing from lazyMaxMin to lazyMin
  would be a large behavioral change affecting all intersections.
- General result processing: capping results at Maybe for template
  types is equivalent to this fix but less targeted.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Pushed. The fix now addresses the reviewer's concern:

What changed: Reverted the hasMethod() override on TemplateMixedType so it inherits MixedType::hasMethod() (both return Yes). Instead, the fix is in ImpossibleCheckTypeHelper::findSpecifiedType() — at the end of the method_exists specific handling block, if the object type contains template or late-resolvable types, return null (uncertain) rather than falling through to the general type specifier logic.

Why the general logic gives the wrong answer for object & T: IntersectionType::hasMethod() uses lazyMaxMin, which short-circuits on Yes. Since TemplateMixedType inherits MixedType::hasMethod() returning Yes, it dominates the intersection — making HasMethodType::isSuperTypeOf() conclude the method definitely exists.

Alternatives considered:

  1. HasMethodType::isSuperTypeOf(): Could check for template types, but would incorrectly return Maybe for bounded templates (T of SomeClass) where the method genuinely exists on the bound
  2. IntersectionType::hasMethod(): Changing from lazyMaxMin to lazyMin would be a large behavioral change affecting all intersections
  3. General result processing: Capping results at Maybe for template types — equivalent but less targeted

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the fix should be inside HasMethodType::isSuperTypeOf() instead.

For instance HasMethodType::isSuperTypeOf(Mixed) should be Maybe only (this would be consistent with MixedType::isSubtypeOf(HasMethod) which is maybe)

…ompoundTypes

HasMethodType::isSuperTypeOf(MixedType) was returning Yes (via
MixedType::hasMethod() which always returns Yes), inconsistent with
MixedType::isSubTypeOf(HasMethodType) which returns Maybe. This caused
false positives for method_exists() on template types like object&T,
where TemplateMixedType (inheriting MixedType::hasMethod()) dominated
IntersectionType::hasMethod() via lazyMaxMin.

The fix delegates to $type->isSubTypeOf($this) for CompoundType
instances (the standard pattern used by other accessory types like
AccessoryNonEmptyStringType). When isSubTypeOf returns No, we fall
back to hasMethod() — this prevents AccessoryTypes in intersections
(e.g. AccessoryLiteralStringType in class-string&literal-string) from
being overly strict, since their isSubTypeOf checks orthogonal
properties that can incorrectly poison the lazyMaxMin result.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

I processed this review but have nothing to report.

@staabm
Copy link
Copy Markdown
Contributor

staabm commented May 7, 2026

why don't we have the same bug for property_exists?

https://phpstan.org/r/01359ac6-3f8e-4ea2-b6f3-82b1e2de8b3f

@VincentLanglet
Copy link
Copy Markdown
Contributor

Indeed, I'll ask the bot

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we have the same bug for property_exists?
https://phpstan.org/r/01359ac6-3f8e-4ea2-b6f3-82b1e2de8b3f

… CompoundTypes

Same pattern as the HasMethodType fix: delegate to $type->isSubTypeOf($this)
for CompoundTypes so that MixedType correctly returns Maybe instead of Yes.

property_exists() doesn't currently trigger the same false positive as
method_exists() because PropertyExistsTypeSpecifyingExtension bails out
early for non-native properties (which MixedType provides). But the
underlying type inconsistency is the same: HasPropertyType::isSuperTypeOf(MixedType)
was returning Yes when it should return Maybe.

Added property_exists test cases to bug-8217 regression test to cover
the template, mixed, and object scenarios.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Pushed. Here's the answer to the reviewer's question:

Why property_exists doesn't have the same bug:

The false positive requires the general type specifier logic to conclude "always true." For property_exists, this doesn't happen because PropertyExistsTypeSpecifyingExtension bails out early at line 87-89: when it finds that the property reflection is non-native (!$propertyReflection->isNative()), it returns empty SpecifiedTypes. Since MixedType provides non-native properties, the extension returns nothing for the impossible check logic to analyze — so no false positive occurs.

For method_exists, there's no equivalent bailout — the MethodExistsTypeSpecifyingExtension always returns specified types, and the dedicated handling in ImpossibleCheckTypeHelper falls through to the general logic for object & T.

But the underlying type inconsistency was the same: HasPropertyType::isSuperTypeOf(MixedType) was returning Yes (because MixedType::hasInstanceProperty() returns Yes) when it should return Maybe (because MixedType::isSubTypeOf(HasPropertyType) returns Maybe). Applied the same CompoundType delegation fix from HasMethodType to HasPropertyType for consistency, with a unit test confirming HasPropertyType::isSuperTypeOf(MixedType) = Maybe. Added property_exists test cases to the regression test.

I also investigated HasOffsetType and HasOffsetValueType which have the same missing delegation, but applying the fix there causes side effects in type narrowing (extra hasOffset() types appearing in intersections) — those would need separate investigation.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use

if ($type instanceof CompoundType) {
			return $type->isSubTypeOf($this);
		}

to fix this issue and explains why it impacts class-string&literal-string.

…delegation

Use the standard pattern `return $type->isSubTypeOf($this)` instead of
falling back to hasMethod()/hasProperty() when isSubTypeOf() returns No.
This is consistent with how other AccessoryType classes handle CompoundTypes.

The simplified delegation changes behavior for `class-string&literal-string`:
since IntersectionType::isSubTypeOf(HasMethodType) uses lazyMaxMin over
individual members, and neither ClassStringType nor AccessoryLiteralStringType
has methods, the result becomes No instead of the previous Maybe. To prevent
this from causing false "always false" reports for method_exists() on
class-string types (where the actual class is unknown), add explicit return
null in ImpossibleCheckTypeHelper for both generic class-string types
(non-final TypeWithClassName) and plain class-string types.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Pushed. Here's the summary:

Changes made:

  1. Simplified HasMethodType::isSuperTypeOf() and HasPropertyType::isSuperTypeOf() — replaced the fallback-on-No pattern with the standard return $type->isSubTypeOf($this) that other AccessoryType classes use.

  2. Added return null guards in ImpossibleCheckTypeHelper to prevent the general type specifier logic from producing false "always false" reports on class-string types.

Why this impacts class-string&literal-string:

With the simplified delegation, HasMethodType::isSuperTypeOf(class-string&literal-string) returns No instead of the previous Maybe. This happens because IntersectionType::isSubTypeOf(HasMethodType) uses lazyMaxMin over the individual intersection members (ClassStringType, AccessoryLiteralStringType), and neither has methods — both return No from HasMethodType::isSuperTypeOf(). The previous code's fallback ($type->hasMethod() on the IntersectionType) would also return No, but wasn't reached when isSubTypeOf returned Maybe or Yes.

The dedicated method_exists handling in ImpossibleCheckTypeHelper already correctly handles generic class-string types (non-final class-string<S>) by not returning false. The fix adds explicit return null for both that case and plain class-string types, preventing fallthrough to the general logic that would now incorrectly report "always false."

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand your explanation

With the simplified delegation, HasMethodType::isSuperTypeOf(class-string&literal-string) returns No instead of the previous Maybe. This happens because IntersectionType::isSubTypeOf(HasMethodType) uses lazyMaxMin over the individual intersection members (ClassStringType, AccessoryLiteralStringType), and neither has methods — both return No from HasMethodType::isSuperTypeOf().

Because both ClassStringType::hasMethod and AccessoryLiteralStringType::hasMethod are supposed to return MAYBE

…() returns No

AccessoryLiteralStringType implements CompoundType, so
HasMethodType::isSuperTypeOf(AccessoryLiteralStringType) was delegating
to AccessoryLiteralStringType::isSubTypeOf(HasMethodType), which returns
No (because HasMethodType::isLiteralString() is No). This made
HasMethodType::isSuperTypeOf(class-string&literal-string) return No
instead of Maybe, since lazyMaxMin propagated the No from the accessory
type even though ClassStringType::hasMethod() correctly returned Maybe.

The fix: when the CompoundType delegation returns No, fall back to the
hasMethod()/hasProperty() check which gives the correct result. This
removes the need for the class-string guard in ImpossibleCheckTypeHelper.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Pushed. Here's the explanation for the reviewer:

Why the class-string guard was needed (and why it's no longer needed):

The reviewer is correct that both ClassStringType::hasMethod() and AccessoryLiteralStringType::hasMethod() return Maybe. The previous explanation was wrong about them returning No from HasMethodType::isSuperTypeOf().

The actual issue: AccessoryLiteralStringType implements CompoundType, so when HasMethodType::isSuperTypeOf(AccessoryLiteralStringType) was called, the unconditional return $type->isSubTypeOf($this) delegated to AccessoryLiteralStringType::isSubTypeOf(HasMethodType). That method checks HasMethodType::isLiteralString() which returns No (via ObjectTypeTrait), making the whole call return No — bypassing hasMethod() entirely. Then in IntersectionType::isSubTypeOf(HasMethodType), lazyMaxMin(Maybe, No) produced No because $hasNo was set.

The fix: When the CompoundType delegation returns No, fall back to the hasMethod()/hasProperty() check. Now HasMethodType::isSuperTypeOf(AccessoryLiteralStringType) correctly returns Maybe (from hasMethod()), making HasMethodType::isSuperTypeOf(class-string&literal-string) return Maybe as expected. This eliminates the need for the class-string guard in ImpossibleCheckTypeHelper.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at code like

$objectType = $scope->getType($args[0]->value);
		if ($objectType->isString()->yes()) {
			if ($objectType->isClassString()->yes()) {
				return $this->typeSpecifier->create(
					$args[0]->value,
					new IntersectionType([
						$objectType,
						new HasMethodType($methodNameType->getValue()),
					]),
					$context,
					$scope,
				);
			}

			return new SpecifiedTypes([], []);
		}

I feel like the issue is that HasMethod::isString (and similar method) return No because the ObjectTrait is used while it should return Maybe WDYT ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants