Release 5.4.0 does not add our phpstan tags

[szepeviktor]

We are adding phpstan tags to WordPress core stubs. https://github.com/php-stubs/wordpress-stubs/blob/master/visitor.php

After upgrading to v5.4.0 we don't see our tags. How to proceed?

[jaapio]

I'm sorry I don't get your question. Can you explain what you are trying to do?

[szepeviktor]

For years, up to v5.3.0 we were adding @phpstan-* tags using nikic/php-parser and phpDocumentor/ReflectionDocBlock. After an upgrade to v5.4.0 those tags are not added.

What should I do to start resolving this?

[szepeviktor]

I do not know how to start debugging. Please help me.

[jaapio]

This library doesn't support the @phpstan- tags. They are handled as generic tags. I don't know how your code works nor I have the time to dive into a project consuming this library. If you have any specific questions regarding usage of the project feel free to reach out to me. I would be able to help you out.

Right now I do not have a clue what the impact of our changes are on your code without debugging it. Which is the same thing you are doing?

[szepeviktor]

We are adding a node visitor to nikic/php-parser and when there is a need we add tags to an existing AST. Everything was fine up to v5.3.0 What was changed in v5.4.0 that may affect adding @phpstan-* tags?

We are adding/creating tags, not parsing them.

[jaapio]

Only thing that could happen is that you have a newer version of php parser. Nothing has been changed on this side regarding tag creation as far as I know.

[jaapio]

The referenced issue is closed, I do not see a need to research this more. If you need any assistance feel free to reopen this issue or create a new one

[IanDelMar]

@jaapio The referenced issue wasn't closed because the issue reported here has already been resolved. Meanwhile, I was able to find that tag descriptions change significantly when updating from v5.3.0 to v5.4.0, resulting in code that relies on the description to fail.

Here's an example

    /**
     * Install a package.
     *
     * Copies the contents of a package from a source directory, and installs them in
     * a destination directory. Optionally removes the source. It can also optionally
     * clear out the destination folder if it already exists.
     *
     * @since 2.8.0
     * @since 6.2.0 Use move_dir() instead of copy_dir() when possible.
     *
     * @global WP_Filesystem_Base $wp_filesystem        WordPress filesystem subclass.
     * @global array              $wp_theme_directories
     *
     * @param array|string $args {
     *     Optional. Array or string of arguments for installing a package. Default empty array.
     *
     *     @type string $source                      Required path to the package source. Default empty.
     *     @type string $destination                 Required path to a folder to install the package in.
     *                                               Default empty.
     *     @type bool   $clear_destination           Whether to delete any files already in the destination
     *                                               folder. Default false.
     *     @type bool   $clear_working               Whether to delete the files from the working directory
     *                                               after copying them to the destination. Default false.
     *     @type bool   $abort_if_destination_exists Whether to abort the installation if
     *                                               the destination folder already exists. Default true.
     *     @type array  $hook_extra                  Extra arguments to pass to the filter hooks called by
     *                                               WP_Upgrader::install_package(). Default empty array.
     * }
     *
     * @return array|WP_Error The result (also stored in `WP_Upgrader::$result`), or a WP_Error on failure.
     */

Description of $args in v5.3.0

object(phpDocumentor\Reflection\DocBlock\Description)#12652 (2) {
  ["bodyTemplate":"phpDocumentor\Reflection\DocBlock\Description":private]=>
  string(1104) "{
    Optional. Array or string of arguments for installing a package. Default empty array.

    @type string $source                      Required path to the package source. Default empty.
    @type string $destination                 Required path to a folder to install the package in.
                                              Default empty.
    @type bool   $clear_destination           Whether to delete any files already in the destination
                                              folder. Default false.
    @type bool   $clear_working               Whether to delete the files from the working directory
                                              after copying them to the destination. Default false.
    @type bool   $abort_if_destination_exists Whether to abort the installation if
                                              the destination folder already exists. Default true.
    @type array  $hook_extra                  Extra arguments to pass to the filter hooks called by
                                              WP_Upgrader::install_package(). Default empty array.
}"
  ["tags":"phpDocumentor\Reflection\DocBlock\Description":private]=>
  array(0) {
  }
}

