search

webksde / ddev-vscode-devcontainer-drupal-template

Drupal DDEV based development container with attached Visual Studio Code
23 stars 3 forks source link

PHPStan Reintegration #91

Closed joshsedl closed 1 year ago

joshsedl commented 2 years ago

We should implement a custom ddev phpstan command. Phpstan shows deprecated functions and is very useful for Drupal development, since it can also fix "bad" code.

Note, that we should use phpstan with Level 2 for reasons stated here:

https://mglaman.dev/blog/tighten-your-drupal-code-using-phpstan?utm_source=The+Weekly+Drop&utm_medium=email&utm_campaign=The_Weekly_Drop_Issue_524_02_24_2022

joshsedl commented 2 years ago

Maybe we should rather run PHPStan on Level 6, since this is the standard for "clean" php code.

Also have a look at this: https://github.com/mglaman/phpstan-drupal

joshsedl commented 2 years ago

Done!

joshsedl commented 1 year ago

The integration takes waaay to long to be useful, since it analyzes the whole project, instead of only the modules folder. Unfortunately, it seems like we can only define which part to analyze via the phpstan.neon...

I created an issue on the phpstan extension repo, asking how to properly use the "paths" variable here: https://github.com/SanderRonde/phpstan-vscode/issues/23#issuecomment-1335729291 but that config key is not what I was looking for.

Furthermore, I am not sure anymore if we should use the core phpstan file as the phpstan level is 0 and as stated above we should at least use 2 for deprecation errors. Also, the phpstan file inside core already defines paths to analyze, which we would like to define on our own. The weird thing is, these paths don't seem to work properly as it is stated in https://phpstan.org/config-reference#analysed-files, that relative paths in the paths key are resolved based on the directory of the config file. This doesn't make sense as it would only lint core, but it also lints "web/modules/contrib"

joshsedl commented 1 year ago

The Drupal patch bot uses https://github.com/mglaman/phpstan-drupal, so we should create a phpstan.neon using level 2 and extend the rules from phpstan-drupal.

This way, the above-mentioned issue might be fixed as well, as we can put the file into our root directory.

joshsedl commented 1 year ago

Super weird... so Jenkins uses "sh ./core/scripts/dev/commit-code-check.sh" internally, which partially uses the phpstan.neon in core... which does NOT have the recomennded phpstan level 2....

I will ask the core team about this...