Tests for docblock quality

[johnbillion]

Attached is a patch which implements various tests for docblocks in PHP files. The patch is experimental and is intended only to determine whether unit tests for docblocks are a good idea or not.

Currently tested for each function/method:

• Existence of docblock.
• Non-emptiness of docblock.
• Correct number of @param tags.

Currently tested for each parameter of a function/method:

• Non-emptiness of description.
• Name matches actual parameter name.
• Various type hint validations (not so appropriate for core).
• Existence or not of "Optional" string.
• Existence or not of "Default" string.

Currently this only tests functions/classes which are loaded in the current scope. ​phpDocumentor's static reflection library allows static analysis of files instead of using PHP's built-in Reflection API, which would allow for all functions and classes to be tested.

cc @DrewAPicture

[johnbillion]

Usage:

1. Adjust the $to_search, $files, $test_functions, and $test_classes variables in Tests_DocBlocks::dataReflectionTestFunctions() as appropriate.
2. Run phpunit --filter Tests_DocBlocks.
3. Observe many failures.

[johnbillion]

The phpDocumentor library that's used is PHP 5.3+, so these tests would need some logic so they only run on PHP 5.3+.

[joehoyle]

Is this not the job of some kind of code sniffer rather than unit testing?

[johnbillion]

Replying to joehoyle:

Is this not the job of some kind of code sniffer rather than unit testing?

Potentially, yes, but we don't have an infrastructure for this currently. ​WPCS could be used, but there's nothing in there for docblock quality and it would have to be run separately from the PHP rules in WPCS because WordPress itself (naturally) doesn't yet adhere to its standards.

[tw2113]

What about collecting data for a report regarding file location and function name to ease finding where one would need to work on?

[jdgrimes]

Replying to johnbillion:

Replying to joehoyle:

Is this not the job of some kind of code sniffer rather than unit testing?

Potentially, yes, but we don't have an infrastructure for this currently. ​WPCS could be used, but there's nothing in there for docblock quality and it would have to be run separately from the PHP rules in WPCS because WordPress itself (naturally) doesn't yet adhere to its standards.

This is actually incorrect. WPCS includes the WordPress-Docs ruleset for checking docblock quality (thanks to @GaryJ). So you can run phpcs --standard=WordPress-Docs and just check for docblock issues. It is possible that the rules won't match exactly what WordPress core expects, but PRs are welcome :-).

[GaryJ]

The rules that exist in WordPress-Docs were just the easy wins that utilised existing Sniffs that are part of existing standards that ship with PHPCS (i.e. Squiz and Generic) - see ​https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/WordPress-Docs/ruleset.xml .

This is in no-way comprehensive, and doesn't check for many WP idiosyncrasies. The ruleset.xml in fact is mainly made up of exclusions from those more typical existing standards, and would therefore need custom Sniffs to add those valid checks in.

[johnbillion]

docblocks.2.diff​ switches to static analysis instead of PHP's built-in Reflection, so classes and functions that are not loaded in the current scope can be tested too. Needs Composer to set up before running the tests:

1. cd tests/phpunit
2. composer install

See the $to_search and $files variables in the Tests_DocBlocks::dataReflectionTestFunctions() method for tweaks that can be made, and also the $ignore_empty static property on the class which silences errors caused by functions and methods which completely lack a docblock.

At this point I think it's clear that such tests don't need to be in core's tests, but having them allows the docs team to work through the issues that this patch does highlight.

See #32246 for ongoing results.

[johnbillion]

docblocks.3.diff​ corrects the composer.json file.

[morganestes]

@johnbillion anything folks can do to help you on this one?

[johnbillion]

I have a standalone repo for this: ​https://github.com/johnbillion/php-docs-standards and I was planning a WordPress specific set of rules based on the above.

Seeing as the static analyses approach requires a Composer dependency, maybe we can close this ticket as maybelater, or we can switch to using WPCS for docblock quality sniffing.

[johnbillion]

This was fun, but it's really the domain of WPCS.

​https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/424