#37867 closed enhancement (fixed)
Document return types for create/create_and_get/create_many() methods of factory classes
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 3.7 |
Component: | Build/Test Tools | Keywords: | has-patch |
Focuses: | docs | Cc: |
Description
The create()
, create_and_get()
, and create_many()
methods are defined on the base factory class, and use other methods that are defined as abstract and implemented by the child classes. So just documenting the return types of the child class methods isn't really that helpful, and probably won't be picked up by IDEs. But by using @method
annotations in the class-level docblocks for each of the factories we can provide this information in a form that will be helpful to those of use who use intelligent IDEs, as well as any developer who is new to the tests.
Attachments (1)
Change History (13)
#2
@
8 years ago
- Keywords reporter-feedback 2nd-opinion removed
37867.diff is a first pass, probably we should decide on a standard for the class descriptions here. What I used differs from the one for bookmarks, which is the only one that had a docblock. I have omitted @since
tags, although they could also be added, and I guess would be 3.7.
#3
in reply to:
↑ 1
@
8 years ago
Replying to johnbillion:
Which IDE are you using? This sounds like something that an IDE should be able to handle easily without needing to be prompted by a potentially very long list of
@method
comments.
PhpStorm. It is only 3 method comments. I hope the patch makes it more clear, maybe my explanation wasn't very good.
#6
follow-up:
↓ 7
@
8 years ago
The PHPDocumentation description for @method
suggests that it's specifically for use in contexts where __call()
is used on a parent class, so that there are no methods where the documentation could naturally appear. This isn't the case for create()
and friends. I'm not sure how PhpStorm and similar IDEs handle this sort of thing, but it seems like they (or their users) should be able to traverse the inheritance tree for this sort of thing. https://www.phpdoc.org/docs/latest/references/phpdoc/tags/method.html
Now, it so happens that the classes in question - create_object()
, update_object()
, create()
, etc on WP_UnitTest_Factory_For_Thing
- do not have any PHPDoc at all. I bet proper documentation of these would be enough to make your IDE document the child classes properly.
#7
in reply to:
↑ 6
@
8 years ago
Replying to boonebgorges:
The PHPDocumentation description for
@method
suggests that it's specifically for use in contexts where__call()
is used on a parent class, so that there are no methods where the documentation could naturally appear. This isn't the case forcreate()
and friends. I'm not sure how PhpStorm and similar IDEs handle this sort of thing, but it seems like they (or their users) should be able to traverse the inheritance tree for this sort of thing. https://www.phpdoc.org/docs/latest/references/phpdoc/tags/method.html
Now, it so happens that the classes in question -
create_object()
,update_object()
,create()
, etc onWP_UnitTest_Factory_For_Thing
- do not have any PHPDoc at all. I bet proper documentation of these would be enough to make your IDE document the child classes properly.
Well, the problem isn't that the methods themselves aren't documented, but that their return values aren't. Documenting them on the base class would require us to use generic values, which wouldn't help. By using the @method
annotation, we're able to specify that calling $factory->user->create_and_get()
will return a WP_User
object, whereas $factory->site->create_and_get()
will return a WP_Site
. This is helpful fore code autocompletion, and just for documentation purposes in general.
It is true of course that advanced IDEs should theoretically be able to figure out this information, but that doesn't help devs who are new to the factories to understand exactly what is going on. The @method
annotations provide explicit documentation that is helpful for everyone, I think.
#8
follow-up:
↓ 9
@
8 years ago
@DrewAPicture Can you have a think about what @jdgrimes is proposing?
#9
in reply to:
↑ 8
@
8 years ago
Replying to boonebgorges:
@DrewAPicture Can you have a think about what @jdgrimes is proposing?
Using @method
annotations would definitely be the most efficient way to go, albeit copping out of doing it the "right" way, which would of course involve redefining and documenting all of the relevant superclass methods in each of the factories.
Using @method
annotations in this way is a bit of a bastardization of their intent to be sure, but it gets the job done. I personally think in the specific context of our unit test factories, the level of potential confusion (in expecting magic methods) would be fairly low.
FWIW, we just implemented custom test factories in a plugin I work on and this is actually the same method we used to indicate return types.
#10
@
8 years ago
- Keywords has-patch added; 2nd-opinion removed
- Milestone changed from Awaiting Review to 4.9
Which IDE are you using? This sounds like something that an IDE should be able to handle easily without needing to be prompted by a potentially very long list of
@method
comments.