Description of $args in v5.4.0

object(phpDocumentor\Reflection\DocBlock\Description)#12594 (2) {
  ["bodyTemplate":"phpDocumentor\Reflection\DocBlock\Description":private]=>
  string(87) "{
Optional. Array or string of arguments for installing a package. Default empty array."
  ["tags":"phpDocumentor\Reflection\DocBlock\Description":private]=>
  array(0) {
  }
}

[IanDelMar]

The problem seems to arise from the description spanning multiple lines.

The problem arises from "empty" lines.

require_once 'vendor/autoload.php';

use phpDocumentor\Reflection\DocBlockFactory;
use phpDocumentor\Reflection\DocBlock\Serializer;

$docComment = '/**
* Example.
*
* @param array $single Single line description.
* @param array $multi1 Description with two lines.
*                      line 2/2
* @param array $multi2 Description with three lines.
*                      line 2/3
*                      line 3/3
* @param array $multi3 Description with curly braces and three lines {
*                      line 2/3
*                      line 3/3
* }
* @param array $empty1 Description with curly braces and empty line {
*
*                      below empty line
* }
* @param array $empty2 Description with empty
*
*                      below empty line
*/';

$factory = DocBlockFactory::createInstance();
$docblock = $factory->create($docComment);
$paramTags = $docblock->getTagsByName('param');
$serializer = new Serializer(0, '',true, null, null, PHP_EOL);
echo $serializer->getDocComment($docblock);

now gives

/**
 * Example.
 *
 * @param array $single Single line description.
 * @param array $multi1 Description with two lines.
 * line 2/2
 * @param array $multi2 Description with three lines.
 * line 2/3
 * line 3/3
 * @param array $multi3 Description with curly braces and three lines {
 * line 2/3
 * line 3/3
 * }
 * @param array $empty1 Description with curly braces and empty line {
 * @param array $empty2 Description with empty
 */

while it was

/**
 * Example.
 *
 * @param array $single Single line description.
 * @param array $multi1 Description with two lines.
 * line 2/2
 * @param array $multi2 Description with three lines.
 * line 2/3
 * line 3/3
 * @param array $multi3 Description with curly braces and three lines {
 *                      line 2/3
 *                      line 3/3
 * }
 * @param array $empty1 Description with curly braces and empty line {
 *
 *                      below empty line
 * }
 * @param array $empty2 Description with empty
 *
 * below empty line
 */

in v5.3.0.

[jaapio]

I'm investigating this, it will take some time as it is a limitation of the phpstan parser. It stops when descriptions span over multiple lines but also when it detects and @

[IanDelMar]

Thank you!

[jaapio]

I took your first example to reproduce the issue and added it to our test suite. It would be nice if you are able to check the new version before I create a new tag.

Thanks for your bug report this will help a lot of people!

[szepeviktor]

You're welcome.

Here it is https://github.com/php-stubs/wordpress-stubs/pull/174

[IanDelMar]

Thank you for the rapid solution once we came up with a reproducible example! For php-stubs/wordpress-stubs, we still see some differences compared to v5.3.0, though they are now far fewer in number. I'll try to pinpoint those, but I can only do so tomorrow.

[IanDelMar]

but I can only do so tomorrow

Ok, I managed to do it now. The remaining differences are due to variations in the handling of spaces/indentation. We rely on the doc comment to adhere to the WordPress coding standards, specifically looking for indentation that consists of 4 spaces or a multiple thereof. In v5.4.0, the indentation is no longer part of the sequence of multiples of 4. Therefor our visitor no longer recognizes the @type array notation.

