Release 5.4.0 does not add our phpstan tags #365

szepeviktor · 2024-04-13T11:44:59Z

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 · 2024-04-13T14:15:22Z

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

szepeviktor · 2024-04-13T14:18:27Z

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 · 2024-04-13T14:19:21Z

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

jaapio · 2024-04-13T14:25:43Z

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 · 2024-04-13T14:28:08Z

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 · 2024-04-13T17:21:57Z

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 · 2024-05-07T05:10:42Z

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 · 2024-05-08T02:34:11Z

@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 · 2024-05-08T15:57:01Z

~~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 · 2024-05-08T16:58:40Z

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 · 2024-05-08T16:59:30Z

Thank you!

The phpstan parser is not consuming the full description when parsing docblocks with a more complex description. For them it's mostlikely not an issue as phpstan doesn't use the descriptions. But it will also parse the descriptions into unexpected tags. This could be an advantage but is not according to the phpdoc spec. Our own tokenizer is already tokenizing the docblocks into the correct parts. So all we needed to do is assume all remaining tokens in the phpstan ast belong to the description. From there our own code is able to handle this as before in v5.3. fixes #365

jaapio · 2024-05-08T18:53:40Z

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 · 2024-05-08T20:02:27Z

You're welcome.

Here it is php-stubs/wordpress-stubs#174

IanDelMar · 2024-05-08T20:11:00Z

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 · 2024-05-08T20:54:25Z

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 · 2024-05-12T20:14:21Z

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 · 2024-05-13T19:55:10Z

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 · 2024-05-13T23:26:53Z

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 · 2024-05-14T04:38:14Z

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

jaapio · 2024-05-17T15:47:42Z

@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?

#370

IanDelMar · 2024-05-17T21:04:28Z

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 · 2024-05-21T02:49:54Z

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

jaapio · 2024-05-21T05:50:03Z

Great! will merge and tag a new version

szepeviktor mentioned this issue Apr 13, 2024

The nice present: core phpstan tags vs. our phpstan tags in WordPress 6.5 php-stubs/wordpress-stubs#163

Closed

jaapio closed this as not planned Won't fix, can't repro, duplicate, stale May 7, 2024

jaapio reopened this May 8, 2024

jaapio mentioned this issue May 8, 2024

Bugfix: resolve issue with multiline descriptions #367

Merged

jaapio closed this as completed in #367 May 8, 2024

jaapio reopened this May 8, 2024

jaapio mentioned this issue May 13, 2024

Hack to work around parser issue #369

Closed

jaapio mentioned this issue May 13, 2024

Newlines stops consuming tag descriptions phpstan/phpdoc-parser#241

Closed

jaapio mentioned this issue May 21, 2024

Fix docblock tag descriptions #370

Merged

jaapio closed this as completed in #370 May 21, 2024

IanDelMar mentioned this issue May 21, 2024

Release phpdocumentor/reflection-docblock lock php-stubs/wordpress-stubs#178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Release 5.4.0 does not add our phpstan tags #365

Release 5.4.0 does not add our phpstan tags #365

szepeviktor commented Apr 13, 2024 •

edited

Loading

jaapio commented Apr 13, 2024

szepeviktor commented Apr 13, 2024 •

edited

Loading

szepeviktor commented Apr 13, 2024 •

edited

Loading

jaapio commented Apr 13, 2024

szepeviktor commented Apr 13, 2024 •

edited

Loading

jaapio commented Apr 13, 2024

jaapio commented May 7, 2024

IanDelMar commented May 8, 2024

IanDelMar commented May 8, 2024 •

edited

Loading

jaapio commented May 8, 2024

IanDelMar commented May 8, 2024

jaapio commented May 8, 2024 •

edited

Loading

szepeviktor commented May 8, 2024

IanDelMar commented May 8, 2024

IanDelMar commented May 8, 2024

jaapio commented May 12, 2024

jaapio commented May 13, 2024

IanDelMar commented May 13, 2024

jaapio commented May 14, 2024

jaapio commented May 17, 2024

IanDelMar commented May 17, 2024

IanDelMar commented May 21, 2024

jaapio commented May 21, 2024

Release 5.4.0 does not add our phpstan tags #365

Release 5.4.0 does not add our phpstan tags #365

Comments

szepeviktor commented Apr 13, 2024 • edited Loading

jaapio commented Apr 13, 2024

szepeviktor commented Apr 13, 2024 • edited Loading

szepeviktor commented Apr 13, 2024 • edited Loading

jaapio commented Apr 13, 2024

szepeviktor commented Apr 13, 2024 • edited Loading

jaapio commented Apr 13, 2024

jaapio commented May 7, 2024

IanDelMar commented May 8, 2024

IanDelMar commented May 8, 2024 • edited Loading

jaapio commented May 8, 2024

IanDelMar commented May 8, 2024

jaapio commented May 8, 2024 • edited Loading

szepeviktor commented May 8, 2024

IanDelMar commented May 8, 2024

IanDelMar commented May 8, 2024

jaapio commented May 12, 2024

jaapio commented May 13, 2024

IanDelMar commented May 13, 2024

jaapio commented May 14, 2024

jaapio commented May 17, 2024

IanDelMar commented May 17, 2024

IanDelMar commented May 21, 2024

jaapio commented May 21, 2024

szepeviktor commented Apr 13, 2024 •

edited

Loading

szepeviktor commented Apr 13, 2024 •

edited

Loading

szepeviktor commented Apr 13, 2024 •

edited

Loading

szepeviktor commented Apr 13, 2024 •

edited

Loading

IanDelMar commented May 8, 2024 •

edited

Loading

jaapio commented May 8, 2024 •

edited

Loading