Conversation
- Add phpstan/phpstan ^2.1 to require-dev - Configure PHPStan at level 8 with baseline for bridge compatibility errors - Fix real type issues: nullable driver properties, preg_replace returns, file_get_contents/preg_split false checks, lazy-init property types, Runner::MODE_* type annotation, and undefined variable in SQL parser - Baseline 50 unfixable errors from multi-version bridge code (Doctrine, Nette, Symfony, Nextras DBAL, Dibi) - Add PHPStan CI job on PHP 8.4
PHPStan 2.x requires PHP 8.1+ but the project supports PHP >=7.1, so having it in require-dev breaks composer install on older PHP versions. Install it separately in the PHPStan CI job instead.
There was a problem hiding this comment.
Pull request overview
This PR introduces PHPStan 2.x static analysis at level 8 (with a baseline for multi-version bridge compatibility) and makes several small type-safety fixes across drivers, configuration, and bridge/controller code. It also adds a dedicated CI job to run PHPStan on PHP 8.4.
Changes:
- Add PHPStan configuration (level 8) plus a baseline to accommodate multi-version bridge code.
- Fix/clarify types and flow for PHPStan in drivers, configuration, controllers, and bridges.
- Add a GitHub Actions QA job to run
phpstan analyseon PHP 8.4.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/Drivers/PgSqlDriver.php |
Tighten $schemaQuoted to string and cast preg_replace() to string for PHPStan. |
src/Drivers/MySqlDriver.php |
Cast preg_replace() to string for PHPStan. |
src/Drivers/BaseDriver.php |
Tighten $tableNameQuoted to string and initialize $queryLength for flow analysis. |
src/Controllers/BaseController.php |
Refine $mode phpDoc to Engine\Runner::MODE_*. |
src/Configurations/DefaultConfiguration.php |
Fix lazy-init property phpDoc types to include null. |
src/Bridges/SymfonyConsole/CreateCommand.php |
Add additional null guard around by-ref out param usage. |
src/Bridges/DoctrineOrm/StructureDiffGenerator.php |
Handle file_get_contents()/preg_split() failure cases more safely. |
phpstan.neon |
Add PHPStan config including baseline and analysis level/paths. |
phpstan-baseline.neon |
Baseline existing compatibility/static-analysis issues in bridge code. |
composer.json |
Add phpstan/phpstan to require-dev. |
.github/workflows/qa.yaml |
Add a PHPStan CI job running on PHP 8.4. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $name = $this->getFileName($label, $extension); | ||
|
|
||
| if ($this->hasNumericSubdirectory($dir, $foundYear)) { | ||
| if ($this->hasNumericSubdirectory($dir, $foundYear) && $foundYear !== null) { |
There was a problem hiding this comment.
$foundYear is used as an out-parameter without being initialized in this scope. In PHP this can lead to an "undefined variable" notice (and it also forces extra null checks for static analysis). Initialize the variable (e.g. to null) before passing it by reference, and then the additional $foundYear !== null guard becomes unnecessary.
| if ($this->hasNumericSubdirectory($dir, $foundYear) && $foundYear !== null) { | |
| $foundYear = null; | |
| $foundMonth = null; | |
| if ($this->hasNumericSubdirectory($dir, $foundYear)) { |
| $content = file_get_contents($this->ignoredQueriesFile); | ||
| if ($content === false) { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
file_get_contents() will emit a PHP warning on failure (missing/unreadable file) even though you handle the false return value. If the intent is to treat an unreadable ignored-queries file as "no ignored queries", consider suppressing the warning (as done elsewhere in the codebase) or checking readability/existence before reading to avoid noisy output.
Summary
Inline fixes
$tableNameQuoted/$schemaQuotedfromnull|stringtostringwith''defaultpreg_replacereturn to(string)ingetInitTableSource()$queryLengthbefore inner loop to satisfy flow analysislist<Group>|null/array<string, IExtensionHandler>|nullfalsechecks forfile_get_contents()andpreg_split()$modetype toEngine\Runner::MODE_*$foundYearout-parameterBaseline (50 errors)
Bridge code uses
method_exists()checks to support multiple library versions. PHPStan can only see the installed version, so it flags calls to methods from other versions as errors. These are baselined:fetchAll(),exec()removed in 4.x) and ORM (getUpdateSchemaSql()signature change)convertToSql,convertBoolToSql,TYPE_*constants from older versions)literal-stringparam,getRowCount()nullability)ServiceDefinitionvsDefinitions\ServiceDefinition,Statementconstructor, etc.)TreeBuilderconstructor change)dibiclass name casing,query()return type)gotoflow analysis limitation)Test plan
vendor/bin/phpstan analyse --no-progressreports 0 errorstests/run-unit.shpasses (26 tests)