[Docs] Persnickety Whitespace Errors

[ghost]

Given the following doc block and method function:

/**
 * Moonwalks an array.
 *
 * Deduplicates multidimensional array by flattening it while preserving
 * the deepest depth and flipping it inside out. Array values become the
 * keys and the new values contain the depths.
 *
 * @since Hyperdrive 1.0.0
 * @license ISC
 * @link https://gist.github.com/jhabdas/2c805d3c9f9abe584ba22c5b5e35b9a3
 * @link https://stackoverflow.com/q/44304599/712334
 * @param array $array A multidimensional array of variable depth.
 * @param array $accumulator A reference identifier for a stored result.
 * @param integer [$depth = 1] Starting depth for array search.
 * @param boolean [$recursing = false] True while doing a moonwalk.
 * @return A flattened, deduplicated array with leaf node values as keys
 *     and deepest depth of any given leaf as the value.
 */
function array_moonwalk( $array, &$accumulator, &$depth = 1, $recursing = false ) {
    array_walk( $array, function ( $element ) use ( &$accumulator, &$depth, $recursing ) {
        is_array( $element )
            ? ++$depth && array_moonwalk( $element, $accumulator, $depth, true )
            : $accumulator[ $element ] = $depth;
    });
    $recursing && --$depth;
}

WordPress-Docs PHPCS ruleset throws the following:

 204 | ERROR | [x] Expected 5 spaces after parameter type; 1 found
 205 | ERROR | [x] Expected 5 spaces after parameter type; 1 found
 206 | ERROR | [x] Expected 1 spaces after parameter type; 0 found
 207 | ERROR | [x] Expected 1 spaces after parameter type; 0 found

Because these sniffs would fail a build under the sample plugin generated by WP-CLI I've configured the generated PHPCS matrix cell to allow failure in my CI setup.

I'm aware I could override these rules, but I wanted to raise this issue as, after digging through the Git repos, neither CodeSniffer nor PSR doc seem to cover the use case whereby one can clearly document default value assignments (206, 207) and I'm curious why a trivial whitespace formatting snafu would be ERROR level and not a WARN (204, 205). Thanks for your thoughts.

[jrfnl]

These documentation rules, as well as how to indicate a default value (in the param description, not the way you are doing it) are documented in the WP Documentation Standards. WPCS just checks what the standard has defined. We don't make the rules.

[GaryJones]

Try the following:

/**
 * Moonwalks an array.
 *
 * Deduplicates multidimensional array by flattening it while preserving
 * the deepest depth and flipping it inside out. Array values become the
 * keys and the new values contain the depths.
 *
 * @since Hyperdrive 1.0.0
 *
 * @license ISC
 * @link https://gist.github.com/jhabdas/2c805d3c9f9abe584ba22c5b5e35b9a3
 * @link https://stackoverflow.com/q/44304599/712334
 *
 * @param array   $array       A multidimensional array of variable depth.
 * @param array   $accumulator A reference identifier for a stored result.
 * @param integer $depth       Optional. Starting depth for array search. Default is 1.
 * @param boolean $recursing   Optional. True while doing a moonwalk. Default is false.
 */

[ghost]

Thanks Gary. I'll give that a shot at least on the plug-in portion of my app as I'd like to start layering on more rules until I hit VIP ruleset. Ideally there'd be a programmatic way to define defaults and optional params syntactically as it seems like a pretty common occurrence. Was surprised not to see anything in the FIG Standards or in Sniffer repo issues. Weird.

Parting thoughts. The above pattern helps readability while looking at code but causes sidescrolling or piling comments as soon as parms get complex. And the vertical alignment causes more lines of code to be changed when method signatures change as a result of the need to re-align things vertically which are unrelated to the change at hand, introducing the potential for bugs.

I know you guys have your standards for a reason, so I'm going to just go with the flow here and define my own standards elsewhere in my plugin-generating app. Cheers to you @GaryJones

[GaryJones]

@jhabdas As @jrfnl said, this repo is predominantly about implementing the standards defined for WordPress. In this case, take a look at https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#1-functions-class-methods which shows the vertical and horizontal whitespace in action.

My example, is just to show you how your code can meet that.

Ideally there'd be a programmatic way to define defaults and optional params syntactically as it seems like a pretty common occurrence.

You can of course choose which parts of the standards to follow, via a phpcs.xml.dist file. Feel free to exclude some or all of the WordPress-Docs you don't like. But remember, the purpose of a standard to exist, is to that it is followed, even if you don't like the output, for the real benefits. Applying only parts of it somewhat misses the point, unless you have good reason. (In my case, I turn off class file-name requirements, as the benefit of PSR-4 autoloading is more useful to cleaner code, than certain files named in a certain way.)

You may also like to look at https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/wiki/Customizable-sniff-properties which shows how you can further customise your phpcs.xml.dist. Note that some bits are for 0.12.0, which is not released yet, but is available in the develop branch.

[ghost]

Thanks Gary, and Juliette. I look forward to reviewing more docs and gaining a better understanding over time. Coming from a JS background I instinctively wrote my plugin using procedural programming and pure functions where possible, with 2 space indentation to help limit rightward code drift. I've since switched to tabs for spaces and it's not bad for me if it looks good to someone else. Tests are written in Kahlan, and definitely will continue to use 2 spaces to keep the code tight and easy to grok. But I'll keep your ideas and more of what I read in mind as I continue to internalize all this exciting new stuff. Cheers.

[ghost]

Also, for the casual onlooker, I wanted to provide a sample of what a JSDoc block might look like for the sake of comparison between languages so as to help contextualize the basis for some of the broader questions raised in this issue (which are really more applicable to PHPDoc than anything):

/**
 * Fetch Inject module.
 *
 * @module fetchInject
 * @param {(USVString[]|Request[])} inputs Resources you wish to fetch.
 * @param {Promise} [promise] A promise to await before attempting injection.
 * @exception {Promise<Error>} Invalid arguments and anything Fetch throws.
 * @returns {Promise<Array<Object>>} Promise that resolves to an Array of
 *     Objects containing Response Body properties used by the Module.
 */

[GaryJones]

@jhabdas While admirable, your efforts to influence Wordpress coding standards for documentation would like to trying to turn a whole fleet of Titantics around. WordPress core, plus an immeasurable number of plugins and themes are already doing it in a certain way, as highlighted in the standards you've already been linked to twice. The fact you might not like it, and prefer aspects of JSDoc that you're familiar with, makes no difference whatsoever (and therefore makes your comment redundant).

Again, this repo is just adding a programmatic way of testing PHP code against those PHP standards - we don't have any influence on amending what those standards are.

Now, there is a page in the core handbook about JavaScript Documentation Standards, though, of course, that's for documenting JavaScript, and not PHP, which is what this repo here is for.

[ghost]

While admirable, your efforts to influence Wordpress coding standards for documentation would like to trying to turn a whole fleet of Titantics around.

Didn't the Titanic sink? 🙃

I feel the current documentation standards are OK. And I can see it's PHPDoc that needs some improvement. It's usually the ship's captain that sets the heading, if I'm not mistaken. I've previously raised an issue in TRAC Re: JavaScript coding standards, which was rejected without debate. But I digress. We should all strive for continuous improvement. Nevertheless there is no intent to diminish the great work which has already been done. But I firmly believe builds should not error on white space snafus. Hence the issue, which I believe is applicable. Thanks again for your thoughtful input, Gary.