Replies: 40 comments
-
IMHO, these sorts of "wildcard" magic methods and properties are best documented at If you actually have a specific set of allowed methods/properties, you can already simply add multiple Besides, a wildcard would be useless for IDEs, since they can't offer a list of hints for those. At best, they could evaluate the regex/wildcard for a method/property after you type it to give you a warning about it being undefined/undocumented. |
Beta Was this translation helpful? Give feedback.
-
indeed, I was thinking mainly in IDEs, because most of them, when using modifiers, they show as an error. Dynamic classes, like ORM, that get the properties from the database directly can't guess the name of the properties for obvious reasons, so I thought using a wildcard for the field would suffice. For example, an ORM class, that has 8 columns in the database, and will have a property for each of them, times the number of modifiers, that would be Since the phpDoc is the starting point for IDEs to follow, I thought implementing it in the official supported syntax would be easier to implement in IDEs. Where would the long desc of the magic method be? I just know the shortdesc |
Beta Was this translation helpful? Give feedback.
-
That's true, but when a feature is ONLY useful to IDEs, we better make it count (as in "define it very carefully, very exhaustively, and make sure we don't change it overnight"), or IDE vendors will simply not implement it, thus making this feature redundant, or they'll implement it differently, thus few people use the feature, thus this feature becomes redundant. There's also another problem with this, in addition to the aforementioned one - Which regex dialect to use?
Also, the very scenario you outline above is one where only a specific subset from the regex is allowed, yet this can't be known from the regex alone. Thus, the kind of "help" this is supposed to give to IDEs is only reduced to the larger subset. Combined with the aforementioned ability to specify multiple I can see the kind of scenario you're talking about, but at least in my opinion, that's one scenario where one has to live with the dynamic nature of PHP.
I mean at the magic method itself, e.g. class ORM {
/**
* Fetches a field.
*
* This is a magic method that lets you fetch a field as if it's a property. The property name takes the form
* of the field name, followed by "_as_", followed by the type you want to take this field as.
* Supported types are "datetime", "int", "json", "array" and "object".
*
* @param string $property
* @return mixed Whatever you've specified you want as the property name.
*/
public function __get($property) {
//...
}
} |
Beta Was this translation helpful? Give feedback.
-
I see, very well thought out, now it really seem redundant. I didn't know I could create a doc above the |
Beta Was this translation helpful? Give feedback.
-
I think there is not much to add, closing the issue. |
Beta Was this translation helpful? Give feedback.
-
example of useful wildcards: /**
* @method Person findOneBy*()
*/
class PersonTable {
//...
}
$person = PersonTable::getInstance()->findOneByNameAndPosition('Jon', 'developer'); |
Beta Was this translation helpful? Give feedback.
-
My critisisms earlier apply to this example as well. How do you address those? |
Beta Was this translation helpful? Give feedback.
-
I will reopen the issue for further inspection; I cannot re-read the discussion at this time and will try to give a proper response at another date |
Beta Was this translation helpful? Give feedback.
-
+1 for wildcard methods |
Beta Was this translation helpful? Give feedback.
-
+1 from me too. I think this would be an incredibly useful feature. What's the latest on this? |
Beta Was this translation helpful? Give feedback.
-
Ughhh... fine... I guess a wildcard (as in the "*" and "?" meta characters, with their Windows wildcard semantics) wouldn't be difficult to implement consistently across IDEs. This means the use case outlined by @pocesar would be invalid, and only the kind of example @lividgreen showed would be valid. Is everyone who "+1"ed this feature OK with such a limitation? Also, as stated previously, IDEs can't offer a list of hints for those (since the set of possibilities is very arbitrary). At best, they could evaluate the wildcard for a method/property after you type it to give you a warning about it being undocumented... doesn't that strike anyone as severely limiting the usefulness of such a feature? |
Beta Was this translation helpful? Give feedback.
-
boen brings up a good point... I revoke my +1. It's also rare that you would literally want a * and would most likely want a set of fields instead. Being a set and not a * would mean we should still document each item individually to ensure accuracy. |
Beta Was this translation helpful? Give feedback.
-
I think it is not a big deal for IDE developers to implement proper resolution of "" wildcard. But it must be in standard first. Another variant: use different tag (something like @method* or @method-template) for that and wait until IDEs are going to implement the support"Wildcard at the end" case is not so rare, for example it is widely used in Doctrine ORM. |
Beta Was this translation helpful? Give feedback.
-
I'm unfamiliar with Doctrine ORM, can you reference an example in the documentation where such a scenario would be useful? If it's referencing a set of methods and it's just easier to use * than define the whole set - I would suggest the team hold off on this since there is an existing work around (document each instance of the method manually) and focus on functionality that isn't there yet. |
Beta Was this translation helpful? Give feedback.
-
You have to realize, that @Property before class don't mean that this property is correctly recognized by __get() magic method. But this notation helps when we use some API. |
Beta Was this translation helpful? Give feedback.
-
But that's just it - in principle, if you have an API design that demands the use of such defined-only-at-runtime properties, your API design is bad. A better one is to have a generic getter with arguments (that are documented properly, of course), as that's what you're actually doing anyway, or better yet, have an $a('date', 'MongoDate'); A common argument against the above is the fact that IDEs can't offer hints for the argument values, which is a fair point - that's why fixed set of Just because you can do something in PHP doesn't mean you should and be further encouraged by pointless additions like wildcards that miss the problem entirely. EDIT: I should point out that at this point, I'm not "against" adding wildcards - but I'm not "pro" either, as the added value appears next to none, while causing the aforementioned readability and accuracy issues. |
Beta Was this translation helpful? Give feedback.
-
+1 for me. I agree that regexes are an overkill though and would rather stick with simple wildcards. For me, a DB model / ORM class seems as a good example where they are useful. Having the following magic methods:
We could phpdoc them:
Or even better, we could name the wildcards:
Now the IDE could benefit of this in the following ways:
This even seems useful for me to be included in the human readable documentation as well. |
Beta Was this translation helpful? Give feedback.
-
Oh, and thinking about the concept, I eventually tried it for real. Finding out wildcards already work in NetBeans, at least Netbeans 7.3. :) The syntax is as follows:
Code completion behaves exactly the same way, I have described. However type hinting for the returned values does not. |
Beta Was this translation helpful? Give feedback.
-
Whoa. Well... that certainly changes the whole discussion. I was actually thinking yesterday about the moving cursor part you were talking about above (and started leaning "pro" due to that), but it already being implemented completely blows my mind. On a closer inspection, this appears to be a bug rather than a feature at this point. Disregarding that for the moment... I'm not sure what syntax would be better, considering we must also cater to properties though. I mean
vs
On the one hand, the first syntax is more verbose (and => descriptive), but with all the "$", it's becoming confusing. Then again, a "*" at the beginning doesn't make it much more readable. Perhaps if we simply remove the "$", i.e.
? Unfortunately, this doesn't currently work in NetBeans, but like I said - the current implementation is a bug anyway. Also another thing to note: The type (of the return/property) should probably NOT be allowed to contain such abstractions, for the sake of providing the second benefit mentioned above. |
Beta Was this translation helpful? Give feedback.
-
FYI, PhpSstorm proposed implementation of "Factory Method" in latest EAP build. /**
* @method-template Core myFactory({'core'}) Get Core
* @method-template Model myFactory({'model'}, name) Get model by name
* @method-template Table_{Model} getTable({Model}) Get table by name
* @method-template Table_{Model} get{Model}Table() Same as above but with magic method
*/ |
Beta Was this translation helpful? Give feedback.
-
Having a separate tag is probably a bad idea BTW, particularly due to properties - we'd have to also add Having that said, with regards to the syntaxes above:
Looks confusing. It can easily be mistaken for a string literal. Why the apostrophes anyway?
The value of the argument - and therefore, the return type - can't be determined statically, unless it's a scalar literal. This eliminates the whole point. That's not going in, for sure.
Interesting... that one could work... IF we make it explicit that it's invalid for the type of a method/property declaration to have a named wildcard that's not present in the name. There's just one potential problem - there was another proposed feature discussed previously (#557) about enumerated values for properties, which then escalated into having ways to impose restrictions on types in general. This syntax would conflict with it. Care to propose a way to reconcile those two? EDIT: Hmm... maybe if we use "<" and ">", but should we use them for this or for that?
vs
|
Beta Was this translation helpful? Give feedback.
-
@method Core myFactory({'core'})
class AClass {...} that means Factory Method support implementation (see http://youtrack.jetbrains.com/issue/WI-6027) AClass::myFactory('core') // returns an instance of Core {Model} means identifier to be used in type resolution |
Beta Was this translation helpful? Give feedback.
-
Why would you do that to begin with? If there's a specific list of possible return types from the factory method, you can do that in a single
If there's a runtime determined pattern, then specifying a literal defeats the point. Something like
Makes more sense in terms of specifying the pattern. But this goes back to what I said above - an argument's value can't be determined statically, again defeating the whole point. If the idea is to enumerate the list of possible values of the argument, and how each would relate to the return type, then IMHO, the value enumeration, combined with the fact /**
* @method
* {{
* Creates a Core object.
* @param string<'core'> $what
* }}
* Core myFactory($what)
*
* @method
* {{
* Creates a CoreExt object.
* @param string<'ext','CoreExt'> $what
* }}
* CoreExt myFactory($what)
*
* @method
* {{
* Creates a Model object from the supplied data.
* @param mixed $data Stuff to fill the model with.
* }}
* {Type}Model get{Type}Model($data)
*/
class AClass Verbose? Hell yeah, but edge cases call for edge syntax. |
Beta Was this translation helpful? Give feedback.
-
Thinking of the whole subject, I came across the following two issues:
As for the following syntax:
It assumes that the names of the returned object types match the names of methods, which may not always be the case. That makes it useful in a very special cases only, doesn't it? Finally, for the Factory method support. I understand this is a common practice in various frameworks and may be useful. It may be worth a separate discussion, as this is not really related to wildcards and may have a completely different syntax.
Anyway, this could be useful in the IDE only when the argument is not passed as a variable, as it was pointed out already. When a variable is passed in, the IDE would have to use the broadest definition available or ignore the type hinting completely. |
Beta Was this translation helpful? Give feedback.
-
They'd simply have the union of all possibilities, which I'm pretty sure they're already doing in cases you have the same method name occurring more than once, and have it return different types different times. If they aren't, IMHO, we should probably clarify that they should.
That one's even easier. As the docs already imply, the definition from the parent is added to the child's one, as you already hinted. If the parent is able to deliver "{Anything}", then it seems logical to let the child handle it too. Otherwise, you simply wouldn't document it like that. In the particular case you describe where the parent can't deliver "{Anything}" but forces children to abide a pattern, the best place to describe the pattern would be parent's
Well, there's still the possibility of a multiple
That syntax here seems like it would pose quite a parsing challenge (especially in terms of efficiency), considering what the
True, but whatever syntax we pick for this feature is going to affect future features (which is why I brought up the enumerated values and inline doc blocks into the mix), so it helps to keep it all in mind to avoid breaking the possibility of a nice new feature before said feature even appears. |
Beta Was this translation helpful? Give feedback.
-
I agree with all points. I even changed my mind on Concerning the Factory method, in any case I did not really like the proposed syntax with both curly braces and quotes:
and would rather have the quotes only or a completely different syntax. |
Beta Was this translation helpful? Give feedback.
-
Is there any conclusive spec for wildcard (or regex) @method PHPDoc? As previously stated it is very handy to use for ORM related functions, like the ones from PHPActiveRecord. I have a lot of It would also help the IDE, so that it doesnt throw warnings when I use those methods. |
Beta Was this translation helpful? Give feedback.
-
I think it's time to support it at this age. |
Beta Was this translation helpful? Give feedback.
-
Sometimes it's really useful to have a way to place wildcards in properties, because since
__get
and__call
are "magic", it's common to use modifiers to existing properties, like_as_something
like_as_json
,_as_array
,_as_int
.field_as_array
for example will return an object as an associative array, so in the Docblock, it would be written asor
that works for multiple fields
Beta Was this translation helpful? Give feedback.
All reactions