Skip to content

CI: migrate PHPStan to PrestaShop/.github action#30

Merged
jolelievre merged 8 commits into
PrestaShop:devfrom
mattgoud:fix-ci-migrate-phpstan-to-action
Jun 11, 2026
Merged

CI: migrate PHPStan to PrestaShop/.github action#30
jolelievre merged 8 commits into
PrestaShop:devfrom
mattgoud:fix-ci-migrate-phpstan-to-action

Conversation

@mattgoud

@mattgoud mattgoud commented Jun 2, 2026

Copy link
Copy Markdown
Contributor
Questions Answers
Branch? dev
Description? The Docker-based PHPStan approach (tests/phpstan.sh) was failing with Error response from daemon: No such container: temp-ps because the prestashop/prestashop:latest Docker image now exits immediately due to entrypoint changes, causing the volume container to disappear before PHPStan can use it.

This PR migrates to the shared PrestaShop/.github PHPStan action (as used by ps_googleanalytics, ps_emailsubscription, blockreassurance):
- Adds tests/php/phpstan/ with version-specific neon configs for 8.2.x, 9.0.3, 9.1.x and develop
- Replaces the single Docker-based job with two action-based jobs: phpstan-74 (PS 8.2.x / PHP 7.2–8.1) and phpstan (PS 9.x — 9.0.3, 9.1.x, develop / PHP 8.1–8.5, with 9.0.3 capped at PHP 8.4 since PS 9.0.x does not support 8.5)
- Expands the php-linter job to check every PHP version from 7.2 to 8.5 (adds the missing 7.3, 8.1, 8.2, 8.3, 8.4 intermediates)
- Removes tests/phpstan.sh
- Aligns the module on the PS 8.2+ baseline targeted by the cleanup Epic: drops the 8.1.7 PHPStan target and bumps ps_versions_compliancy min to 8.2.0
- Fixes the 3 pre-existing PHPStan errors directly (instead of suppressing them via ignoreErrors): removes the unused $html property, replaces isset() on the non-nullable ModuleGridCore::$_direction with !empty(), and declares the executeS() row shape via a @var PHPDoc so the avgPriceSold/totalPriceSold offsets are recognized. All ignoreErrors blocks are now empty/removed.
Type? improvement
BC breaks? no
Deprecations? no
Fixed issue or discussion? Fixes #29, Fixes #31, Fixes #32, Fixes #33
Part of PrestaShop/PrestaShop#41648
Sponsor company PrestaShop SA

Replace the Docker-based phpstan.sh approach with the shared
PrestaShop/.github PHPStan action. The old approach was failing because
the prestashop/prestashop:latest Docker image now exits immediately
(due to entrypoint changes), causing "No such container: temp-ps" errors.

- Add tests/php/phpstan/ with version-specific neon configs for
  8.1.7, 8.2.x, 9.1.x and develop
- Add prepend-constants.php for PS 9.x bootstrap constants
- Replace single Docker-based phpstan job with two action-based jobs:
  phpstan-74 (PS 8.x, PHP 7.4-8.1) and phpstan (PS 9.x, PHP 8.1-8.5)
- Remove tests/phpstan.sh (no longer needed)

Fixes #ISSUE

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ps-jarvis

Copy link
Copy Markdown

Hello @mattgoud!

This is your first pull request on statsbestproducts repository of the PrestaShop project.

Thank you, and welcome to this Open Source community!

