Commenting.PackageSniff fix seems broken when first doc comment is an inline one

[micaherne]

I'm seeing multiple issues with PackageSniff with code similar to this:

namespace availability_strathenrol;

use enrol_plugin;

class frontend extends \core_availability\frontend {

    protected function get_javascript_init_params($course, \cm_info $cm = null, \section_info $section = null) {
        $instances = enrol_get_instances($course->id, false);

        /** @var enrol_plugin[] $plugins */
        $plugins = enrol_get_plugins(false);
    }
}

PHPCS is correctly identifying that there is no package tag:

1 | ERROR   | [x] DocBlock missing a @package tag for file frontend.php. Expected @package availability_strathenrol
    |         |     (moodle.Commenting.Package.Missing)

But running the fix gives these two issues:

1. It tries to add the tag to the inline docblock where it's clearly not appropriate.

2. In the first pass it adds the tag to the end of the inline comment on the same line:

/** @var enrol_plugin[] $plugins * @package availability_strathenrol
*/

This is then detected as still violating the sniff, so it runs a second pass and adds it again on the line with the closing tag.

So the final diff is this:

-        /** @var enrol_plugin[] $plugins */
+        /** @var enrol_plugin[] $plugins * @package availability_strathenrol
+ * @package availability_strathenrol
+ */

[micaherne]

This is with the latest dev-main 85e9e0a.

[stronk7]

Hi @micaherne ,

thanks for reporting the problem, it looks really ugly! I'll give a try to it now, looking forward an acceptable solution, thanks!

[stronk7]

Ok, tracing down the problem it seems that the troubles are caused by Docblocks::getDocBlockPointer() that, when invoked for an arbitrary pointer (for example, the <php open tag, that uses to be pointer = 0) it's returning the FIRST phpdoc block in the file (in your case corresponding to the $plugins type-hinting block). And that's a bug.

In the mean time, and as a workaround, I think that if you add the file or class phpdoc block (the later is preferred in modern code), then you won't face the problem (it only happens when both phpdoc blocks are missing). If any of the blocks exist... then the fixer works ok, so far.

I'm creating a test and, hopefully, a fix... ciao :-)

[stronk7]

I've created #173 that, hopefully, fixes this edge case (it's really edge!). In any case, if accepted, it will also save a few iterations when looking for those file phpdoc blocks. 🤞

Ciao :-)

[stronk7]

@micaherne , if you've any other case, different (structurally), from the above, it would be great to know about it. With the proposed patch, I think we are covered for any other combination.

Ciao :-)

[micaherne]

Brilliant, thanks @stronk7! I haven't spotted any other issues with it so hopefully that'll be it.