Original ($deps)

    /**
     * Registers the script module if no script module with that script module
     * identifier has already been registered.
     *
     * @since 6.5.0
     *
     * @param string            $id       The identifier of the script module. Should be unique. It will be used in the
     *                                    final import map.
     * @param string            $src      Optional. Full URL of the script module, or path of the script module relative
     *                                    to the WordPress root directory. If it is provided and the script module has
     *                                    not been registered yet, it will be registered.
     * @param array             $deps     {
     *                                        Optional. List of dependencies.
     *
     *                                        @type string|array ...$0 {
     *                                            An array of script module identifiers of the dependencies of this script
     *                                            module. The dependencies can be strings or arrays. If they are arrays,
     *                                            they need an `id` key with the script module identifier, and can contain
     *                                            an `import` key with either `static` or `dynamic`. By default,
     *                                            dependencies that don't contain an `import` key are considered static.
     *
     *                                            @type string $id     The script module identifier.
     *                                            @type string $import Optional. Import type. May be either `static` or
     *                                                                 `dynamic`. Defaults to `static`.
     *                                        }
     *                                    }
     * @param string|false|null $version  Optional. String specifying the script module version number. Defaults to false.
     *                                    It is added to the URL as a query string for cache busting purposes. If $version
     *                                    is set to false, the version number is the currently installed WordPress version.
     *                                    If $version is set to null, no version is added.
     */

v5.3.0

  [2]=>
  object(phpDocumentor\Reflection\DocBlock\Tags\Param)#28 (6) {
    ["variableName":"phpDocumentor\Reflection\DocBlock\Tags\Param":private]=>
    string(4) "deps"
    ["isVariadic":"phpDocumentor\Reflection\DocBlock\Tags\Param":private]=>
    bool(false)
    ["isReference":"phpDocumentor\Reflection\DocBlock\Tags\Param":private]=>
    bool(false)
    ["type":protected]=>
    object(phpDocumentor\Reflection\Types\Array_)#29 (3) {
      ["valueType":protected]=>
      object(phpDocumentor\Reflection\Types\Mixed_)#30 (0) {
      }
      ["keyType":protected]=>
      NULL
      ["defaultKeyType":protected]=>
      object(phpDocumentor\Reflection\Types\Compound)#31 (2) {
        ["types":"phpDocumentor\Reflection\Types\AggregatedType":private]=>
        array(2) {
          [0]=>
          object(phpDocumentor\Reflection\Types\String_)#32 (0) {
          }
          [1]=>
          object(phpDocumentor\Reflection\Types\Integer)#33 (0) {
          }
        }
        ["token":"phpDocumentor\Reflection\Types\AggregatedType":private]=>
        string(1) "|"
      }
    }
    ["name":protected]=>
    string(5) "param"
    ["description":protected]=>
    object(phpDocumentor\Reflection\DocBlock\Description)#27 (2) {
      ["bodyTemplate":"phpDocumentor\Reflection\DocBlock\Description":private]=>
      string(668) "{
    Optional. List of dependencies.

    @type string|array ...$0 {
        An array of script module identifiers of the dependencies of this script
        module. The dependencies can be strings or arrays. If they are arrays,
        they need an `id` key with the script module identifier, and can contain
        an `import` key with either `static` or `dynamic`. By default,
        dependencies that don't contain an `import` key are considered static.

        @type string $id     The script module identifier.
        @type string $import Optional. Import type. May be either `static` or
                             `dynamic`. Defaults to `static`.
    }
}"
      ["tags":"phpDocumentor\Reflection\DocBlock\Description":private]=>
      array(0) {
      }
    }
  }

