Problem/Motivation

If a class implements the magic __call() method, there is no way for an IDE like PhpStorm to properly inspect the code and determine the proper type signature for code completion, and other IDE inspections.

PHPDoc allows the @method annotation to document this.

Drupal Coding Standards should be updated to allow this.

Proposed resolution

TBD about how, where, spacing, etc.

Remaining tasks

TBD

Comments

mpdonadio created an issue. See original summary.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Issue summary: View changes
mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented

OK, proposal

- @method declarations go at the end of comment at the top of the class, with a blank line before, none after, and none between multiple @method declarations
- spacing in type signature follows standard Drupal rules for function/method declarations
- all arguments are type hinted to the most generic type, e.g. interface instead of concrete class unless a proxied function declares more specific types; this mirrors @param
- return type follows C style declaration, and mirrors @retun; $this can be used as a return type for polymorphic declarations; methods that don't return anything must use void as the return type

I don't know how this works with multiple, explicit return types instead of just mixed. Same for arguments.

Open to suggestions on the trailing description. Leaning towards optional, but not required, and except from 80 col limit.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Status: Active » Needs review
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

When __call() is used for a fixed set of potential method names, sure, +1 for mandating this. Using it is already allowed, I think, since it's not explicitly forbidden (as far as I'm aware). But it's useful to have, if it can be included, so yes: should be mandatory in those cases.
@property tags for "magic" properties accessed via __get() and __set() should follow the same rules, then, I'd say. Maybe we want to include this right here as well?

However, I'm not sure how often it happens in either case that the available method/property names are actually known in advance. It seems to me that, in most cases, you'd just use explicit methods/properties in that case.

nevergone’s picture

nevergone
Primary language Hungarian
commented

How can we move this issue forward? Related issue: #3323252: Add @method PhpDoc for EntityStorageInterface descendants

chi’s picture

chi commented

Drupal Coding Standards should be updated to allow this.

Was it actually forbidden?

neclimdul’s picture

neclimdul
He/Him
commented
Issue tags: +DX (Developer Experience)

I don't believe so but our CS tends toward if its not documented, its not allowed. Like without explicit documentation of how we allow its usage, its not supported in tools and reviews get held up bike-shedding or excluding. And without that documentation on how we allow its use, we can end up with inconsistent usage that we end up having to spend a lot of time unraveling.

That said, I don't know what would hold this up. We should just accept the phpdoc version and move on.

chi’s picture

chi commented

Coding Standard will never cover everything. Meaning there will be always some degree of freedom. If something is not prohibited explicitly you are free to use it.

BTW there are examples of @method usage in core.
https://git.drupalcode.org/project/drupal/-/blob/10.1.0-rc1/core/lib/Dru...

I propose we close this issue as "works as expected".

gueguerreiro’s picture

gueguerreiro
Primary language Portuguese, Portugal
Location Lisboa
commented

It is used there, but it is clearly not supported if we head to the api.drupal.org page: https://api.drupal.org/api/drupal/core!lib!Drupal!Component!Datetime!Dat... It looks like broken text that shouldn't be there.

I'm not sure if there's an issue queue specific for the parser that is being used to build the api.drupal.org pages, but wouldn't @method calls being explicitly supported by the Coding Standards be a pre-requirement to have that support in api.drupal.org also? See #3323252-12: Add @method PhpDoc for EntityStorageInterface descendants in the related issues, and #3077520-22: Fix and improve DateTimePlus documentation, which pertains directly to the DateTime documentation with @methods that was linked above.

I agree with #5 that if we're including @method support, we should also add support for @property methods.

chi’s picture

chi commented

@gueguerreiro, it's just a bug in API project that needs to be reported and fixed. It has nothing to do with Drupal coding standards.