Issues with @typedef

[mindplay-dk]

I have numerous issues with the current @typedef, and, as I understand, it was merged as a preliminary measure to keep the spec moving forward.

I'd like to propose a number of changes.

(1) I'd like to change the syntax from:

@typedef ["Type"] [<"QCN">] [<"Inline PHPDoc">]

To:

@typedef ["QCN"] [<"Type">] [<"Inline PHPDoc">]

The order currently is opposite from that of most languages that support typedefs: it should be the name of the type you're defining first, and optionally the type you're extending second. Apart from being inconsistent with e.g. C, reverse order means that the name of the type you're defining doesn't consistently appear first, which makes this statement harder to read, since you have to parse it and see if it's one type or two, to figure out if the first type is the one being extended or defined.

Also, for some reason, it appears the QCN is optional, and the Type is required? Isn't that the other way around? A typedef always needs to include the QCN being defined, how can that be optional? Extending a type, on the other hand, must surely be optional?

(2) The description opens up with a list of examples - let's keep these out of the description and put them in the examples section, that's generally how it's done in the rest of the document? (I'd like to review those examples and make sure those are all shown, with code samples, in the examples section below.)

(3) This section doesn't make sense to me:

The second parameter MAY be omitted, in which case an "Inline PHPDoc" MUST be defined

The defined QCN can be omitted? As mentioned above, this still doesn't make sense to me - I think what was meant here was, the type being extended can be omitted?

That paragraph as a whole doesn't make sense to me:

The second parameter MAY be omitted, in which case an "Inline PHPDoc" MUST be defined. The information in the "Inline PHPDoc" will augment the existing base class. Using this mechanism it is possible to provide additional information with an existing class, such as methods or properties that could or were not documented in the original.

In the same paragraph, you're saying, we can leave out the class being extended, but then you continue to explain how the doc-blocks can provide additional information.

Or is this trying to say you can augment real types, not typedefs? And why would you do that? This isn't Javascript or Ruby - you can't patch a existing class with new members, unless we're talking runkit or some weird exotic thing like that?

(4) Class-level typedefs add unnecessary complexity - I'd like to limit typedefs to file-level.

For one, allowing class-level typedefs, as argued previously, distorts the concept of classes to use them as a "namespace" for virtual types. This adds complexity and learning curve by forcing you to invent a syntax to designate types nested inside other types - this syntax hasn't been discussed or defined in the current spec at all, how would you even reference a type nested inside a class? If this were supported, the ABNF would probably need to be augmented to allow some special character, e.g. \namespace\classname#typedeftype to desinate nested virtual types.

I understand the motivation for this, is so you can put your virtual types somewhere whey they can't collide with real types. First of all, I don't see this as a problem, at all - I see it as an advantage, being able to refactor between virtual types and real types. But secondly, if I did accept the argument that this is a problem, then in my opinion, we shouldn't support file-level virtual types at all, why would you, if they're so risky?

I'd like to simplify, so we can cut back on the lengthy explanations, examples and complexity. The class-level typedef raises unnecessary questions and it's not strictly necessary.

I'd very much like to simplify (and clarify) this whole feature and cut back on rules and exceptions. I think maybe the changes I'm proposing are best reviewed as a whole, so if you don't mind, I'd like to write up an alternative version of this whole section, so we can compare them side-by-side?

[jacobsantos]

It is consistent with Go and I think Swift.

https://golang.org/ref/spec#Types

https://developer.apple.com/library/prerelease/ios/documentation/Swift/Conceptual/Swift_Programming_Language/TheBasics.html (Section Type Aliases)

C style typedefs should be avoided at all costs.

[mindplay-dk]

It is consistent with Go and I think Swift.

In Go, the type you're aliasing isn't optional, so the order doesn't matter.

And you're wrong about Swift - here's what it looks like various languages:

typealias AudioSample = UInt16              // Swift

using MyList = Dummy2.CompleXList;          // C#

type MyInt = int;                           // Hack

type Row = List[Int]                        // Scala

type MyNumber = number;                     // Typescript

Foo = Bar                                   // Ruby

Bat = Baz;                                  // Javascript

Most C-family languages that have type aliasing use a <keyword> = <type> form.

I'd actually prefer to follow that form, because it improves readability - glossing over type definitions, you would notice the = at a glance.

Compare this:

/**
 * @typedef Foo Bar { ... }
 * @typedef Baz { ... }
 */

With this:

/**
 * @typedef Bar = Foo { ... }
 * @typedef Baz { ... }
 */

It's not clear in the first example that Bar is the type being defined - even having read the spec, I found the examples were hard to read. I think it's natural to expect the same operand to appear on the left? Since the right operand is optional, in our case, it makes even more sense, and the = would call further attention to the fact that you're extending a type. (Besides the fact it aligns with other languages.)

[jacobsantos]

Right. You are correct. I do like the change then.

@typedef <Keyword> "Type"

is much better.

@typedef MyCollectionType object {
    @property string $something
}

@typedef MyClosure \Closure {
    Summary of function

    @param mixed $something
}

Actually, I may have been accidentally using the above way anyway with my confusion with what the spec says. It doesn't say a lot.

[jacobsantos]

The Type is optional? The phpdoc spec I mean. It seems like it is required.

[jacobsantos]

Yes, I checked my code using @typedef. I am in fact doing

 @typedef MapCollectionCallback \Closure {
     @param mixed $element
       Item in the collection.
     @param string|int $index
       Index of element in the collection.
     @param \Traversable|array $collection
       Collection. Writes to collection will not do anything.
     @return mixed
 }

I'm also using this in the file docblock. I am also using @typedef in function level docblock.

[mindplay-dk]

The Type is optional?

I think so? if you're defining a new type, it would be redundant to say you're "extending" object, wouldn't it?

The same for array most likely, you don't need to extend anything if you're defining a tuple (known array keys).

PHP types are a bit like Javascript types, in the sense that the same object can be both an object (with methods and properties), a tuple/map/collection (via ArrayInterface) and a callable (via __invoke()) - and in PHP, even an Iterator - a type can be all of those things at the same time; with PHP code, it depends on the members you choose to implement, and with annotations, it depends on the nested tags you choose to use.

We should maybe allow to "extend" object for documentation purposes?

And probably array to indicate that the type is an actual array, meaning it's traversable?

[mindplay-dk]

Alright, while rewriting this section, I'm having some thoughts.

When associated with a File the type definition is considered to be global and available throughout your project. It is NOT RECOMMENDED to use it in this fashion without due consideration as you are making your documentation harder to read without generator or IDE.

This is merely opinion, it doesn't belong in the spec. I happen to have the opposite opinion - I think that hiding type-definitions inside class definitions will make it harder to read; I personally would recommend that any type-definitions be plainly visible at the top of the file, for the same reason I have use statements for every referenced class at the top of every file.

What you're describing here is nested classes, which are unsupported by PHP. In languages like C# you can put a type-definition inside another type-definition. Even if that were possible in PHP, the use cases for nested classes are truly infrequent.

Type definitions that are associated with a Class MUST only be used inside that class, or its descendants, and are considered to have a visibility similar to protected.

This is a strange comparison - we're talking about the visibility of types, not of members, so there isn't really any comparison you can make here, other than perhaps to something like "internal" in C#, but even that is a stretch. To clarify, here's an example:

/**
 * @typedef Bird {
 *     @var string $tail
 *     @var string $feathers
 * }
 */
class Foo
{
    /**
     * @return Bird
     */
    public function bar() {
        return array("tail" => "...", "feathers" => "...");
    }
}

Calling this public function, I'm going to get a return-type of, what, Foo\Bird? Foo#Bird?

This nested typedef isn't "protected" - even if that type is internally defined in that class, a value of that type can absolutely get exposed to code in other files. If you were to try to do something with that value, you would need some means of type-hinting against it type.

We can't leave that issue unaddressed, so we would need a type syntax extension such as Foo#Bird to support it, unless we treat the class-name as kind of pseudo-namespace and just use a syntax like Foo\Bird, both of which are strange and foreign ideas to PHP.

In PHP, all types are visible from anywhere - I really do not think we should be introducing new rules about type visibility. We should treat virtual types just like real types, to the extent that that's possible and makes sense. We shouldn't invent new semantics or rules, if we can avoid it. This will make it easier to refactor between real types and typedefs, too.

Okay, so the bottom line of what I'm getting to is this: we need to simplify. Type definitions should be just one thing with consistent, predictable behavior - it needs to be easy to explain and intuitive to use.

Now, highlighting some of the differences, pros and cons, of an exclusive file- or class-level approach:

File-level typedefs could mean you put more than one type-definition, e.g. one typedef and one real class, in the same file, which arguably goes against common code standards - but that's your choice, and you could opt for dedicated files for every typedef. I understand some people may think that's strange at first - a file with just a doc-block and no code. To me, that's the best option, under the circumstances - at least I can stick to the usual code standard of one type per file, whether that's a class, interface, trait, or typedef.

Class-level typedefs would require new type expression syntax, new rules, and much longer explanations.

Bottom line, I really think we need to kill class-level typedefs - although file-level typedefs are not ideal, I don't think there is any ideal solution, both approaches have drawbacks, so I'd opt for the simplest solution.

Another thing to consider, is that class-level typedefs are a means of putting multiple types in the same file - now, whether you think that's good or bad, it's not really any different from just putting a file-level typedef in the top of a class file. The only real difference is the visual layout of the source code. The net result is a class and a typedef either way.

As explained, I personally would stick to one file per type, but for those who aren't comfortable with that, putting typedefs inside your class files is still an option.

I know this is a long post, so let that simmer for a bit, please.

I will have a draft up soon, I just mean to warm you up before springing it on you ;-)

[mindplay-dk]

Alright take a look.

I think that @implements also needs to be augmented, since it could now be used not only to designate implemented generic interfaces, but also implemented virtual types. (I haven't made that update, since this would mean you can't merge the typedef and generics branches individually.)

[mindplay-dk]

(don't bother with the diff, just read through the new section, it's basically a complete rewrite.)

[mvriel]

Based on feedback and that this item has not seen any real world usage I am planning on removing @typedef from PSR-5; I am considering taking your suggestions and adding it to phpDocumentor itself but for a standard it seems to be too contentious momentarily. As soon as it sees more real world usage we may consider putting it up for a new PSR.

Should any of the participants wish to discuss this further then I request that we do that on the PHP-FIG mailing list so that more people may chime in.