-
-
Notifications
You must be signed in to change notification settings - Fork 400
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
targetEntity short syntax, fixes #561 #573
Conversation
It's not so easy (sorry). I'll fix it. |
Haha, no problem. Yes, it's tricky. And if the target class is in a different namespace, we also need a |
@weaverryan what yout think about create a new class to wrap the string value? So we can use it in the // ClassSourceManipulator::quoteAnnotationValue
if ($value instanceof TargetClassName) {
// TargetClassName::__tostring
return sprintf('%s::class', $value);
} This approach will prevent to change the ps: If you agree with that, please suggest me a name for this new class (and the namespace). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this! Only minor comments.
We could/should also do this for the repository annotation:
@ORM\Entity(repositoryClass="<?= $repository_full_class_name ?>")
This code is from Entity.tpl.php
. It could also be smarter it seems :)
|
||
public function getShortName(): string | ||
{ | ||
if ('self' === $this->typeHint) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The self
case is (I believe) for cases when you're adding a relation that is from one class to itself. In that case, why would we not just use the short class name like normal? I think I'm missing something :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was afraid to use self::class
, then I'd it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does self::class()
work inside the relation annotations? If it does, I guess, yea, let's use it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to use self
inside annotation but it doesn't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, no problem :).
So... is this if
statement needed? I don't see how self
is a special case. We pass the $typeHint
as the second arg to the constructor. This comes from ClassSourceManipulator
, where it has already done the logic to figure out the correct "short name" that's needed... including (I believe) when a relation is being made it itself. Can we remove this whole if block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, the ClassSourceManipulator
resolve the typeHint
, that can be self
(when self relation [1]
), class shortname (normal use [2]
), or fqcn (when conflict [3]
). Only in the case that the typeHint
value is self
we can't use it, we need to specify the class name (shortname for the self class).
class Product {
/** [1]
* @var self
* @ORM\ManyToOne(targetEntity=Product::class)
*/
private $parent;
/** [2]
* @var Category
* @ORM\ManyToOne(targetEntity=Category::class)
*/
private $category;
/** [3]
* @var \App\Entity\Subdir\Category
* @ORM\ManyToOne(targetEntity=\App\Entity\Subdir\Category::class)
*/
private $subCategory;
}
BTW I can't figure out a better way to resolve it. 😕
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor things! We're close!
return $this->typeHint; | ||
} | ||
|
||
public function __toString() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to remove this method :)
|
||
public function getShortName(): string | ||
{ | ||
if ('self' === $this->typeHint) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, no problem :).
So... is this if
statement needed? I don't see how self
is a special case. We pass the $typeHint
as the second arg to the constructor. This comes from ClassSourceManipulator
, where it has already done the logic to figure out the correct "short name" that's needed... including (I believe) when a relation is being made it itself. Can we remove this whole if block?
Thank you @rogeriolino! This is awesome - great work! |
Relation annotation targetEntity value as short class name:
Target::class