Skip to content

Use Psalm for static code analysis #1764

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

Open
wants to merge 35 commits into
base: dev
Choose a base branch
from

Conversation

demiankatz
Copy link
Member

@demiankatz demiankatz commented Oct 24, 2020

Psalm is a static code analysis tool that we can add to our toolkit for improving VuFind's code quality. This PR introduces it.

TODO:

@demiankatz
Copy link
Member Author

I spent some more time today bringing this up to date and trying to resolve issues. One of the most prevalent errors is that Psalm doesn't like factories stating @throws ContainerInterface because ContainerInterface does not implement Throwable. I think it should be possible to overcome this with an intersection type -- see #1923 -- but this unfortunately does not work, due to this apparent known bug in Psalm: vimeo/psalm#3621

We should revisit this in the future and see if the bug is fixed.

@demiankatz
Copy link
Member Author

I'm flagging this as "on hold" until vimeo/psalm#3621 is resolved, since we currently can't get the error list to clear out without intersection types working properly.

@demiankatz
Copy link
Member Author

I took a few minutes today to bring this up to date. I think the existing problem with union types remains unresolved, but I thought it might be helpful to get this integrated with GitHub Actions, etc.

@demiankatz
Copy link
Member Author

demiankatz commented Feb 28, 2024

I've managed to get the build to pass here by suppressing several of the false-positive errors that have been causing problems. Perhaps the best approach for moving forward here is to either suppress or resolve all remaining errors until we have a clean baseline. Then we can re-evaluate the suppressed checks to see which ones are completely useless and which ones might be valuable in combination with appropriate code annotations. I can already see that some of the issues Psalm can detect and fix will lead to improvements in code quality, so it seems worth getting some of those benefits as soon as we can, even if it means not taking full advantage of everything Psalm has to offer right away.

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

Successfully merging this pull request may close these issues.

1 participant