@github-project-automation github-project-automation Bot moved this to Ready for review in PR Dashboard Jun 2, 2026
@mattgoud mattgoud added the bug label Jun 2, 2026
@mattgoud mattgoud self-assigned this Jun 2, 2026
Suppress three pre-existing PHPStan errors exposed by the new 8.1.7/8.2.x
analysis that were not caught by the old Docker-based 1.7.x checks.
These should be addressed in a separate code-quality PR.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mattgoud and others added 3 commits June 2, 2026 19:49
- Add missing blank line after <?php in index.php (php-cs-fixer)
- Add ignoreErrors for pre-existing PHPStan errors in phpstan-9.1.x.neon
  and phpstan-develop.neon (refs PrestaShop#31, PrestaShop#32)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Drop the 8.1.7 target to match the project-wide module cleanup Epic
which standardizes native modules on PS 8.2+:
- php.yml: phpstan-74 matrix now runs against 8.2.x only (PHP 7.4/8.1)
- ps_versions_compliancy min bumped to 8.2.0
- remove the now-unreferenced phpstan-8.1.7.neon config

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve the 3 pre-existing PHPStan errors directly and drop all
ignoreErrors entries from the neon configs:
- remove the unused $html property (declared, never read)
- replace isset() on the non-nullable ModuleGridCore::$_direction with !empty()
- declare the executeS() row shape via @var PHPDoc so the
  avgPriceSold/totalPriceSold array offsets are recognized

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread .github/workflows/php.yml
Comment thread .github/workflows/php.yml Outdated
uses: prestashop/github-action-php-lint/8.1@master
# Run PHPStan against the module (PHP 7.4 – 8.1)
phpstan-74:
name: PHPStan (PHP 7.4 - 8.1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not start at 7.2?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok @jolelievre done

Comment thread .github/workflows/php.yml Outdated
runs-on: ubuntu-latest
strategy:
matrix:
presta_version: ['9.1.x', 'develop']

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
presta_version: ['9.1.x', 'develop']
presta_version: ['9.0.3', '9.1.x', 'develop']

Maybe 9.0.3 is still relevant even if the branch doesn't exist anymore, this ensure the module is still compatible with old PS versions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok @jolelievre done, added 9.0.3 to the matrix and created the matching tests/php/phpstan/phpstan-9.0.3.neon (the config path is resolved from presta_version, so the job would have failed without it).

mattgoud and others added 2 commits June 9, 2026 16:12
…9.0.3

- php-linter: add missing intermediate syntax checks (7.3, 8.1, 8.2, 8.3, 8.4)
- phpstan-74: lower floor to PHP 7.2 (real minimum for PS 8.2.x)
- phpstan: add PrestaShop 9.0.3 (tag, the 9.0.x branch no longer exists) + neon config

Part of PrestaShop/PrestaShop#41648

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PrestaShop 9.0.x supports PHP 8.1 up to 8.4 only, so the cross-product
matrix was generating an invalid 9.0.3 x 8.5 combination. Switch to an
explicit include list so each PrestaShop version tests its proper PHP
floor and ceiling (9.0.3: 8.1/8.4, 9.1.x and develop: 8.1/8.5). Also
make the "Prepare PHP env" step name reflect the actual matrix version.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mattgoud

mattgoud commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

correction: this is a major bump, not a patch — version set to 3.0.0 (statsbestproducts.php + config.xml)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mattgoud
mattgoud force-pushed the fix-ci-migrate-phpstan-to-action branch from 4e424d7 to 89eac96 Compare June 11, 2026 09:38

@jolelievre jolelievre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mattgoud

@ps-jarvis ps-jarvis added the Waiting for QA Status: Action required, Waiting for test feedback label Jun 11, 2026
@jolelievre
jolelievre merged commit 401d841 into PrestaShop:dev Jun 11, 2026
12 checks passed
@ps-jarvis ps-jarvis moved this from Ready for review to To be tested in PR Dashboard Jun 11, 2026
@github-project-automation github-project-automation Bot moved this from To be tested to Merged in PR Dashboard Jun 11, 2026
@ps-jarvis ps-jarvis moved this from Merged to To be tested in PR Dashboard Jun 11, 2026
@ps-jarvis

Copy link
Copy Markdown

PR merged, well done!

Message to @PrestaShop/committers: do not forget to milestone it before the merge.

@mattgoud mattgoud added this to the 3.0.0 milestone Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug enhancement Waiting for QA Status: Action required, Waiting for test feedback

Projects

Archived in project

4 participants