[phpstan-bot]

Summary

Restricted-usage extensions (deprecations, @internal, etc.) were not called for members accessed on a nullable object type. For example, $requestStack->getCurrentRequest()->get() where getCurrentRequest() returns ?Request was not reported. $nullable?->get() was reported. The same gap existed for other non-object union members (Foo|false, Foo|int), for first-class callables and string casts, and for static access, class constants and static properties on ?Foo and on class-string<Foo>.

Changes

• New src/Rules/RestrictedUsage/RestrictedUsageObjectClassNames.php:
  • get(Type): returns getObjectClassNames(). If that list is empty and the type may be an object, it intersects the type with object and reads the class names from the result.
  • getForStaticAccess(Type): removes null, applies getObjectTypeOrClassStringObjectType() so class-strings resolve to their object type, then calls get().
• Instance access uses get(): RestrictedMethodUsageRule, RestrictedPropertyUsageRule, RestrictedMethodCallableUsageRule, RestrictedUsageOfDeprecatedStringCastRule.
• Static access (::) uses getForStaticAccess() on the type found by RuleLevelHelper: RestrictedStaticMethodUsageRule, RestrictedStaticMethodCallableUsageRule, RestrictedStaticPropertyUsageRule, RestrictedClassConstantUsageRule.

Root cause

UnionType::getObjectClassNames() returns an empty list as soon as one member is not an object, so Request|null produced no class names and the extensions were never called. The static rules used FoundTypeResult::getReferencedClasses(), which comes from the same getObjectClassNames() call. That list is also empty for class-string<Foo>, so $className::deprecatedMethod() was never reported either.

Test

tests/PHPStan/Rules/RestrictedUsage/data/bug-15681.php has a testBug15681 case in each of the eight restricted-usage rule tests. It covers:

• method calls on ?Foo, on Foo|false, Foo|Other|null and Foo|int, and on a ?Foo-returning method call
• property fetches on the same kinds of types
• first-class callables
• (string) casts
• static method calls, static callables, static properties and class constants on ?Foo, class-string<Foo> and class-string<Foo>|null

All eight tests fail without the fix.

Fixes phpstan/phpstan#15681

[staabm]

can this be simplified to $referencedClasses = TypeCombinator::removeNull($classTypeResult->getType())->getObjectTypeOrClassStringObjectType()->getObjectClassNames() ?

alternatively we need a test-case which shows it needs to stay like that.

same applies to other similar changes in this PR.

[phpstan-bot]

I made your simplification in all four static-access rules: RestrictedStaticMethodUsageRule, RestrictedStaticMethodCallableUsageRule, RestrictedStaticPropertyUsageRule and RestrictedClassConstantUsageRule. Each now uses:

$referencedClasses = TypeCombinator::removeNull($classTypeResult->getType())->getObjectTypeOrClassStringObjectType()->getObjectClassNames();

The intersection with object did nothing in these rules. They get their type from RuleLevelHelper::findTypeToCheck(), which already drops union members that don't have the member being accessed. I checked this outside the test suite with S|int, S|false and class-string<S>|int: the old and new versions report exactly the same errors.

The four instance-access rules (method call, property fetch, method callable, string cast) still need the intersection, so I left them unchanged. There the type comes straight from $scope->getType() with no filtering. Without the intersection the code would be the original $type->getObjectClassNames(), which returns an empty list for Foo|null, and that empty list is the bug itself. The existing testBug15681 cases for those rules (?Request, Request|false, Request|int, and the getCurrentRequest()->doFoo() chain) already show this: they fail without the intersection.

The change is committed as 2e262ac and not pushed. make tests passes (23,546 tests, 108 skipped), and make phpstan and make cs are clean.

[staabm]

can this be simplified to $referencedClasses = TypeCombinator::removeNull($classTypeResult->getType())->getReferencedClasses();?

[staabm]

no, I was wrong.

[phpstan-bot]

I didn't change any code, and I didn't make a new commit or rerun the tests for this round. In the comment on RestrictedStaticMethodCallableUsageRule.php, staabm took back the suggestion to use TypeCombinator::removeNull(...)->getReferencedClasses() ("no, I was wrong"), and has since approved the PR. The code is the same as at commit 2e262ac, which the last round's make tests, make phpstan and make cs passed on.

The suggestion wouldn't have worked anyway. getReferencedClasses() returns every class mentioned anywhere in a type, including generic type arguments, so it's not the same as the class you access a member on. It also doesn't resolve class-string<Foo> to Foo the way getObjectTypeOrClassStringObjectType() does, so $className::doFoo() with a class-string<StaticRequest> would stop being checked.

All four static-access rules still use:

$referencedClasses = TypeCombinator::removeNull($classTypeResult->getType())->getObjectTypeOrClassStringObjectType()->getObjectClassNames();

[phpstan-bot]

I kept getObjectTypeOrClassStringObjectType()->getObjectClassNames() and didn't switch to getReferencedClasses(). As you suggested earlier, I added a test case that shows why the current code needs to stay.

Why getReferencedClasses() doesn't work here: it doesn't only return the class whose member is being accessed. It returns every class mentioned anywhere in the type, including generic type arguments. GenericObjectType::getReferencedClasses() adds the referenced classes of each type argument, and GenericClassStringType passes the call through to its inner type. For GenericStaticRequest<StaticRequest>|null, it returns both GenericStaticRequest and StaticRequest, so the rules would also check StaticRequest::doFoo(), a class that isn't being called.

New test case: I added testGeneric() to tests/PHPStan/Rules/RestrictedUsage/data/bug-15681.php. It does static method calls, static callables, static property reads and class constant reads on:

• GenericStaticRequest<StaticRequest>|null
• class-string<GenericStaticRequest<StaticRequest>>

StaticRequest also has a doFoo() method, a $foo property and a FOO constant, which makes the difference visible. I added the expected errors to the four static-rule tests.

I ran it both ways:

• With your suggested change: each new line is reported twice, once for GenericStaticRequest and once for StaticRequest. In RestrictedStaticMethodUsageRuleTest, lines 115 and 120 each showed two Cannot call doFoo errors.
• With the current code: each line is reported once, which is correct.

make tests passes (23,546 tests, 108 skipped), and make phpstan and make cs are clean. The test is committed as 4914ca9 and not pushed.

[ondrejmirtes]

👍

[bbrala]

I noticed a few things whil working on this, i ddin push before you fixed it xD See references pr