PHP 8.2 deprecations

Ok, I just found out that the error why I created ( 🐛 Dynamic property error on PHP 8.2 Fixed ) has been fixed in 1.2.0. But I suggest to keep this to run static code analysis for other deprecations.

The dynamic properties are an accident - they should have a declaration. However I develop on PHP8.1 so I don't see a warning.

In the page you linked to it says

It is possible to programmatically emit a deprecation in older versions in the interest of consistency across multiple PHP versions.

I'd be very happy with that if you know how to do it.

I didn't look on this issue for a while, but unfortunatley don't exactly know what it meant by

It is possible to programmatically emit a deprecation in older versions in the interest of consistency across multiple PHP versions.

PHPStan though is able to detect these issues, as I just got another one

Deprecated function: Creation of dynamic property Drupal\symfony_mailer\Processor\EmailBuilderManager::$proxy is deprecated in Drupal\symfony_mailer\Processor\EmailBuilderManager->getProxyMapping() (line 204 of modules/contrib/symfony_mailer/src/Processor/EmailBuilderManager.php).

which by PHPStan is reported as

 ------ ---------------------------------------------------------------------------------------------- 
  Line   src/Processor/EmailBuilderManager.php                                                         
 ------ ---------------------------------------------------------------------------------------------- 
  204    Access to an undefined property Drupal\symfony_mailer\Processor\EmailBuilderManager::$proxy.  
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property          
 ------ ---------------------------------------------------------------------------------------------- 

Find the full PHPStan report of the symfony_mailer 1.2.1 attached for reference (as it is too long to post it directly in the comment).

Thanks. Do you know if we could include phpstan in the automatic tests for this module? As far as I can see, mglaman/phpstan-drupal is required by D10 core, but not D9.

Looking at phpstan.symfony_mailer.txt I found 4 actual minor bugs, and another maybe D10 bug (MailerPolicyListBuilder). So that's good to know.

The rest are in various categories

1. Missing definitions from external code
2. In theory the warning is true, but in practice it never is
3. "\Drupal calls should be avoided in classes" which is a reasonable principle but also can cause BC problems and lots of extra code
4. Limitation/bug of the analysis tool (MailManagerReplacement)

Possibly in some cases the code could be altered/improved to make the warning go away. So the tool might be useful, but it will require some work.

Which warning exactly do you mean with #2? And #3 btw doesn't get triggered by static methods.

Which warning exactly do you mean with #2

59 Variable $messages might not be defined.
38 Variable $addresses might not be defined.
Etc. Static analysis notices that the loop might never be executed, however I believe the arrays cannot be empty. However I guess it's easy enough to add the code that will fix the warning.

#3 btw doesn't get triggered by static methods.

I am aware of that. IMO dependency injection adds a lot of tedious "boiler plate" code, even 20-30 lines just to make use of a single function from a service. Also if you want to call a new service from an existing class, it's not fully BC, as it changes the constructor.

If the #3 warning should cause problems (it would block CI if you include phpstan in the automatic tests) this could be a workaround:

class MyService {

  protected static function getMessenger() {
    return \Drupal::messenger();
  }
}

I made the method protected to help with testing, this way it can be overridden to provide a mocked service instance easily.

59 Variable $messages might not be defined.

$messages is assigned inside a nested if statement inside the foreach loop, so the fix is very easy. Just add $messages = []; directly before the foreach loop.

if you include phpstan in the automatic tests

Is this possible to do?

I made the method protected to help with testing, this way it can be overridden to provide a mocked service instance easily.

Neat.

I think new gitlab testing catches this now