Skip to content

Do not add HasMethodType to class-string types when the method is native#5738

Open
phpstan-bot wants to merge 2 commits into
phpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-be8jrm7
Open

Do not add HasMethodType to class-string types when the method is native#5738
phpstan-bot wants to merge 2 commits into
phpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-be8jrm7

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When method_exists() was used to guard a static method call on a class-string type, PHPStan failed to report visibility errors for private and protected methods. This affected both literal class name calls (X::privateFoo()) and class-string variable calls ($row::privateFoo()).

Changes

  • Modified src/Type/Php/MethodExistsTypeSpecifyingExtension.php to use createFuncCallSpec instead of adding HasMethodType to the class-string type when all class reflections have the method natively. This preserves the original GenericClassStringType / ConstantStringType representation.
  • Added regression test in tests/PHPStan/Rules/Methods/data/bug-14684.php and tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php covering:
    • Private/protected static methods on non-final classes with class-string parameter
    • Private/protected static methods on final classes with class-string parameter
    • Literal class name calls (X::privateFoo()) guarded by method_exists()

Analogous cases probed

  • Instance method calls ($obj->privateFoo() with method_exists($obj, 'privateFoo')): Already reports visibility errors correctly — MethodCallCheck always reaches its visibility check without early returns.
  • property_exists() narrowing: Uses createFuncCallSpec for string inputs already, so class-string types are not modified. Not affected.
  • Object branch of MethodExistsTypeSpecifyingExtension: Instance method calls through this path reach the visibility check in MethodCallCheck. Not affected.

Root cause

MethodExistsTypeSpecifyingExtension was adding HasMethodType to class-string types even when the method already existed natively on the class. This changed the type representation from GenericClassStringType (or ConstantStringType) to an IntersectionType([GenericClassStringType, HasMethodType]). This corrupted representation caused StaticMethodCallCheck to skip visibility checking through two paths:

  1. Name node path (e.g., X::privateFoo()): The $classStringType->hasMethod() check returned yes due to HasMethodType in the intersection (via TrinaryLogic::lazyMaxMin), triggering an early return that skipped all further checks including visibility.
  2. Expression node path (e.g., $row::privateFoo()): The intersection type was not recognized as GenericClassStringType by the instanceof check, falling through to isString()->yes() which also returned yes (via lazyMaxMin), causing another early return before visibility checking.

The fix prevents the type corruption by not adding redundant HasMethodType when the method is already native. The createFuncCallSpec approach instead records that the method_exists() call evaluates to true, without modifying the class-string type.

Test

  • testBug14684 in CallStaticMethodsRuleTest.php: Tests that private and protected static method calls report errors when guarded by method_exists(), for both class-string<X> parameters and literal class name calls.
  • Verified that existing tests (testHasMethodStaticCall, testBug1267, testBug6147, testBug4550) continue to pass — these use non-native methods or public methods, so the fix does not affect them.

Fixes phpstan/phpstan#14684

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 think this solution will report

class X {
	final static private function privateFoo():void {}
}

class Y {
}

/** @param class-string<X|Y> $row */
function testClassString($row): void
{
	if (method_exists($row, 'privateFoo')) {
		$row::privateFoo();
	}
}

While it does for

class X {
	final private function privateFoo():void {}
}

class Y {
}

function testClassString(X|Y $row): void
{
	if (method_exists($row, 'privateFoo')) {
		$row->privateFoo();
	}
}

…ative

- In `MethodExistsTypeSpecifyingExtension`, when narrowing class-string
  types after `method_exists()`, use `createFuncCallSpec` instead of
  adding `HasMethodType` when all class reflections have the method
  natively.
- Previously, adding `HasMethodType` to the class-string type changed
  its representation from `GenericClassStringType` to an
  `IntersectionType`, which caused `StaticMethodCallCheck` to skip
  visibility checking via two paths: the early return at line 72-74
  for Name nodes, and the `isString()->yes()` bail-out at line 203-205
  for expression nodes.
- With this fix, the class-string type is preserved, and the normal
  visibility checking flow in `StaticMethodCallCheck` correctly reports
  calls to private/protected static methods even when guarded by
  `method_exists()`.

Closes phpstan/phpstan#14684
@VincentLanglet VincentLanglet force-pushed the create-pull-request/patch-be8jrm7 branch from 065df84 to af78744 Compare May 24, 2026 11:32
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 think this solution will report

class X {
	final static private function privateFoo():void {}
}

class Y {
}

/** @param class-string<X|Y> $row */
function testClassString($row): void
{
	if (method_exists($row, 'privateFoo')) {
		$row::privateFoo();
	}
}

While it does for

class X {
	final private function privateFoo():void {}
}

class Y {
}

function testClassString(X|Y $row): void
{
	if (method_exists($row, 'privateFoo')) {
		$row->privateFoo();
	}
}

…intersection

When method_exists() narrowing adds HasMethodType to a class-string type
(e.g. class-string<X|Y>&hasMethod(foo)), StaticMethodCallCheck previously
returned early at the isString()->yes() check, skipping visibility checking
entirely. This also affected the union case class-string<X|Y> where only
some classes have the method natively.

Fix: when the type is a class-string (isClassString()->yes()), extract the
concrete class types that have the method natively and proceed with visibility
checking on those, rather than returning early.

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

Here's a summary of the changes made to address the review:

Problem: The reviewer pointed out that for class-string<X|Y> where only X has the method natively, the original fix wouldn't report visibility errors because $allNative would be false (Y doesn't have the method), causing HasMethodType to be added to the class-string type. StaticMethodCallCheck would then skip visibility checking entirely via the isString()->yes() early return.

Root cause in StaticMethodCallCheck: When HasMethodType is added to a class-string type, creating IntersectionType([GenericClassStringType(X|Y), HasMethodType(foo)]), the type doesn't match instanceof GenericClassStringType but does match isClassString()->yes() (via lazyMaxMin short-circuit). The old code at this branch just returned [[], null], skipping all visibility checks.

Fix: Added a new elseif ($classType->isClassString()->yes()) branch in StaticMethodCallCheck (between the GenericClassStringType check and the isString() check) that:

  1. Extracts the class-string object type via getClassStringObjectType()
  2. Filters to only concrete class reflections that have the method natively
  3. Proceeds with visibility checking on those concrete types
  4. Falls through to return [[], null] only when no class has the method natively (i.e., the method is only known via HasMethodType)

Test: Added the reviewer's exact test case — class-string<X|Y> where X has privateFoo/protectedFoo natively and Y doesn't — verifying visibility errors are reported for private and protected methods, while public methods pass.

Copy link
Copy Markdown
Contributor

@staabm staabm left a comment

Choose a reason for hiding this comment

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

resolve the conflict and rebase

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