Decline a bootstrap autoloader only when it would re-include a loaded file - #6265
Conversation
01f2943 to
c12c14e
Compare
|
@ondrejmirtes rebased onto Instead of guessing from the name it now probes the autoloaders under the file-read trap and declines only when they would re-include an already-loaded file, which is what #14988 was actually about. Diff is +218/-17 across the locator and its new test. Verified locally on the rebased head:
GitHub Actions is in a major outage right now (no runs repo-wide since 15:32Z), so the checks here will only populate once it recovers. One thing worth knowing for coverage: an |
… file The guard added for phpstan/phpstan#14988 declined this locator whenever a function of the class's name existed. Classes and functions live in separate symbol spaces, so that also blocked class names which merely coincide with a function - Laravel's facade aliases are exactly that shape, since Cache, File, Str and friends coincide with the global helpers cache(), file() and str(), and `use Cache;` started reporting class.notFound. The hazard was never the name: it was a catch-all autoloader resolving a class name to the function's own file and including it a second time. Probing the autoloaders under the file-read trap says which file they would read without executing it, so only that case declines. A loader that defines the class without reading a file - class_alias(), eval() - now runs as it did before. Closes phpstan/phpstan#15102 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
c12c14e to
720c7c3
Compare
|
phpstan/phpstan, where the compiled PHAR exists, also has e2e/ tests. Would be nice to reproduce it there with actual project, make sure it's red, and then make sure here it's green. |
An autoloader that resolves a short alias whose name is also a global function - Laravel's Cache, File, Str and Hash all are - is declined by the locator without the fix, so `use File;` reports class.notFound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Added as
I also checked it against the release phars on a copy of the same project, which shows the regression window:
The project is the reduced facade-alias shape: a prepended autoloader that resolves Gates on the pushed head: full suite 21157, self-analysis clean, On the real-project route you suggested: larastan is already in |
|
Thank you! |
Fixes the second regression reported in phpstan/phpstan#15102 - the one @hoetaek reduced to a portable repro. This is not the ordering issue that opened that ticket; that one is #6069's heuristic and needs its own change.
What was wrong
#6185 added this to
AutoloadFunctionsSourceLocator:Classes and functions occupy separate symbol spaces, so a class name coinciding with a function name is legal and common. Laravel is the worst case: the facade aliases
Cache,File,Str,Hashcoincide with the global helperscache(),str()and with PHP's ownfile(),hash().Illuminate\Foundation\AliasLoadercreates those aliases lazily from a prepended autoloader, so with the guard in placeuse Cache;reportsclass.notFoundfor every larastan user. Verified against the release phars on @hoetaek's repro:File(collides withfile())Alias(no such function)What the hazard actually was
phpstan/phpstan#14988 was not about the name. A catch-all autoloader - PHP_CodeSniffer's, falling back to Composer's
findFile()- resolved a class name to a function's file and plain-included it a second time, fatally redeclaring the function.So the check now asks that question instead of guessing from the name: probe the autoloaders under
FileReadTrapStreamWrapper, which reports which file they would read without executing it, and decline only when that file is already inget_included_files(). An autoloader that defines the class without reading a file -class_alias(),eval()- runs exactly as it did in 2.2.8. The probe only runs for names that actually collide with a function, and not at all when the bucket holds no autoloaders - so a project without bootstrap autoloaders never reaches it: analysingsrc/Rulestriggers 0 probes. Where it does run,get_included_files()held ~35 entries, so the scan is nothing.One subtlety the trap forces: it intercepts file reads, not execution, so the probe really does run the autoloaders. An autoloader that defines the class without reading a file has therefore already done its work by the time the probe returns, and calling it again would redeclare what it defined -
class_alias()warns that the name is already in use. So when the class exists after the probe, the locator reflects it directly instead of looping over the autoloaders again; the test asserts the autoloader runs exactly once. For the same reason the probe stops at the first autoloader that defines the name, the wayspl_autoload_call()does - otherwise a later catch-all autoloader in the same bucket could still resolve that name to a loaded file and veto a class the first one had already defined.Verification
AutoloadFunctionsSourceLocatorTestpins two cases, both verified failing without the change: the alias case (on2.2.xthe locator declines and returns null), and a defining autoloader followed by a catch-all one in the same bucket. It also asserts the autoloader is invoked exactly once - the outcome assertion alone passes either way, only the count (2 vs 1) separates them.e2e/bug-14988still exits 0 - the redeclare fatal does not come back, it is prevented by the trap rather than by the name.e2e/bug-12972b,e2e/bug-12972c(Consult bootstrap-registered custom autoloaders only after the static source locators #6069) andCollectNewAutoloadFunctionsTestunchanged and green.2.2.x.Result cache E2E bug-11826, thephpstan-doctrinelane (Ask the node-callback scope directly instead of toMutatingScope() phpstan-doctrine#789 fixes it once 2.2.10 is out) and the integration tests, all red on every PR.Coverage:
e2e/bug-15102bcovers it - red without the fix (2xclass.notFound), green with it. An earlier version of this description claimed only a phar exhibits this; that was wrong, and came from a result cache shared between source states (same2.2.x-devcache key). Each measurement now uses a freshtmpDir.