chore: add phpstan strict type checking

[millnut]

Additional context here: localgovdrupal/localgov#685

1. Imported bleedingEdge rules to match Drupal core (https://git.drupalcode.org/project/drupal/-/blob/10.3.x/core/phpstan.neon.dist?ref_type=heads#L5)
2. Set level to 1 to match Drupal core (https://git.drupalcode.org/project/drupal/-/blob/10.3.x/core/phpstan.neon.dist?ref_type=heads#L9)
3. Added phpstan/phpstan-strict-rules
4. Updated phpstan.neon to include rule checkFunctionArgumentTypes as per https://www.drupal.org/project/drupal/issues/3404246
5. Turned off all strict rules from phpstan/phpstan-strict-rules except strictCalls
6. Set to mirror the results from drupal/upgrade_status
7. Rename Deprecated check on workflow to PHPStan

[rupertj]

Looks good to me. The errors saying

Class                                                                                     
         Drupal\localgov_step_by_step\Plugin\PreviewLinkAutopopulate\StepBySteps                   
         extends unknown class                                                                     
         Drupal\preview_link\PreviewLinkAutopopulatePluginBase.     

looked like they could be a failure to find all the classes in contrib, but that's the only instance of that kind of thing, and that class is relatively new, so I think it might be a problem with step_by_step rather than the CI setup.

[stephen-cox]

Class                                                                                     
         Drupal\localgov_step_by_step\Plugin\PreviewLinkAutopopulate\StepBySteps                   
         extends unknown class                                                                     
         Drupal\preview_link\PreviewLinkAutopopulatePluginBase.     

looked like they could be a failure to find all the classes in contrib, but that's the only instance of that kind of thing, and that class is relatively new, so I think it might be a problem with step_by_step rather than the CI setup.

This is because we have included some plugins that are defined in a patch on the Preview Link module, but we haven't release the code pulling in the patch because of test failures: localgovdrupal/localgov#730 (comment)

Just looking at potential fixes for this.

[millnut]

Think this is ready to go now.

I've created a PHPStan baseline to ignore the remaining errors for now so we can fix them in future. The reason for this is we are getting more people contributing and this feels more important to get in now so it runs on the workflows and for future code contributions.

[stephen-cox]

Largely looks fine. Just a couple of comments.

In Microsites we have ignored some Drupal 12 deprecations until we can drop support versions of Drupal that don't support the changes. See https://github.com/localgovdrupal/localgov_microsites_project/blob/4.x/phpstan.neon. There's a draft PR adding these with #174. Do we want to include this here?

Is the plan to create issues for all the ignored rules in each repo so we can fix them and then remove the exceptions here?

[millnut]

Yep that makes sense @stephen-cox I've updated the PR to include ignoring Drupal 12 deprecations.

Also correct on the second point, once merged I plan to create issue tickets for everything in the baseline and tag with "good first issue" or similar so over time we can chip away at that file and then remove it.

EDIT: Looks like I need to regenerate the baseline https://github.com/localgovdrupal/localgov_project/actions/runs/10217264291/job/28270808622, leave this PR with me until completed

[millnut]

Hi @stephen-cox phpstan baseline updated, test failures unrelated

[finnlewis]

It looks like the pull request from @MattOz-CDS here inludes some of the same lines for Drupal 12 deprecations: https://github.com/localgovdrupal/localgov_project/pull/171/files

I note that in that pull request, it mentions renderPlain twice, and specifies the interface and class, where as here we just mention it once and don't specifiy the interface / class.

    - '#Call to deprecated method renderPlain\(\) of interface Drupal\\Core\\Render\\RendererInterface#'
    - '#Call to deprecated method renderPlain\(\) of class Drupal\\Core\\Render\\Renderer#'

Is it just less specific to avoid mention of interface or class?

[millnut]

Is it just less specific to avoid mention of interface or class?

@finnlewis yes when I tested recently the message had changed slightly so it wasn't ignoring, this makes it less specific but still allows for it to be ignored