v5.4.0

  [2]=>
  object(phpDocumentor\Reflection\DocBlock\Tags\Param)#42 (6) {
    ["variableName":"phpDocumentor\Reflection\DocBlock\Tags\Param":private]=>
    string(4) "deps"
    ["isVariadic":"phpDocumentor\Reflection\DocBlock\Tags\Param":private]=>
    bool(false)
    ["isReference":"phpDocumentor\Reflection\DocBlock\Tags\Param":private]=>
    bool(false)
    ["type":protected]=>
    object(phpDocumentor\Reflection\Types\Array_)#43 (3) {
      ["valueType":protected]=>
      object(phpDocumentor\Reflection\Types\Mixed_)#44 (0) {
      }
      ["keyType":protected]=>
      NULL
      ["defaultKeyType":protected]=>
      object(phpDocumentor\Reflection\Types\Compound)#45 (2) {
        ["types":"phpDocumentor\Reflection\Types\AggregatedType":private]=>
        array(2) {
          [0]=>
          object(phpDocumentor\Reflection\Types\String_)#46 (0) {
          }
          [1]=>
          object(phpDocumentor\Reflection\Types\Integer)#47 (0) {
          }
        }
        ["token":"phpDocumentor\Reflection\Types\AggregatedType":private]=>
        string(1) "|"
      }
    }
    ["name":protected]=>
    string(5) "param"
    ["description":protected]=>
    object(phpDocumentor\Reflection\DocBlock\Description)#48 (2) {
      ["bodyTemplate":"phpDocumentor\Reflection\DocBlock\Description":private]=>
      string(1049) "{
Optional. List of dependencies.

                                       @type string|array ...$0 {
                                           An array of script module identifiers of the dependencies of this script
                                           module. The dependencies can be strings or arrays. If they are arrays,
                                           they need an `id` key with the script module identifier, and can contain
                                           an `import` key with either `static` or `dynamic`. By default,
                                           dependencies that don't contain an `import` key are considered static.

                                           @type string $id     The script module identifier.
                                           @type string $import Optional. Import type. May be either `static` or
                                                                `dynamic`. Defaults to `static`.
                                       }
                                   }"
      ["tags":"phpDocumentor\Reflection\DocBlock\Description":private]=>
      array(0) {
      }
    }
  }

[jaapio]

It seems that the new parser is automatically trimming the white space from the description. But as does this only for the first lines the white space trimming of our description factory doesn't work anymore.

So my initial fix isn't complete. I'm still searching for a good solution.

[jaapio]

I pushed a very ugly hack to see if this solves the issues you are facing in a separate branch. It would be very nice to get some feedback on that to see if it completely solves your issues before I dive into the phpstan code to see what is wrong and how to solve this.

[IanDelMar]

Unfortunately, things have worsened again. One cause seems to be that the hack adds unexpected new lines.

object(phpDocumentor\Reflection\DocBlock\Description)#86980 (2) {
  ["bodyTemplate":"phpDocumentor\Reflection\DocBlock\Description":private]=>
  string(93) "Optional. Type of resource $object_type is. Accepts 'post_type'
or 'taxonomy'. Default empty."
  ["tags":"phpDocumentor\Reflection\DocBlock\Description":private]=>
  array(0) {
  }
}

becomes

object(phpDocumentor\Reflection\DocBlock\Description)#86958 (2) {
  ["bodyTemplate":"phpDocumentor\Reflection\DocBlock\Description":private]=>
  string(94) "Optional. Type of resource $object_type is. Accepts 'post_type'

or 'taxonomy'. Default empty."
  ["tags":"phpDocumentor\Reflection\DocBlock\Description":private]=>
  array(0) {
  }
}

[jaapio]

Ok, but do you see your tags again? I assume that the extra white lines are not breaking the visitor.

[jaapio]

@IanDelMar I did try another deep dive into this issue and found a better solution. Which makes the second test pass. Would you have time to give it another try?

https://github.com/phpDocumentor/ReflectionDocBlock/pull/370

[IanDelMar]

Sorry, I have been quite busy.

Ok, but do you see your tags again? I assume that the extra white lines are not breaking the visitor.

That's exactly what's happening. The tags are disappearing again. While this could probably be fixed in our visitor, it is still a breaking change for our case. I'll have a look at #370 on the weekend.

[IanDelMar]

@jaapio #370 seems to resolve the issue! No more missing tags! :tada:

[jaapio]

Great! will merge and tag a new version