The issue #2608444: Some docblock fixes for PHP type 'NULL' contains some fixes for the incorrect use of PHP type 'NULL' in docblock @param, @return and @var directives. In comment #15 of that issue, @xjm requested that we create or refine a code_sniff rule for this documentation standard violation.
It appears that the sniff FunctionCommentSniff checks for Drupal documentation standards in function and method docblocks. Reviewing that file, it appears that no check currently exists for this documentation standard violation.
An example of good documentation is:
* @param array|null $var1
* (optional) An associative array. Defaults to NULL.
* @param string|null $var2
* (optional) A string. Defaults to NULL.
*
* @return array|null
* An array or NULL in case of error.
*/
function example($var1 = NULL, $var2 = NULL) {
A bad documentation example is:
* @param array|NULL $var1
* (optional) An associative array. Defaults to NULL.
* @param string|NULL $var2
* (optional) A string. Defaults to NULL.
*
* @return array|NULL
* An array or NULL in case of error.
*/
function example($var1 = NULL, $var2 = NULL) {
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | diff-2620856-6-11.txt | 1.67 KB | anoopjohn |
| #11 | coder-fix-correct-param-type-casing-of-NULL-etc-2620856-11.patch | 3.75 KB | anoopjohn |
Comments
Comment #2
klausiYou confused the good/bad example, fixed that.
Comment #3
lars toomre commentedThank you @klausi. I did indeed have what were good and bad examples reversed.
To what files should the good and bad code examplex be added? Locally l have the sniff working for this case. I am now trying to learn/figure out how to add a test for my updated sniff. Any suggestions?
Comment #4
lars toomre commentedI have spent some time today trying to implement a code sniff for this violation of Drupal's documentation standards. Attached is a patch that locally is correctly catching the incorrect use of NULL, FALSE and TRUE in @param, @return and @var directives.
I am unsure how to write tests for this modified sniff. Hence, I am also uploading the good and bad example files that I have been testing against. The following is the result I am getting locally when running phpcs against both the good and bad examples together:
FILE: D:\Code\phpcs\bad-docblock-example.php
----------------------------------------------------------------------
FOUND 9 ERRORS AFFECTING 8 LINES
----------------------------------------------------------------------
16 | ERROR | [x] Expected "bool" but found "boolean" for parameter
| | type
| | (Drupal.Commenting.FunctionComment.IncorrectParamVarName)
18 | ERROR | [x] Expected "true" but found "TRUE" for parameter type
| | (Drupal.Commenting.FunctionComment.IncorrectParamVarName)
21 | ERROR | [ ] Expected "string|false" but found "string|FALSE" for
| | function return type
| | (Drupal.Commenting.FunctionComment.InvalidReturn)
36 | ERROR | [x] Expected "bool" but found "Bool" for parameter type
| | (Drupal.Commenting.FunctionComment.IncorrectParamVarName)
36 | ERROR | [x] Expected "null" but found "NULL" for parameter type
| | (Drupal.Commenting.FunctionComment.IncorrectParamVarName)
38 | ERROR | [x] Expected "true" but found "TRUE" for parameter type
| | (Drupal.Commenting.FunctionComment.IncorrectParamVarName)
41 | ERROR | [ ] Expected "string|false" but found "string|FALSE" for
| | function return type
| | (Drupal.Commenting.FunctionComment.InvalidReturn)
58 | ERROR | [x] Expected "int" but found "Integer" for parameter
| | type
| | (Drupal.Commenting.FunctionComment.IncorrectParamVarName)
60 | ERROR | [x] Expected "int" but found "Int" for parameter type
| | (Drupal.Commenting.FunctionComment.IncorrectParamVarName)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 7 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
Comment #5
klausiCool, thanks!
Can you add the good and bad examples to coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.inc and the *.fixed variant? You will also have to modify coder_sniffer/Drupal/Test/Commenting/FunctionCommentUnitTest.php to indicate on what lines errors appear.
Then run PHPUnit as indicated in README.md and check that it all passes.
Comment #6
anoopjohn commentedPlease find attached a patch that includes the test cases. While creating the test cases I found that the autocorrect of param types with multiple types was not working and reported an error and provided a patch for the same #2735365: Error in fixing incorrect parameter types when multiple parameter types are possible in one param tag.
The test cases provided here depends on the other patch and I had therefore applied the previous patch before creating this patch. So for this patch to apply cleanly the former should be applied. The conflict is in the test files as both patches add test functions and updates error arrays. Not sure how to prevent conflicts when applying consecutive patches in the same test file. This patch will apply cleanly if the patch in #2735365: Error in fixing incorrect parameter types when multiple parameter types are possible in one param tag is applied first. Hope we can take both of these issues to closure together.
Comment #7
anoopjohn commentedComment #8
klausiThanks, looks like this needs a reroll now.
Comment #9
rajeshwari10 commentedHi klausi,
I was not able to reroll the patch, so applied manually all the above changes in the patch submitted.
Please review.
Thanks!!
Comment #10
klausithanks, but phpunit is failing. I think we need to wait for #2735365: Error in fixing incorrect parameter types when multiple parameter types are possible in one param tag
Comment #11
anoopjohn commentedNow that #2735365: Error in fixing incorrect parameter types when multiple parameter types are possible in one param tag has gone in I have rerolled the patch. Please find the patch and the diff. Interdiff failed. BTW when interdiff fails do we do anything else other than diff?
Comment #13
klausiCommitted, thanks!