Align PSR-19 @method signatures with PHP7 with regard to return types

[rquadling]

Originally : #899

Currently, the placement of the return type does not match that of PHP7.

For a new developer, this would seem 'wrong' as there are now 2 different signatures for methods.

In addition, allows for the documentation to support static methods that are processed by __callStatic().

And finally, this change removes the ambiguity where a method may be one of two;

<?php

class Parent
{
    public function __call()
    {

    }

    public static function __callStatic()
    {

    }
}

/**
 * @method static getString()
 */
class Child extends Parent
{
    <...>
}

Is getString() a static method returning void, or a regular method returning a static instance?

By having the scope and return type separated, the ambiguity is removed.

[drupol]

LGTM

[WinterSilence]

I think need use traditional style i.e. base on @property-read, @property-write tags, readonly property(added in PHP 8.1) can be declared as @property-readonly and static method as @method-static.

[WinterSilence]

Unfortunately, none of the variant allows declare link/alias in @method i.e. specify a method of another class without parameters and return type:

class A
{
    public static function run(array $options)
    {
        <...>
    }
}
/**
 * @method A::run()
 * or
 * @method run() aliasOf A::run()
 */
class B
{
    public static function __callStatic(string $name, array $arguments)
    {
         if ($name === 'run') {
             return A::(...$arguments);
         }
         throw new RuntimeException('...');
    }
}

This would solve the problem of obsolescence: if we set returning type for A::run(), then need update @method tag for all classes calling A::run() as magic method.

[WinterSilence]

it can confuse, when project require vendor packages using difference styles for @method: @method static foo() - static is modifier or returning type.

[rquadling]

Using @method-static is certainly 1 way to remove the potential ambiguity around this. It is nicely declarative.

For me, the exercise is to provide what things should look like for new developers. Having gotten the PHP Documentation to generate the signatures in a way that a new developer would understand, I can see no real reason why the standards documentation shouldn't also provide the same view of method signatures for __call() and __callStatic().

So,

@method [name]([type] [parameter], [...])[: return type] [description]
@method-static [name]([type] [parameter], [...])[: return type] [description]

But as the placement of scope and return type of static are "correct" (i.e. their placement ties up with the PHP Documentation of functions/methods, I don't see the need for a separate docblock tag.

So a docblock entry of:

@method someMethod(): static

would be expected to be understood as:

class SomeClass {
  public function someMethod(): static {}
}

whereas a docblock of:

@method static someMethod()

would be expected to be understood as:

class SomeClass {
  public static function someMethod(): void {}
}

And:

@method static someMethod(): static

would be expected to be understood as:

class SomeClass {
  public static function someMethod(): static {}
}

The placement of static is consistent with the PHP Documentation as well as a possible subclass wanting to override a parent class's magic method.

There is simply no ambiguity.

[WinterSilence]

@rquadling it's standard, it's can't be changed easely - need update all tools uses standard too. Do you have time do this? (:

[ashnazg]

Do any of the tag tokenizing applications (phpstorm, phpstan, psalm) use or allow this notation? We need to remember that this effort is to standardize on what's in use, not generate a new standard that they all must bend to.

[ondrejmirtes]

I don't think it's a good idea to make such a change now. The @method annotation has been in wide use for many years - you're not asking for a change, you're asking everyone to support both formats at once. It's not guaranteed this would gain any traction.

AFAIK this suggestion also makes it incompatible with the current format, as parsers in @method [static] [name]([type] [parameter], [...])[: return type] [description] would consume the entired end [: return type] [description] as description.

[rquadling]

Considering the return type itself is of a known format ...

: type Single type
: type1 | type2 Union type

I disagree that upgrading the parser to support union types would be that much effort.

No more than

@method type1 | type2 name()

is difficult.

The usage of : to act as the trigger for return types and | for union type and [:space:] as the separator between type(s) and description (just like [:space:] is the separator between type(s) and name at the moment) ... doesn't feel particularly complex.

[rquadling]

I have also raised this issue with JetBrains several years ago : https://youtrack.jetbrains.com/issue/WI-42107

[jaapio]

There is an issue with this proposal. The original format is clear and easy to process. The new format has some problems that need to be addressed. The most important one is. What is a type and where does the description start?

In the original format the optional words before the part that defines the method name is defined:

@method [return type] [name]([type] [parameter], [...]) [description]

Adding few new keywords in front like private static etcetera is possible. but moving the return type to the back would cause problems. Given this example:

   @method name(): MyClass | Will return something. 

How should a parser understand that MyClass is the return type and the | Will is the start of some description? There are no limitations on the chars used in a description. However, it might be odd to start a description with a | you might be able to use it. Especially because the description can be interpreted as markdown. A table would be common. Yes, there will be ways to overcome this, but it will make the parsers way more complex.

Besides that, in the earlier given feedback, the @method format exists for years and should not be changed is valid IMO.

I do agree with the format of adding [static] as an optional part of the magic method.

[ashnazg]

@rquadling , I think I'm with @jaapio in that the additions for keywords like static could fit the scope of this effort... though I also agree with him and @ondrejmirtes both that it's too ingrained to move the location of the type placeholder in the syntax.

Closing this, but submit a new PR with just the keyword additions if you're game. If not, we'll probably separately get around to adding it.

I do thank you for the effort though... I follow your reasoning for this... I just can't see us dictating a big change over historical usage.