Skip to content

Pin phpVersion on the 9.x PHPStan jobs and drop the AdminController cast helper - #143

Open
mattgoud wants to merge 1 commit into
PrestaShop:devfrom
mattgoud:fix/phpstan-phpversion-9x
Open

Pin phpVersion on the 9.x PHPStan jobs and drop the AdminController cast helper#143
mattgoud wants to merge 1 commit into
PrestaShop:devfrom
mattgoud:fix/phpstan-phpversion-9x

Conversation

@mattgoud

@mattgoud mattgoud commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
Questions Answers
Description? Pin phpVersion: 80100 on the four PrestaShop 9.x PHPStan configurations, and remove the getAdminController() cast helper that existed only to work around the error caused by not pinning it.
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #142
How to test? Every PHPStan job must stay green, including the two 8.2.x ones which are deliberately left untouched. Functionally, open Modules > Module Manager, search "Newsletter subscription", click Configure on both PrestaShop 8.2 and 9.x: the configuration page and its translatable fields must render and save exactly as before.
Sponsor company @PrestaShopCorp

The error the helper was hiding

The four 9.x configurations did not pin phpVersion, so PHPStan took the syntax level from the composer platform declaration, 7.1.0. At that level it cannot parse PrestaShop\PrestaShop\Core\Context\LegacyControllerContext, which uses constructor property promotion, readonly and union types. Removing the helper without pinning anything produces three of these, at lines 1141, 1242 and 1281:

Call to method getLanguages() on an unknown class
PrestaShop\PrestaShop\Core\Context\LegacyControllerContext.

Worth being precise about the message: it is an unknown class, not an undefined method on the AdminController|FrontController|LegacyControllerContext|null union. The union is fine. getLanguages() exists on AdminController and on LegacyControllerContext alike, so the call has always been correct at runtime on both 8.2 and 9.x.

The @var AdminController annotation made the type resolvable again, which silenced the error without addressing its cause, and left a @return AdminController that is untrue on 9.x, where Context::$controller holds a LegacyControllerContext.

Measured

Locally against a PrestaShop 9.2.0 core, PHPStan 1.12.12, level 5, using the same vendor/prestashop/php-dev-tools/phpstan/ps-module-extension.neon merge the shared CI action performs.

Case Result
Before this PR, helper in place no errors
Helper removed, nothing else 3 errors, unknown class
This PR, phpVersion: 80100 plus helper removed no errors, on 9.0.3, 9.1.5, 9.2.x and develop

Why this is worth more than a tidier annotation

The same helper, with a native : AdminController return type instead of a phpdoc, threw a TypeError on the module configuration page of ps_wirepayment on every PrestaShop 9.x shop, see PrestaShop/ps_wirepayment#103. This module was never affected, since PHP does not enforce a phpdoc. But the annotation is the same untruth, and it is exactly what makes a wrong assumption look verified to static analysis. Pinning phpVersion removes the reason to assert it at all.

What is deliberately not in this PR

  • tests/php/phpstan/phpstan-8.2.x.neon is untouched. LegacyControllerContext does not exist on 8.2, where the union is AdminController|FrontController|LegacyControllerBridgeInterface|null, and that job runs on PHP 7.2, so pinning a PHP 8 syntax level there would be wrong.
  • composer.json still declares require.php: >=7.1 and config.platform.php: 7.1.0, while ps_versions_compliancy has been at 8.2.0 since the alignment on [EPIC] Modules - Cleaning Compatibility PrestaShop#41648 and PrestaShop 8.2 requires PHP 7.2.5. Worth aligning on 7.2.5, but it touches composer.lock and does not affect the issue fixed here, since 7.2.5 is still below the PHP 8.0 needed to parse the core class.
  • tests/php/phpstan/phpstan.neon carries text accidentally pasted from the GitHub review UI on its first comment line. Harmless, left out to keep this diff focused.

@github-project-automation github-project-automation Bot moved this to Ready for review in PR Dashboard Sep 9, 2026
The four 9.x configurations did not pin phpVersion, so PHPStan analysed the
PrestaShop 9.x core with the PHP version taken from the composer platform
declaration, 7.1.0. At that syntax level it cannot parse
LegacyControllerContext and reports it as an unknown class, which is the
error getAdminController() was silencing with a @var AdminController that is
untrue on 9.x.

Pinning phpVersion: 80100 removes the cause, so the helper and its three call
sites go back to the direct Context::$controller call. getLanguages() exists
on AdminController and on LegacyControllerContext alike, so the call is
correct at runtime on both 8.2 and 9.x.

phpstan-8.2.x.neon is left untouched: LegacyControllerContext does not exist
there and that job runs on PHP 7.2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

PHPStan 9.x jobs analyse the core as PHP 7.1, which is why the AdminController cast helper exists

2 participants