Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from PHPStan to Psalm #63

Merged
merged 15 commits into from
Jan 4, 2022
Merged

Switch from PHPStan to Psalm #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
373fddd
feat: remove Prophecy usage
weierophinney Jan 3, 2022
fb44204
qa: remove phpstan from project
weierophinney Jan 3, 2022
f492f17
qa: add Psalm to CI pipeline
weierophinney Jan 3, 2022
af38c2f
feat: remove Psalm dependencies and configuration during installation
weierophinney Jan 3, 2022
4c63fbc
qa: create Psalm baseline and apply automated ruleset
weierophinney Jan 3, 2022
bf140de
qa: make it possible to run phpcs, and apply autofixes
weierophinney Jan 3, 2022
65b71ed
qa: ensure dependencies are installed correctly and phpcs can run in …
weierophinney Jan 3, 2022
6cfb6d1
qa: bump minimum supported mezzio/mezzio-tooling version
weierophinney Jan 3, 2022
229030c
qa: bump minimum supported PHPUnit to 9.5.11
weierophinney Jan 3, 2022
ce4dc51
qa: ignore resources and config when running Psalm
weierophinney Jan 3, 2022
3195759
fix: bump vfsstream minimum version to 1.6.10
weierophinney Jan 3, 2022
2756602
fix: bump pimple to latest supported version
weierophinney Jan 3, 2022
b6de910
qa: do not ignore platform reqs for PHP 8.0, 8.1
weierophinney Jan 3, 2022
33bcc58
qa: replace tabs with spaces in composer.json
weierophinney Jan 3, 2022
8b2ed15
fix: remove psalm baseline on installation
weierophinney Jan 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .laminas-ci.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"additional_composer_arguments": [
"--no-scripts",
"--no-plugins"
],
"ignore_php_platform_requirements": {
"8.1": true
"8.0": false,
"8.1": false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've indicated that the --ignore-platform-requirement flag should NOT be used with PHP 8.0 and 8.1. composer update operations run normally without the flag, and, when using --prefer-lowest, we need to get requirements that are appropriate for those versions (otherwise, tests fail due to syntax errors!).

Copy link
Member

Choose a reason for hiding this comment

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

AFAIK, if the property ignore_php_platform_requirements is available, all PHP versions will have false by default.
So it would be enough to simply create the property as an empty object to ensure that platform reqs won't be ignored.

If the ignore_php_platform_requirements property is not available, PHP 8.0 will always have ignored platform reqs.

}
}
5 changes: 5 additions & 0 deletions .laminas-ci/pre-run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/bin/bash

# Due to the fact that we are disabling plugins when installing/updating/downgrading composer dependencies
# we have to manually enable the coding standard here.
composer enable-codestandard
31 changes: 17 additions & 14 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,19 @@
"jsoumelidis/zend-sf-di-config": "^0.5",
"laminas/laminas-coding-standard": "~2.3.0",
"laminas/laminas-development-mode": "^3.3.0",
"laminas/laminas-pimple-config": "^1.1.1",
"laminas/laminas-pimple-config": "^1.4",
"laminas/laminas-servicemanager": "^3.6.4",
"mezzio/mezzio-fastroute": "^3.2.0",
"mezzio/mezzio-laminasrouter": "^3.1.0",
"mezzio/mezzio-laminasviewrenderer": "^2.3.0",
"mezzio/mezzio-platesrenderer": "^2.3.0",
"mezzio/mezzio-tooling": "^2.0",
"mezzio/mezzio-tooling": "^2.1",
"mezzio/mezzio-twigrenderer": "^2.7.0",
"mikey179/vfsstream": "^1.6.8",
"phpspec/prophecy": "^1.10.3",
"phpspec/prophecy-phpunit": "^2.0",
"phpstan/phpstan": "^0.12",
"phpstan/phpstan-strict-rules": "^0.12",
"phpunit/phpunit": "^9.5.5",
"roave/security-advisories": "dev-master"
"mikey179/vfsstream": "^1.6.10",
"phpunit/phpunit": "^9.5.11",
"psalm/plugin-phpunit": "^0.16.1",
"roave/security-advisories": "dev-master",
"vimeo/psalm": "^4.17"
},
"autoload": {
"psr-4": {
Expand All @@ -98,13 +96,18 @@
"mezzio": "laminas --ansi",
"check": [
"@cs-check",
"@test",
"@analyze"
"@test"
],
"analyze": "phpstan analyze -l 3 -c ./phpstan.installer.neon ./src ./config",
"clear-config-cache": "php bin/clear-config-cache.php",
"cs-check": "phpcs -s",
"cs-fix": "phpcbf",
"enable-codestandard": "Dealerdirect\\Composer\\Plugin\\Installers\\PHPCodeSniffer\\Plugin::run",
"cs-check": [
"@enable-codestandard",
"phpcs -s"
],
"cs-fix": [
"@enable-codestandard",
"phpcbf"
],
Comment on lines +102 to +110
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was proposed by @boesing in #57, and I expanded on it here as we need to install without plugins or scripts executing. These changes allow running phpcs/phpcbf even whn plugins are not installed.

Copy link
Member

Choose a reason for hiding this comment

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

TBH, we could automate some of this in the laminas CI setup (to auto-push to contributor branches after message confirmation, potentially)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only necessary on projects where we are using the --disable-plugins and/or --disable-scripts switches. We might want to detect that, but I think there's only going to be a very small number of places where it will be useful; since there are ways to work around it with the existing tooling, I'm not sure if it makes sense.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this is somehow solvable in another way.
I remember that phpstan has some similar magic and most of my colleagues which do use phpstan in other projects (at work) got rid of that composer plugin magic and added the plugins path to their config.

Yes, I know that would mean we would have to change all repositories if we wanted to roll that out for all components but IMHO its only necessary here due to the fact that we do, in fact, ignore composer plugins intentional.

So instead of still relying on composer, we could simply change the path to the coding standard in the codestyle config instead. WDYT?

"serve": "php -S 0.0.0.0:8080 -t public/",
"test": "phpunit --colors=always",
"test-coverage": "phpunit --colors=always --coverage-clover clover.xml"
Expand Down
Loading