Skip to content

Fix issues reported by PHPStan #1334

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
javiereguiluz opened this issue May 5, 2022 · 5 comments · Fixed by #1370
Closed

Fix issues reported by PHPStan #1334

javiereguiluz opened this issue May 5, 2022 · 5 comments · Fixed by #1370

Comments

@javiereguiluz
Copy link
Member

When we introduced PHPStan, we added lots of errors to a "baseline file" to ignore them instead of fixing them. So, let's fix them.

In my opinion there are two kinds of issues:

(1) Issues that are PHP errors or improvements. Such as https://github.com/symfony/demo/pull/1331/files We need to fix those.

(2) Issues that are solved by adding metadata for PHPStan. I'm not sure 100% about fixing these. This is a Symfony Demo app, so only Symfony is important. If these issues require lots of changes and future maintenance, I wouldn't fix them.

What do you think?

@javiereguiluz
Copy link
Member Author

About (2) @stof added the following in #1332 (comment)


Well, it also helps IDEs that support generic classes (PHPStorm has partial support for them, improving it in each release).
Generics are actually a de-facto standard, being supported with the same syntax by phpstan, psalm, PHPStorm (and maybe VSCode but I haven't checked)

And MakerBundle generates that @extends tag now. So users generating a User entity today will have it.

@tacman
Copy link
Contributor

tacman commented Jun 14, 2022

What do you think of bumping to Symfony 6.1 (which would obviously require PHP 8.1)? If we're going to fix PHPStan errors to have an app that shows best practices, it seems like it'd be nice to leverage PHP 8.1 and Symfony 6.1.

Since demo 2.0 is for Symfony 6.0, we could make demo 2.1 for Symfony 6.1.

@stof
Copy link
Member

stof commented Jun 14, 2022

@tacman upgrading to Symfony 6.1 is an unrelated topic (and in progress in #1339)

@kniziol
Copy link
Contributor

kniziol commented Nov 5, 2022

No errors found. PHPStan said [OK] No errors. Details below:

$ ./vendor/bin/phpstan --memory-limit=-1 -vvv
Note: Using configuration file phpstan.neon.dist.
 44/44 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100% < 1 sec/< 1 sec 40.0 MiB

Result cache is saved.

                                                                                                                        
 [OK] No errors                                                                                                         
                                                                                                                        

Used memory: 40 MB

$ php -v
PHP 8.1.12 (cli) (built: Oct 28 2022 18:58:22) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.12, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.12, Copyright (c), by Zend Technologies
    with Xdebug v3.2.0RC1, Copyright (c) 2002-2022, by Derick Rethans

@stof
Copy link
Member

stof commented Nov 10, 2022

@kniziol running phpstan ignores the errors that are in the baseline (that's the goal of the baseline). To empty the baseline, you need to look at the baseline file (or to empty that file before running phpstan)

javiereguiluz added a commit that referenced this issue Dec 8, 2022
…x PHP-CS-Fixer errors #1334 (kniziol)

This PR was squashed before being merged into the main branch.

Discussion
----------

Update PHPDoc and remove unnecessary PHPDoc applied to fix PHP-CS-Fixer errors #1334

Commits
-------

0d10adb Update PHPDoc and remove unnecessary PHPDoc applied to fix PHP-CS-Fixer errors #1334
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.

4 participants