localgovdrupal / localgov_project

Project template for Drupal 10 sites built with the LocalGov Drupal distribution.
https://localgovdrupal.org
GNU General Public License v2.0
10 stars 8 forks source link

PHPStan deprecation tests fail. #144

Open rupertj opened 9 months ago

rupertj commented 9 months ago

I'm currently getting test failures on localgov_publications: https://github.com/localgovdrupal/localgov_publications/actions/runs/6853042624/job/18633021243

I've seen it on localgov_forms too, so this could be widespread: https://github.com/localgovdrupal/localgov_forms/actions/runs/6852604081/job/18631575333

I've tracked the error to localgov_project.

To recreate this:

Gives the output:

Note: Using configuration file /Users/rupert/Repos/localgov_project/phpstan.neon.
PHP Fatal error:  Cannot declare class MyEventSubscriber, because the name is already in use in /Users/rupert/Repos/localgov_project/web/modules/contrib/scheduled_transitions/scheduled_transitions.api.php on line 27
Fatal error: Cannot declare class MyEventSubscriber, because the name is already in use in /Users/rupert/Repos/localgov_project/web/modules/contrib/scheduled_transitions/scheduled_transitions.api.php on line 27

This is caused by there also being an api.php file in date_recur that uses the same example classname:

localgov_project % grep -rin MyEventSubscriber *
web/modules/contrib/scheduled_transitions/scheduled_transitions.api.php:21: *    class: Drupal\my_module\EventSubscriber\MyEventSubscriber
web/modules/contrib/scheduled_transitions/scheduled_transitions.api.php:27:class MyEventSubscriber implements \Symfony\Component\EventDispatcher\EventSubscriberInterface {
web/modules/contrib/date_recur/date_recur.api.php:26: *    class: Drupal\my_module\EventSubscriber\MyEventSubscriber
web/modules/contrib/date_recur/date_recur.api.php:32:class MyEventSubscriber implements \Symfony\Component\EventDispatcher\EventSubscriberInterface {

I've tried to exclude *.api.php from the files that phpstan looks at, but adding this to the phpstan.neon had no effect at all.

parameters:
    excludePaths:
        - *api.php
millnut commented 9 months ago

Hi @rupertj thanks for the ticket; this is a valid error as classes shouldn't have the same name I'll submit a ticket against the date_recur module as it is the more active one to update their class naming so it is unique as they both look like they have copied from a template/example and have not namespaced their code.

In the meantime I'll look at what excluding I can add to PHPStan itself.

millnut commented 9 months ago

Hi @rupertj doing some further debugging this looks like it is caused by an update of the package mglaman/phpstan-drupal. The last successful pipeline used version 1.2.2 and the failed one used 1.2.3.

This ticket covers the issue we are seeing here https://github.com/mglaman/phpstan-drupal/issues/640

rupertj commented 9 months ago

Cheers @millnut - good find! I was planning on filing issues against both modules to suggest adding a namespace to their examples, as that'd make them better examples anyway. But if you're up for it, go for it :)

millnut commented 9 months ago

@rupertj this is now fixed in mglaman/phpstan-drupal (1.2.4), the pipeline that ran today is now green https://github.com/localgovdrupal/localgov_project/actions/runs/6874041455

rupertj commented 9 months ago

Thanks for the update! I had to delete a runner cache to get my pipeline to run with the newer version of phpstan-drupal, but they're good now.