Added static analysis step to the CI.

[rupertj]

No description provided.

[finnlewis]

Thanks for this @rupertj

I understand from @willguv that we're aiming to bring localgov_publications into the default core localgov installation.

With that in mind, it would be good to be consistent in our testing across the repositories.

@millnut is currently working on setting up shared workflows, specifically with the aim of pulling in consistent workflows across the board, so it would be great to get input from @millnut on this pull request.

Also flagging for @stephen-cox too... should we be aiming to be consistent with https://github.com/localgovdrupal/localgov_project/blob/3.x/.github/workflows/test.yml in the short term?

Might we want to add some of these changes to localgov_project?

[millnut]

Hi @rupertj if you could update this to use the new shared workflow that would be great. I've added an example to the localgov/localgov_alert_banner.

I know on Slack we discussed the possibility of modules having a higher PHPStan level vs the default level and I'm all for this, especially with the Drupal Core plans to increase this level in future.

What I think would be good is creating a PR at https://github.com/localgovdrupal/localgov_shared_workflows to add an optional input for the workflow, something like use_module_phpstan_config or similar which would allow you to use a module-level config.

[stephen-cox]

As @millnut has mentioned, I think we're going to move all Github testing config to use https://github.com/localgovdrupal/localgov_shared_workflows/blob/1.x/.github/workflows/test-module.yml, so I suggest moving the change to the workflow there.

[rupertj]

Thanks all. I've moved this to use the new shared workflow, using the branch that I've made to it for this PR: localgovdrupal/localgov_shared_workflows#1

[millnut]

Thanks @rupertj I was thinking about this change the other day, if it's only changing level maybe we just allow it to use the project phpstan but pass the level in the phpstan command e.g. --level=5 and have a PHPSTAN_LEVEL optional input on the shared workflow. What do you think?

[rupertj]

@millnut The problem with just changing the level is going to be ignoring errors. IME as you turn up the error reporting level, you end up having to ignore more things. The errors you get at lower levels are unambiguous things that you just fix because they're obviously wrong. Some of the higher level stuff might be stuff you either can't fix or don't want to fix, so you have to add your own ignore rules for it.

I've had a look around d.o for examples, and the best one I've found is the one from search_api: https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/phpstan.neon?ref_type=heads

I did wonder if we could set the level and then use the baseline feature to let our modules suppress errors, but it seems it's impossible to set the baseline via an argument to phpstan. Baselines can only be set via the phpstan.neon. This also made me realise that if people want to use baselines, they'll need to have their own phpstan.neon too.

[millnut]

Hi @rupertj added a comment to localgovdrupal/localgov_shared_workflows#1 (comment) but I agree with your use cases mentioned