Automated code and style checks #338

New issue

Open

opened 2020-08-19 11:00:25 +00:00 by matthijskooijman · 2 comments

matthijskooijman commented

2020-08-19 11:00:25 +00:00

(Migrated from github.com)

Previously, I wrote:

One type of testing is static analysis or linting, where no code is actually run, but a script looks at the code and checks for common mistakes. There's roughly two things that these scripts can check:

Code formatting: Whether the code complies with certain style guidelines
Bad practices: Certain constructs or bits of code that are considered bad practice
Code checks: Whether the code contains mistakes (e.g. references to undefined functions, variables, etc.)
Type checks: Whether the types of expressions are correct (e.g. not passing a string where an int is expected). This is something that's enabled by recent improvements in PHP where variables and parameters can be annotated by their type, though often types can be derived as well.

I've looked at a few of these analyzers:

Psalm is a fairly new one, that seems to focus on code and type checks. It is quite configurable with different strictness levels, not sure if it's easy to add your own checks. It supports a "baseline file", to exclude any existing problems making it easier to gradually improve by (somewhat) only scanning newly added code.
PHPCS (code sniffer) is a scanner for (exclusively, it seems) code formatting. It does seem easy to add specify a customized style, reusing existing "sniffs" and adding your own. It also supports automatically fixing most issues (though that might end up producing a mess, of course).
PHPMD (mess detector) seems to scan mostly for bad practices.
Note that this is all different from actual testing, where you run custom-written (Hypha-specific) testing code that actually runs Hypha code and evaluates whether the output is as expected.
PHPSTAN (static analysis) mostly does code checks (and maybe some limited type checks). It is quite configurable with different strictness levels, custom checks also seem to be possible. Also supports a "baseline file".

It seems that Psalm might have the most extensive code and type checking. PHPSTAN seems a bit less complete, but has nice output and can be easily integrated into my editor (vim, using syntastic), so I'll probably be enabling PHPSTAN and maybe see how much issues it misses that psalm ort PHPMD find. We could maybe add PHPCS for code formatting checks, once we decide what we want in this area.

This is split off #132, so that issue can remain about actual (unit) tests that actually run Hypha code, and this issue can be about automatic code (style) checks (i.e. static analysis) using third-party tools. Previously, I wrote: One type of testing is static analysis or linting, where no code is actually run, but a script looks at the code and checks for common mistakes. There's roughly two things that these scripts can check: 1. Code formatting: Whether the code complies with certain style guidelines 2. Bad practices: Certain constructs or bits of code that are considered bad practice 3. Code checks: Whether the code contains mistakes (e.g. references to undefined functions, variables, etc.) 4. Type checks: Whether the types of expressions are correct (e.g. not passing a string where an int is expected). This is something that's enabled by recent improvements in PHP where variables and parameters can be annotated by their type, though often types can be derived as well. I've looked at a few of these analyzers: - [Psalm](https://psalm.dev) is a fairly new one, that seems to focus on code and type checks. It is quite configurable with different strictness levels, not sure if it's easy to add your own checks. It supports a "baseline file", to exclude any existing problems making it easier to gradually improve by (somewhat) only scanning newly added code. - [PHPCS](https://github.com/squizlabs/PHP_CodeSniffer) (code sniffer) is a scanner for (exclusively, it seems) code formatting. It does seem easy to add specify a customized style, reusing existing "sniffs" and adding your own. It also supports automatically fixing most issues (though that might end up producing a mess, of course). - [PHPMD](https://phpmd.org/) (mess detector) seems to scan mostly for bad practices. Note that this is all different from actual testing, where you run custom-written (Hypha-specific) testing code that actually runs Hypha code and evaluates whether the output is as expected. - [PHPSTAN](https://phpstan.org) (static analysis) mostly does code checks (and maybe some limited type checks). It is quite configurable with different strictness levels, custom checks also seem to be possible. Also supports a "baseline file". It seems that Psalm might have the most extensive code and type checking. PHPSTAN seems a bit less complete, but has nice output and can be easily integrated into my editor (vim, using syntastic), so I'll probably be enabling PHPSTAN and maybe see how much issues it misses that psalm ort PHPMD find. We could maybe add PHPCS for code formatting checks, once we decide what we want in this area.

matthijskooijman commented

2020-08-19 11:20:29 +00:00

(Migrated from github.com)

Given that Psalm also seemed to have more extensive checks, turns out to be a lot faster (<1s for a single file), supports checking single files and integrating it into my editor turned out to be rather easy, I've now switched to psalm, which has already caught quite a few typos and other small oversights in the code I was writing.

Good next steps could be to fix the remaining errors, or establish a baseline (but there's only 64 errors at level 5, most of which are probably problems with psalm not seeing all code properly rather than actual bugs, so fixing is probably better) and then enable automatic checking of all code for all pullrequests and pushes.

From there, we could try increasing the strictness (https://psalm.dev/docs/running_psalm/error_levels/) and fix more of those issues and/or establish a baseline again, so new code can be subjected to more checks.

Eventually, I'd like more strict checks for missing type annotations, but that probably requires a newer PHP version so we can actually use native annotions rather than resorting to comments for a lot of annotations (See #302).

I've tried PHPSTAN for a bit, but it is not ideal. I ran into some problems running it from my editor (it is apparently not intended to run for one file, but should be ran on the entire project). It is also quite slow (12s to run on the entire project, 4s for one file). Given that Psalm also seemed to have more extensive checks, turns out to be a lot faster (<1s for a single file), supports checking single files and integrating it into my editor turned out to be rather easy, I've now switched to psalm, which has already caught quite a few typos and other small oversights in the code I was writing. Good next steps could be to fix the remaining errors, or establish a baseline (but there's only 64 errors at level 5, most of which are probably problems with psalm not seeing all code properly rather than actual bugs, so fixing is probably better) and then enable automatic checking of all code for all pullrequests and pushes. From there, we could try increasing the strictness (https://psalm.dev/docs/running_psalm/error_levels/) and fix more of those issues and/or establish a baseline again, so new code can be subjected to more checks. Eventually, I'd like more strict checks for missing type annotations, but that probably requires a newer PHP version so we can actually use native annotions rather than resorting to comments for a lot of annotations (See #302).

matthijskooijman commented

2020-08-24 10:36:08 +00:00

(Migrated from github.com)

If we apply a style checker, we might want to include checks for consistent naming (e.g. #324).

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

harmen/hypha#338

No description provided.

Rows
Columns