Test documentation standards

[ScreamingDev]

Please add tests for documentation standards:
https://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/

[GaryJones]

Not sure if we'd test with tokens or with Reflection. If the latter, WP-Parser may have code we could use, cc @Rarst.

[Rarst]

This is not the parser you are looking for. waves hand

[JDGrimes]

PHPDocumentor uses reflection, so if you are going to leverage its code, I think you'll have to use reflection. I'm doubtful you'd find anything helpful out of WP-Parser at the moment though.

[JDGrimes]

On second thought, it would be great to leverage WP-Parser's exporter, but I think it's still entangled a bit with WP-CLI, so it might take a bit of work. It would parse hook docs for you though, which is something PHPDocumentor isn't gonna do on its own.

[GaryJones]

Anyone happen to know if individual tokens might actually exist inside a comment block? I'm guessing not...

[JDGrimes]

Anyone happen to know if individual tokens might actually exist inside a comment block? I'm guessing not...

No, they don't.

[JDGrimes]

If you want to take a whack at leveraging WP-Parser, take a look at the parse_files() function. You could use that to parse the file's docs into an array, which you could check for the expected info. (The function parses some other information as well, like what functions are used, but you can just ignore that part of the returned data.)

[ScreamingDev]

Please approve this gist (test for @since): https://gist.github.com/sourcerer-mike/38f9984d559a5a147023

It is copied from PEAR Coding Standard and if you agree I help develop in that way. Please note that the comments in those files are not revised and it only tests for @since.

@GaryJones

Anyone happen to know if individual tokens might actually exist inside a comment block? I'm guessing not...

If you mean something like "T_DOC_COMMENT_OPEN_TAG" then the answer is yes: https://pear.php.net/package/PHP_CodeSniffer/docs/latest/PHP_CodeSniffer/_PHP_CodeSniffer-2.0.0RC1---CodeSniffer---Tokens.php.html

[GaryJones]

Please note that the comments in those files are not revised and it only tests for @SInCE

If it could test for all it mentions (except the A return type exists, since WP does follow that), then it would be a great start.

Getting ahead of ourselves, I'd foresee this as being a WordPress-Docs ruleset, since I could imagine that folks would want to test documentation standards separately from non-comment code quality.

Updated: Actually, I disagree with that. Documentation is a vital part of coding, it's a standard on make.wp.org, so we should be encouraging it as part of the WordPress-Core / Extra standards like non-comment standards.

[GaryJones]

https://github.com/squizlabs/PHP_CodeSniffer/blob/phpcs-fixer/CodeSniffer/Standards/Generic/Sniffs/Commenting/DocCommentSniff.php

It's not quite perfect for WP, but it's an extremely good starting point.

[GaryJones]

See https://github.com/squizlabs/PHP_CodeSniffer/tree/phpcs-fixer/CodeSniffer/Standards/Squiz/Sniffs/Commenting
and
https://github.com/squizlabs/PHP_CodeSniffer/tree/phpcs-fixer/CodeSniffer/Standards/Generic/Sniffs/Commenting

Some will need WP variants as the Squiz and Generic ones don't quite match what we want, or we need to use the originals but with severity = 0 so it's not reported.

[westonruter]

See @Rarst's tweet in regards to incorporating the Squiz.Commenting sniffs: https://twitter.com/rarst/status/567237335495221248

[GaryJones]

I'm going to change my mind again. I think a WordPress-Docs ruleset is best, but include that in the overall WordPress ruleset, so that it's still on by default.

[GaryJones]

I've started https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/compare/temp/241-commenting?expand=1

Lots of exclusions from the default Squiz.Commenting, and not looked to see if Generic might be better.

Plenty of cases where we're going to need to write custom sniffs, if there aren't parameters to tweak what we need.

I've created a quick test file and asked the Docs channel on Slack to see if this is right so we can check the perfect case doesn't generate false positives.

[GaryJones]

I'd like to whitelist which messages are reported, but this isn't possible - we can only include rules, and then exclude messages - but we can include the overall Squiz.Commenting and then still exclude messages, as already done in the branch.

[JDGrimes]

I'd like to whitelist which messages are reported, but this isn't possible

See squizlabs/PHP_CodeSniffer#586.

[JDGrimes]

