A warning is being thrown whenever a hook implementation does not read

/**
 * Implements hook_foo().
 */
function mymodule_foo() {

This doesn't actually cover all possible cases described in http://drupal.org/node/1354#hookimpl as this:

/**
 * Implements hook_foo_BAR_ID_baz() for some_type_bar().
 */
function mymodule_foo_some_type_baz() {

is also a valid declaration.

CommentFileSizeAuthor
#7 1346754_drupalcs_hook-implements.patch1.04 KBsirkitree

Comments

sirkitree’s picture

sirkitree commented

Took a look at this, and found the code where this happens:

        // Check if hook implementation doc is formated correctly.
        if (preg_match('/^[\s]*Implement[^\n]+?hook_[^\n]+/i', $comment->getShortComment(), $matches)) {
            $formattingIssue = 0;
            if(!strstr($matches[0], 'Implements ')){
                $formattingIssue++;
            }
            if(!preg_match('/ hook_[\S\(\)]+\.$/', $matches[0])){
                $formattingIssue++;
            }
            if($formattingIssue){
                $phpcsFile->addWarning('Format should be * Implements hook_foo().', $commentStart + 1);
            }
        }

But I'm totally incapable of discerning a modification to the preg_match() that will account for this. :(

doitDave’s picture

doitDave commented

Not tested, just from mind to keyboard:

        // Check if hook implementation doc is formated correctly.
        if (preg_match('/^[\s]*Implement[^\n]+?hook_[^\n]+/i', $comment->getShortComment(), $matches)) {
            $formattingIssue = 0;
            if(!strstr($matches[0], 'Implements ')){
                $formattingIssue++;
            }
            if(!preg_match('/ hook_[a-zA-Z0-9_]+\(\)( for [a-z0-9_]+)?\.$/', $matches[0])){
                $formattingIssue++;
            }
            if($formattingIssue){
                $phpcsFile->addWarning('Format should be "* Implements hook_foo()." or "Implements hook_foo_BAR_ID_baz() for xyz_bar."', $commentStart + 1);
            }
        }

HTH.

Edit: preg pattern.

sirkitree’s picture

sirkitree commented

No good. Still detects the example as a warning, just spits out an extended message.

doitDave’s picture

doitDave commented

^^ Try again.

(Funny as it is: The code itself wouldn't even pass the indentation check... *ahum*)

sirkitree’s picture

sirkitree commented

Still getting the error.
Here is what I'm checking against:


/**
 * @file
 */

/**
 * Implements hook_foo_BAR_ID_baz() for some_type_bar().
 */
function mymodule_foo_some_type_baz() {

}
doitDave’s picture

doitDave commented

That is correct. (The error.) The proper syntax is "Implements hook_foo_ BAR_ID_baz() for some_type_bar."

E.g.:

Implements hook_form_FORM_ID_alter() for comment_form.

sirkitree’s picture

sirkitree commented
Status: Active » Reviewed & tested by the community
StatusFileSize
new 1346754_drupalcs_hook-implements.patch1.04 KB

Oh, ok - I was going by your example in your original post. In that case, this works and here is a patch.

doitDave’s picture

doitDave commented
Status: Reviewed & tested by the community » Needs review

Damn. I was right, you were right, I was wrong. Indeed it is twice "()" as of http://drupal.org/node/1354 - so, again, finally:

        // Check if hook implementation doc is formated correctly.
        if (preg_match('/^[\s]*Implement[^\n]+?hook_[^\n]+/i', $comment->getShortComment(), $matches)) {
            $formattingIssue = 0;
            if(!strstr($matches[0], 'Implements ')){
                $formattingIssue++;
            }
            if(!preg_match('/ hook_[a-zA-Z0-9_]+\(\)( for [a-z0-9_]+\(\))?\.$/', $matches[0])){
                $formattingIssue++;
            }
            if($formattingIssue){
                $phpcsFile->addWarning('Format should be "* Implements hook_foo()." or "Implements hook_foo_BAR_ID_baz() for xyz_bar."', $commentStart + 1);
            }
        }
das-peter’s picture

das-peter commented
Status: Needs review » Fixed

Thank you guys for the bug-hunting and elimination, very appreciated :)
This looks good to me and thus -> http://drupalcode.org/project/drupalcs.git/commit/4a54f1a
(The only change I made additionally, was that I added examples to the good / bad test-files)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.