Do not inline overridable methods of abstract and @api classes in the phar build - #6412
Merged
Conversation
… phar build The getter inliner treated src/ and the bundled vendor packages as a closed world: a non-final method nothing there overrides was inlined at PHPStan's own call sites. Third-party subclasses were the blind spot, and the classes they subclass are exactly the ones whose hooks exist to be overridden: RuleTestCase::getCollectors() became `new CollectorRegistry([])` in the phar (shipmonk/dead-code-detector reported nothing), shouldPolluteScopeWithLoopInitialAssignments() became `true` (phpstan-strict-rules), ObjectType::describeAdditionalCacheKey() became '' (phpstan-symfony overrides it in ParentObjectType and TreeBuilderType). The closed world now stops at the extension surface: an abstract class or a non-final class tagged @api may be subclassed by extensions under the backward compatibility promise, so its overridable methods stay calls and its properties are not made public. Final classes, final and private methods and non-@api concrete classes are inlined as before. 7,177 of 8,578 call sites remain; the 1,401 dropped are in 33 classes, mostly the constant type getters. The scanned directories are an optional constructor argument so the collector can be tested against a fixture world. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UMpKB12KsL5HB8iUMCgrUM
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.
Fixes the
shipmonk/dead-code-detectorintegration job and the phpstan-strict-rules / phpstan-symfony / phpstan-doctrine extension-tests jobs on phpstan/phpstan 2.2.x, failing since the phar built from 9f7d822 (last green: d8ed7dd).Root cause: cd5c22a (getter inlining in the phar build) treated src/ + bundled vendor as a closed world and inlined any non-final method nothing there overrides. Third-party subclasses were invisible to that scan, and the affected classes are the ones whose hooks exist to be overridden:
RuleTestCase::getCollectors()→new CollectorRegistry([])in the phar, so dead-code-detector's collectors never ran and itsCollectedDataNoderule was never dispatched (375 failing tests).RuleTestCase::shouldPolluteScopeWithLoopInitialAssignments()→true(phpstan-strict-rules).ObjectType::describeAdditionalCacheKey()→''inside ObjectType's cache-key builder, while phpstan-symfony overrides it inParentObjectType/TreeBuilderType.Fix:
InlineCallCollector::isExtensible()keeps abstract classes and non-final@apiclasses out of the closed world, for callees and for properties an inlined body would make public. Final classes, final/private methods and non-@apiconcrete classes (MutatingScope, php-parser and BetterReflection value classes) are inlined as before. On this tree 7,177 of 8,578 call sites remain; the 1,401 dropped sit in 33 classes, mostlyConstantStringType/ConstantArrayType/ConstantIntegerTypegetters.Verification with a phar built from this branch:
New unit test
tests/PHPStan/Build/InlineCallCollectorTest.phppins the rule against a fixture world; without the guard it reportsAbstractHooks::getHooks => []andApiGetter::describeAdditionalCacheKey => ''.🤖 Generated with Claude Code
https://claude.ai/code/session_01UMpKB12KsL5HB8iUMCgrUM