@GaryJones I think this can be closed now that WordPress-Docs ruleset was added in #381?

[GaryJones]

No @JDGrimes - that was the first pass / easy wins. There are still plenty of aspects of comments / inline docs that are not yet tested, so I'd like to keep this open.

Alternatively, it can be closed, and a new one opened if that makes sense for the release paper trail.

[mehtryx]

Just wanted to comment, as it resulted in us changing how we used the WordPress coding standards.

According to here: https://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/

Wordpress makes an attempt to adopt PSR-5 standards, but they are not intending it to be PSR-5 compliant. Further, the language of the drafted standards here use SHOULD and RECOMMENDED throughout, but this changes the intent to almost a MUST.

While I agree that these SHOULD be present, I'm also answering to developers that are working to ensure we meet the standards they MUST have according to the Wordpress standards defined. Do you have special insight into this process that suggests WordPress standards will enforce the need for this, as we have had none.

My thought is, these should not be listed as errors when they are merely recommended and should be present but not required. (It pains me to say that as I would rather they be required!)

It makes it complicated when we have a standard being enforced as the "WordPress" standard which doesn't actually match "WordPress' standard".

[GaryJones]

No coding standards is a MUST, certainly not one from created from a community, instead of internal company decisions. As such, it makes all the suggestions optional.

While the language in PSR-5 is a little forgiving, the coding standards in the WP Handbook are less so, and that's what the WPCS sniffs try to emulate.There's no special insight here - the choice to have inline comments and documentation in a certain way is no different than the choice of naming things or use of whitespace in other parts of the handbook. If you're company decision is to take all of them at the SHOULD level instead of MUST, then that's your right to do so.

cc @DrewAPicture

[mehtryx]

Sorry, I guess what I meant is prior to #381 Wordpress-Docs was not included in the Wordpress ruleset, thus in the past we were not forced to exclude tests. This is what I meant by "Must" versus "Should", if it is optional, and having sent lots of code to VIP the commenting is pretty optional, then why make those checks part of a basic Wordpress ruleset by default?

Certainly we work around it...by adding exclusions all throughout our code for something that we previously did not have to exclude.

[JDGrimes]

Sorry, I guess what I meant is prior to #381 Wordpress-Docs was not included in the Wordpress ruleset, thus in the past we were not forced to exclude tests. This is what I meant by "Must" versus "Should", if it is optional, and having sent lots of code to VIP the commenting is pretty optional, then why make those checks part of a basic Wordpress ruleset by default?

If you are checking code for VIP, you might just want to use the WordPress-VIP standard instead of WordPress. The VIP standard doesn't include the WordPress-Docs ruleset.

Certainly we work around it...by adding exclusions all throughout our code for something that we previously did not have to exclude.

If you still want to use the whole WordPress standard instead of just the VIP rules, you can easily configure it without placing any exclusions in your code at all. Just create a custom ruleset.xml file that excludes WordPress-Docs. Something like this:

<?xml version="1.0"?>
<ruleset name="WordPress-Custom">
    <description>Custom WordPress Coding Standards</description>

    <rule ref="WordPress"/>
    <exclude ref="WordPress-Docs"/>
</ruleset>

[mehtryx]

Thanks.

[JDGrimes]

I sort of implied this in passing above, but we should sniff hook docs as well. I think that will require us to write some custom sniffs, though.

[GaryJones]

It will - this ticket was only ever about re-using existing sniffs that match what WP wants already. There's plenty of aspects of non-hook DocBlocks that are not tested yet.

[JDGrimes]

OK, I've make a separate ticket: #424

[danielbachhuber]

I'm not sure where this is at in getting over the finish line, but I'd be happy to put some time towards it. I think this would be a huge benefit to the community.

[westonruter]

I think the work needing to be done for this is to create a test suite of code that uses the WordPress documentation standards, and then to run PHPCS with WordPress-Docs against this suite to then ensure that all of the expected errors and warnings are accounted for. There are also unique features of the WordPress documentation standards that need sniffs, specifically regarding hooks (#424). Up to now, we've only been re-using existing sniffs from upstream. So we also need a list of which features of WordPress documentation standards do have upstream sniffs we can re-use, and list out which features we need to create new sniffs for. This would be a large effort, but it would indeed be very valuable.