Skip to content

Add phpstan #1260

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.

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

Closed
BackEndTea opened this issue Aug 15, 2021 · 2 comments · Fixed by #1265
Closed

Add phpstan #1260

BackEndTea opened this issue Aug 15, 2021 · 2 comments · Fixed by #1265

Comments

@BackEndTea
Copy link

A lot has changed since #733, we now have baselines and a good way to ignore errors.

Is there any interest in adding it to the project?

@javiereguiluz
Copy link
Member

Yes ... this demo project follows Symfony practices as close as possible ... so, given that Symfony code already uses PHPStan, we can try to add it here too. Thanks!

@burned42
Copy link
Contributor

burned42 commented Oct 3, 2021

I gave this a try and created #1265 I hope that's ok :)

sayjun0505 added a commit to sayjun0505/sym_proj that referenced this issue Apr 16, 2023
This PR was squashed before being merged into the main branch.

Discussion
----------

Add phpstan to github actions workflow

Fixes symfony/demo#1260

I tried checking different repos under https://github.com/symfony to see how those solve running phpstan and tried to apply that here. I saw that all the repos seem to install phpstan in the actions workflows instead of adding them to composer.json,so I did that, too. Also it seems like all of the projects I checked out are using the phpstan level 5, so that's what I was using. What I also found out is that most of them configured phpstan to use some phpunit autoload file, that's also why phpunit has to be installed in the workflow before running phpstan.

Most of the errors reported with this configuration are probably not that hard to solve, but I didn't want to include any unrelated code changes here, so I added a baseline file. I can also try to fix them if you think that should be part of this PR.

I think adding phpstan to the lint workflow sounds right, but I wasn't too sure if phpstan should be added to the 'linters' job or if it should get its own job. I decided to add it to the existing one to avoid having to duplicate all the composer/php setup steps into another job.

I hope that's something like what you expected to see regarding running phpstan for this project :)

Commits
-------

30a6fd9 Add phpstan to github actions workflow
spider-yamet added a commit to spider-yamet/sym_proj that referenced this issue Apr 16, 2023
This PR was squashed before being merged into the main branch.

Discussion
----------

Add phpstan to github actions workflow

Fixes symfony/demo#1260

I tried checking different repos under https://github.com/symfony to see how those solve running phpstan and tried to apply that here. I saw that all the repos seem to install phpstan in the actions workflows instead of adding them to composer.json,so I did that, too. Also it seems like all of the projects I checked out are using the phpstan level 5, so that's what I was using. What I also found out is that most of them configured phpstan to use some phpunit autoload file, that's also why phpunit has to be installed in the workflow before running phpstan.

Most of the errors reported with this configuration are probably not that hard to solve, but I didn't want to include any unrelated code changes here, so I added a baseline file. I can also try to fix them if you think that should be part of this PR.

I think adding phpstan to the lint workflow sounds right, but I wasn't too sure if phpstan should be added to the 'linters' job or if it should get its own job. I decided to add it to the existing one to avoid having to duplicate all the composer/php setup steps into another job.

I hope that's something like what you expected to see regarding running phpstan for this project :)

Commits
-------

30a6fd9 Add phpstan to github actions workflow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants