Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline element definitions #7

Closed
mvriel opened this issue Aug 9, 2013 · 32 comments
Closed

Inline element definitions #7

mvriel opened this issue Aug 9, 2013 · 32 comments
Assignees

Comments

@mvriel
Copy link
Member

mvriel commented Aug 9, 2013

Suppose I have a webservice that returns the following JSON:

{id: 1, name: "This is my name"}

In my code this would (simplistically speaking) be expressed by json_decoding the string and probably returning it in a method. The problem
here is that the caller only knows that an stdClass will return but nothing in the documentation makes clear what it represents or what
members it has.

As such I propose that we add the capability to define meta-elements in DocBlocks to be able to return information on these stdClasses.

For example:

/**
 * @define myReturnedClass {
 *     @property integer $id
 *     @property string $name
 * }
 *
 * @return @myReturnedClass
 */

as is visible; the concept of Inline DocBlocks is reused here. The details of this new syntax still need to
be written and defined, the tag name and method to invoke a definition are not final and should still
be considered

@philsturgeon
Copy link

Would a definitions block elsewhere make more sense? This currently suggests multiple returns of @myReturnedClass would need multiple definitions.

@mvriel
Copy link
Member Author

mvriel commented Aug 16, 2013

My original thought on this was to give the definitions block scope. So the following business rules would be in use:

  1. Any definition is available on the element on which it is defined and any of its child elements
    • Example: A definition on a method is only available on that method
    • Example: A definition on a class is available on that class and its constants, properties and methods
  2. A definition on a class, trait or interface may be referred to from another element using a FQSEN and the definition
    • Example:
      @return My\Class@myDefinition

I would like to emphasize that I am not sold on using the @ sign to refer to definitions. They look confusing to me and should be reconsidered

@mindplay-dk
Copy link

Just for reference, I posted some notes on this subject here.

@mindplay-dk
Copy link

I would like to emphasize that I am not sold on using the @ sign to refer to definitions. They look confusing to me and should be reconsidered

I agree - why not simply use regular type-names? In languages where you can define function signatures as types, they are usually just regular type-identifiers like any other type.

A type is a type - whether it's a concrete type, interface, trait, pseudo-type, or even type-alias, there is no good reason why the name of a type should reflect the kind of type.

The kind (or "type of type") belongs in the type definition, not in the type name.

@sun
Copy link

sun commented May 22, 2014

The main problem I see with this proposal is that it is a global state construct.

Similar to the @global tag used by some, the definition can live anywhere in a code base (and you don't know where), and it applies to all code that is being parsed.

  1. What happens if the parsed code base contains code of multiple independent projects?
  2. What if an element is defined more than once, but differently? (perhaps even in your own code)
  3. How do you keep track of what is being defined how and where?

Basically for these reasons, the @defgroup and @mainpage are avoided today, because they assumed that the entire code base is developed by a single party and under your own control.

I understand the use-case, but I wonder whether there are sufficient real world situations in the wild that would significantly benefit from a native solution like this, and whether they outbalance aforementioned maintenance problems

The inline suggestion in #19 made more sense to me, because it avoids the problem of global state in documentation.

Lastly, since the topic starter is about an anonymous/stdClass object, #47 on documenting collections appears to be closely related.


FWIW, the tag name @define is ambiguous; "define" normally refers to defined PHP constants.

@mvriel
Copy link
Member Author

mvriel commented May 24, 2014

As I see it a definition should always be related to a block element and not in the global space. This will automatically namespace it to prevent/reduce conflicts and provide a clear and definite location where to find the definition

@mvriel
Copy link
Member Author

mvriel commented May 24, 2014

Allright, I have had time to think this through and would like to propose the following:

Using inline statements you can

  1. assign keys with associative arrays and collections inline with your @type tag:

    @type array {
        @type integer $keyName 
    }
    

    or

    @type \ArrayObject<string,mixed> {
        @type integer $keyName 
    }
    
  2. assign properties and methods to both existing and non-existing objects:

    @type object {
      @property integer $myProperty
      @method string $myMethod(integer $myParameter = 1)
    }
    

    or

    @type \SimpleXMLElement {
      @property integer $myProperty
      @method string $myMethod(integer $myParameter = 1)
    }
    
  3. Define a re-usable type definition within the current scope on a class/interface/trait element (thus excluding file docblocks, variables, constants, properties and methods)

    @type-definition object MyDefinition {
      @property integer $myProperty
    }
    

    and invoke it with:

    @type #MyDefinition
    

    So if you want to include a definition from another class then you'd write:

    @type \My\Other\Container#MyDefinition
    

I would really love feedback, be it positive or negative on this suggestion

@mindplay-dk
Copy link

I would really love feedback, be it positive or negative on this suggestion

My main issue with this, is the idea that definitions are names in a separate namespace, as defined by item 3 above.

Why aren't they just type-names, in the same namespace as other types? The type namespace already contains things that aren't strictly classes, such as pseudo-types - why the # symbol and dedicated "definition names"? They are types in every other sense, and since the # is not valid as part of a class-name, it could complicate refactoring operations.

In my opinion, type-definitions should be namespaced, but the defined type, like any other type, should belong to the namespace, not to a class/interface/trait - they should be declared at the file-level, most likely, in the namespace of the given file.

I propose the following alternative to item 3:

Define a re-usable type definition within the current namespace of a file, by using a file-level doc-block:

<?php

namespace Foo;

/**
 * @type-definition object MyDefinition {
 *   @property integer $myProperty
 * }
 */

and invoke it with:

@type Foo\MyDefinition

This is considerably simpler (to understand, and likely to implement) than permitting some new concept like "type definition" which can be nested inside all kinds of other elements - nesting in this way doesn't make much sense, because you end up effectively using e.g. the parent class-name or interface-name as merely a namespace; the definition is an independent element with an independent fully-qualified name, with a new syntax (the #), but it's only structurally nested, in terms of source-code, logically it's not really "nested", since the parent type-name is treated as merely a namespace.

I also would recommend @typedef rather than the verbose @type-definition, since the word "typedef" is pretty generic to every other language that supports it. I don't even think most people actually pronounce it as anything other than "typedeaf" ;-)

Note that assigning properties and methods to both existing and non-existing objects (as per item 2 above) is generally termed as "ambient declarations" - these are widely used in e.g. TypeScript, and the reasons for allowing these apply equally to PHP and JavaScript, so I'm glad to see you thought of these!

The concept of type-definitions can be greatly simplified though - a type is a type, let's treat them as such, rather than inventing new syntax and more complex concepts?

@mvriel
Copy link
Member Author

mvriel commented May 24, 2014

Perhaps I am too afraid of FQCN conflicts.

On the other hand it might actually be a strength because you could replace the typedef with a real class without too much problems (I'd recommend a real class anyway but I can see the need and use-case for this).

Regarding type-definition vs. typedef; I have done a bit more googling and also found the typedef tag in JSDoc which does a similar thing. So despite that I dislike such abbreviations I agree that it is a recognized term and would be better suited.

@mvriel
Copy link
Member Author

mvriel commented May 24, 2014

In addition; perhaps we should even go further with this and allow @typedef in the File DocBlock and then give it a global character. Although this is heavily discouraged are there two other issues in this tracker that would be resolved with this feature:

  1. Add pseudo type: scalar #20, where people ask for the pseudotype numeric and scalar; you can allow people to define it by having a typedef @typedef int|bool|string|float scalar
  2. @alias for cover class_alias()`ed classes IDE completion #48, where Laravel uses class_alias to create the alias Session from a class Storage; this can be defined as @typedef Storage Session

@mindplay-dk
Copy link

Perhaps I am too afraid of FQCN conflicts.

There is no reason to fear conflicts - just use namespaces, just as you would for any other type. No conflicts will occur then. (anymore than conflicts will occur between any other user-defined types.)

On the other hand it might actually be a strength because you could replace the typedef with a real class without too much problems

This is what I mean by refactoring - a type-name like "Foo\Bar#Baz" cannot refactor into a concrete type implementation. (because the # is incompatible with PHP type-names.)

perhaps we should even go further with this and allow @typedef in the File DocBlock and then give it a global character

That is what I'm suggesting - but I'm suggesting also, that if you have this feature, there is actually no practical reason to define types nested inside other types, because the name of a class/interface/trait will not be treated like a type-name at all, it will be treated as a name-space. (unlike e.g. C#, PHP does not support nested types, which is why you don't have the # syntax or something equivalent in PHP.)

@mindplay-dk
Copy link

Also, +1 for the two examples you quoted.

What problem do you see the introduction of nested types solving?

@mvriel
Copy link
Member Author

mvriel commented May 24, 2014

@mindplay-dk it was not necessarily intended as a nested type, moreover as preventing a conflict if there were a class with the same FQCN as the typedef

@mindplay-dk
Copy link

it was not necessarily intended as a nested type

Intended or not, the way it was proposed, the defined type is nested inside a real type, hence the # separator necessary to separate the names.

moreover as preventing a conflict if there were a class with the same FQCN as the typedef

Understood - but as explained, that's what namespaces are for. You don't need to invent new syntax, or a new concept/idea, or a new name-space for that new concept/idea/type.

Whether you have a \ or a # separating the namespace from the name, really doesn't (substantially) affect the risk of name collision.

We already have namespaces as the defined means of avoiding ambiguity and collisions in PHP - using class/trait/interface names as a kind of "extra" namespace for defined types is trying to address a problem that is already addressed by namespaces.

It's no better or worse - it's just not necessary. If you did want an extra name for some reason, in case you're really worried about ambiguity in some case, you can just add another namespace... that is to say, Foo\Bar\Baz is no better or worse than Foo\Bar#Baz, it's just a different name-separator, which creates problems with refactoring, but doesn't really add anything...

@mvriel
Copy link
Member Author

mvriel commented May 24, 2014

I tend to disagree with your assessment but discussing that further is much better suited over a beer or other beverage as I think we agree on the direction :)

I am now writing the first draft of an @typedef tag definition which will describe what it can do and what it is used for.

@mindplay-dk
Copy link

I wish I could meet you for a beer - we're probably a world apart ;-)

@mvriel
Copy link
Member Author

mvriel commented May 24, 2014

Denmark - Holland ain't that far away; just a bit too far to drive for just a beer :) You are not attending conferences?

@mindplay-dk
Copy link

I do attend conferences, but probably not in the very near future - my situation right now is a bit complicated... I just arrived back in Denmark a couple of months ago, after living in the US for the past 6 years - currently living with my parents, trying to get my life back on track, find a job, a place to live and so on... divorce...

@mvriel
Copy link
Member Author

mvriel commented May 25, 2014

Ouch, those are difficult times indeed. Take care of yourself and I wish you strength in getting back on track

@mindplay-dk
Copy link

Thanks man :-)

@mvriel
Copy link
Member Author

mvriel commented Jul 15, 2014

@mindplay-dk @ashnazg and others, I have written a draft for the new @typedef as discussed earlier. I am going to assume it needs better wording at various points and even clarifications; could you take a look at it and provide your feedback? The PR is #54

@corphi
Copy link

corphi commented Jul 16, 2014

Array keys can contain arbitrary bytes, so it should include a way to specify variable names beyond the allowed variable name characters. The same applies to object members. An empty name should also be possible, although it is hard to access on objects ((object) array('' => 'foo')).

And I’d like to document the leaf level of deeply nested structures (well, your @typedef allows that).

mvriel pushed a commit that referenced this issue Dec 9, 2014
[Cache] Adding InvalidArgumentException
@mindplay-dk
Copy link

Two thoughts:

  1. I still think that @typedef should be allowed only at the file-level - optionally treating a class as parent/namespace is confusing. PHP doesn't support nested types, so this adds a new concept and learning curve. But above all, it's unnecessary - file-level typedefs will be quite adequate. Can we simplify, please?
  2. Thoughts on allowing @param and @return in typedefs? This would allow us to define callback signatures, or even more complex signatures for types implementing __invoke() as well as other methods and properties. The ability to define signatures for callbacks is desperately missing for IDE support, I run into this often.

@teolaz
Copy link

teolaz commented Jan 29, 2018

Still opened since 2013? No way to simply solve this?
I'm quite interested in @ param and @ return for associative arrays...

@mindplay-dk
Copy link

@teolaz I'm pretty sure this PSR is dead.

@ashnazg
Copy link
Member

ashnazg commented Jan 29, 2018

I intend to resurrect it this spring, after phpdoc3 is done.

@Crell
Copy link

Crell commented Jan 29, 2018

Wouldn't it make more sense to resurrect before you finish the new phpdoc version, so you can make the new phpdoc version match the spec?

@ashnazg
Copy link
Member

ashnazg commented Jan 29, 2018

Indeed... a better description of my plan is to get the CI of everything underlying phpdoc3 in place... then, while the others work on completing their own phpdoc3 stuff, I'll switch to the PSR5 WG effort. I had realized that I wouldn't have the time available to do both at the same time, so opted for the CI first. Once I get past the current blocker on that front (can't test new ReflectionDocBlock because PHPUnit requires Prophecy requires older ReflectionDocBlock), I'll start reading up on how to get the WG created.

@TysonAndre
Copy link

I would really love feedback, be it positive or negative on this suggestion

For annotating arrays and possibly objects, supporting inline array{name: string} would be another possibility

I like the approach mentioned in https://github.com/vimeo/psalm/wiki/Typing-in-Psalm#makeshift-structs for these reasons

  • It allows you to use the same syntax in @method, @property, and anywhere else that allows a generic union type.
  • It allows you to write annotations on single lines, which is easier to parse (But possibly harder for human readers)

However, we can provide a more-specific return type by using a brace annotation:

/** @return array<int, array{name: string, type: string, active: bool}> */
function getToolsData() : array {
  return [
    ['name' => 'Psalm',     'type' => 'tool', 'active' => true],
    ['name' => 'PhpParser', 'type' => 'tool', 'active' => true]
  ];
}

Other thoughts:

  • It would be nice to be able to indicate that inline elements are optional (e.g. array{optionalField?:string|null} or array{optionalField:string|null=} (Similar to JSDoc))
  • It would be nice to support stdClass{name:string,type:string} or object{name:string,type:string} as well.

@XedinUnknown
Copy link

IMHO it could be useful to in general be able to define/document the structure of data, not just array keys.

@zonuexe
Copy link

zonuexe commented Jul 19, 2018

Phan's array shapes syntax is reasonable and I like it, but the disadvantage is that this format can not contain documentation for array properties.

ashnazg pushed a commit that referenced this issue Oct 2, 2018
Improve ContainerInterface::has() specification
ashnazg pushed a commit that referenced this issue Oct 2, 2018
* Added a short summary and "why bother"

* Fixes

* Fixes

* Fixes
@ashnazg
Copy link
Member

ashnazg commented Oct 15, 2018

If this needs to be pursued further, please bring this up as a new thread on the FIG mailing list for discussion -- https://groups.google.com/forum/#!forum/php-fig

@ashnazg ashnazg closed this as completed Oct 15, 2018
@ashnazg ashnazg self-assigned this Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests