Prevent autoloader invocation when checking for PHP core classes#617
Open
fain182 wants to merge 8 commits into
Open
Prevent autoloader invocation when checking for PHP core classes#617fain182 wants to merge 8 commits into
fain182 wants to merge 8 commits into
Conversation
class_exists($className, $autoload=true) (the default) causes the Composer autoloader to load arbitrary class files encountered during file parsing. When an optional package is installed but a required PHP extension is missing (e.g. doctrine/mongodb-odm installed, mongodb ext absent), loading the class file throws a PHP Error that propagates through class_exists() and is caught by FileParser's \Throwable handler – converting a valid PHP file into a spurious parsing error and failing the check with exit code 1. Fix: pass $autoload=false to all *_exists() checks in checkIsPhpCoreClass. PHP built-in classes are always pre-loaded; any symbol that is not already present in the process cannot be a PHP internal class, so triggering the autoloader is both unnecessary and harmful. Regression test: test_adding_dependency_on_unloaded_optional_vendor_class_does_not_throw Reported by Sylius/Sylius#19003 – phparkitect upgrade from 0.8 to 1.0 broke the architectural check due to this side-effect. https://claude.ai/code/session_01A9BJtoBs8od2HvUya66nq2
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #617 +/- ##
============================================
- Coverage 98.33% 98.29% -0.05%
- Complexity 693 696 +3
============================================
Files 87 87
Lines 1981 1991 +10
============================================
+ Hits 1948 1957 +9
- Misses 33 34 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
enum_exists() is a PHP 8.1+ function. The symfony/polyfill-php81 bootstrap is not in autoload_files so it is not loaded in the PHAR, meaning on PHP 8.0 the function does not exist at all. In the previous code (autoload=true) this was harmless: class_exists() or interface_exists() would short-circuit before enum_exists() was reached. With autoload=false all three return false for unloaded symbols, so enum_exists() is called for every dependency – throwing a Fatal Error on PHP 8.0 that FileParser catches as a spurious parsing error. Fix: skip the enum_exists() check on PHP < 8.1. Enums cannot exist on 8.0 so the guard is unnecessary there. https://claude.ai/code/session_01A9BJtoBs8od2HvUya66nq2
Replace the chained negations with a dedicated method that reads as a series of positive early-returns. https://claude.ai/code/session_01A9BJtoBs8od2HvUya66nq2
The comment belongs next to the code it explains. https://claude.ai/code/session_01A9BJtoBs8od2HvUya66nq2
- Countable (PHP built-in interface) must be detected and filtered - UserTestTrait (user trait, pre-loaded by the test file) must pass through trait_exists() but NOT be filtered (isInternal() = false) https://claude.ai/code/session_01A9BJtoBs8od2HvUya66nq2
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
2 participants
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.
Summary
Fixes a bug where
checkIsPhpCoreClass()could trigger the Composer autoloader during file parsing, causing a spurious "parsing error" and exit code 1 on valid PHP files.How we found it
Reported via Sylius/Sylius#19003, where upgrading phparkitect from
^0.8to^1.0broke the architectural check with:The file is a perfectly valid Symfony DI config. The error was triggered because phparkitect's own process called
class_exists('Doctrine\ODM\MongoDB\DocumentRepository', true)while analysing it, which invoked the autoloader. Since themongodbPHP extension was not installed in that CI environment, loading the class file threw a PHPErrorthat propagated out ofclass_exists()and was caught byFileParser's\Throwablehandler — turning a valid file into a parsing error.Fix
Pass
$autoload = falseto all*_exists()calls incheckIsPhpCoreClass(). PHP built-in classes are always pre-loaded; if a symbol is not already present in the process it cannot be a PHP internal class, so triggering the autoloader is both unnecessary and harmful.https://claude.ai/code/session_01A9BJtoBs8od2HvUya66nq2