This rule is returning an error when the code is correct - and not returning an error when the code is incorrect.

i.e

/**
* Display help and module information
*
* @param string $path
*   Path which path of the site we're displaying help.
* @param array $arg
*   Arg array that holds the current path as would be returned from
*   arg() function.
*
* @return string
*   Help text for the path.
*/

fails currently - whereas

/**
* Display help and module information
*
* @param $path
*   Path which path of the site we're displaying help.
* @param $arg
*   Arg array that holds the current path as would be returned from
*   arg() function.
*
* @return string
*   Help text for the path.
*/

passes.

I've marked this as critical because people using this module will change their code - and then their code will fail the code sniffer test which means they's have to change their code back again.

Comments

Morbus Iff’s picture

Status:Active» Closed (duplicate)

Duplicate of http://drupal.org/node/1361528.

And note that the data-typed version is *only* correct in Drupal 8 code or higher. You should *not* be adding data types to params for D6 or D7 patches.

bailey86’s picture

Hi,

Thanks for the reply. The current coding standards say data types *can* be added - http://drupal.org/node/1354#functions.

Also, the code standards testing tool phpcs expects the data types to be there. http://drupal.org/project/drupalcs

But if this is a duplicate - and the check is going to be removed then that's issue resolved as far as I'm concerned.

And thanks for the excellent tool.

Morbus Iff’s picture

Per the other issue, webchick has specifically said that they SHOULD NOT be applied to D7 or D6 core patches. You're more than welcome to do so in your own code, but the style guide has been, in the past, indicated as only applying to "future" code, not legacy code. Since the data types addition happened after D7 was released, it is "only" applicable (in a honorable sorta way) to D8 code. Style checks that run under D7 and D6 don't "have to" respect it. See my comments in the other issue for more on this.

bailey86’s picture

Thanks for the reply.

But the docs do say:

'Note: This section of the standard was adopted as of Drupal 8. In previous versions, data type specification was recommended in the case of non-obvious types or specific classes/interfaces only.'

so, since the data type could be added under certain circumstances my thinking is that your code should not flag having the data type set as as an error.

This would make your code fine for D6/D7/D8 etc.