Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for running Psalm #140

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add support for running Psalm #140

wants to merge 1 commit into from

Conversation

joehoyle
Copy link
Member

@joehoyle joehoyle commented Jul 10, 2020

Add support for Psalm + Psalm Plugin WordPress. This is a bit different to other "linters" as this is more of a typechecker, but I think it fits the general paradigm well, and in project that use Psalm, we can at least run it in those contexts. In the future we might want to expand this to run a default config for things like taint analysis.

The major difference with Psalm is we need to install Composer dependencies as those are part of the type flow, and needed for Psalm scanning. At the moment I just installed production dependencies, however I'm not sure on the best route here. It wouldn't be uncommon to include your tests and the like in the Psalm config, in which case you're likely to also need the composer dev deps. I think it would be ok to not run Psalm against tests, but we'd need to somehow exclude them from any Psalm configuration.

Along with this, there's a couple other things needed that hm-linter doesn't directly include (maybe we should also address this at some point)

  • composer.phar needs to be pushed to s3://hm-linter/bin/composer
  • Base Psalm + Psalm-plugin-wordpress copied to s3://hm-linter/standards/psalm-x.zip
  • Some other rigging up to make sure the above are correctly downloaded, and Psalm is only run if you have a psalm.xml in the project

Add support for Psalm + Psalm Plugin WordPress. This is a bit different to other "linters" as this is more of a typechecker, but I think it fits the general paradigm well, and in project that use Psalm, we can at least run it in those contexts. In the future we might want to expand this to run a default config for things like taint analysis.

The major difference with Psalm is we need to install Composer dependencies as those are part of the type flow, and needed for Psalm scanning. At the moment I just installed production dependencies, however I'm not sure on the best route here. It wouldn't be uncommon to include your tests and the like in the Psalm config, in which case you're likely to also need the composer dev deps. I think it would be ok to not run Psalm against tests, but we'd need to somehow exclude them from any Psalm configuration.
@joehoyle joehoyle requested a review from rmccue July 10, 2020 08:26
@ntwb
Copy link
Member

ntwb commented Jul 15, 2020

Another consideration is to allow custom stubs, projects may have a stubs file for the project code.

This is also similar to being able to also add custom PHPCS rules sets not included in HM CS

So, having the ability for Composer to be able to install these custom deps per project would be great

Edit: Just realised this is a pull request and not an issue, the workaround for the above which would still be possible I think in the context of this PR would be a custom S3 deployment of HM CS, this was discussed in Slack last week, so don't worry about my comments here preventing this PR from being merged 😏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants