search

laminas / laminas-cli

Console command runner, exposing commands written in Laminas MVC and Mezzio components and applications
https://docs.laminas.dev/laminas-cli
BSD 3-Clause "New" or "Revised" License
55 stars 23 forks source link

Adds psalm as QA tool #39

Closed weierophinney closed 4 years ago

weierophinney commented 4 years ago

Adds Psalm as a QA tool, and provides:

From there, it applies changes based on the Psalm report. In many cases, this is in the form of using webmozart/assert to assert various truths about variable types and structure. In other cases, it means providing more specific types via either annotations or explicit typehints.

I've had to use @psalm-suppress a handful of times, either because we're using known deprecated classes (PackageVersions) or because an upstream library (symfony/console) is too vague in its own type hinting to allow either Psalm or us to be 100% accurate in type inference. Still other times are due to having had checks previously for things like file existence.

In a few cases, I extracted methods so I could provide more robust validations and verifications.

The majority of test changes were around exception types thrown (as I switched from generic exceptions to those thrown by webmozart/assert); as such, I feel the changes are fairly stable, and will allow us to be more stable going forwards.

Fixes #27

weierophinney commented 4 years ago

Relevant psalm results: https://travis-ci.com/github/laminas/laminas-cli/jobs/369535883#L508-L539

One note: I did not apply psalm to the test directory; an initial run showed I was going to need to segregate asset classes from unit tests, and re-do namespaces, most of which seemed like more effort than the gains were worth.

weierophinney commented 4 years ago

@Ocramius I've added unit tests to what psalm examines, and updated accordingly.